Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

changing default spaces #1182

Closed
eroux opened this issue Jul 10, 2016 · 21 comments
Closed

changing default spaces #1182

eroux opened this issue Jul 10, 2016 · 21 comments
Assignees
Milestone

Comments

@eroux
Copy link
Contributor

eroux commented Jul 10, 2016

Many of the current spaces have not been carefully chosen (by me), and I think changing them is only possible for quite big projects as it requires to understand how the spacing work, which is not obvious. So having better default spaces should be quite beneficial to many. The Antiphonale Monasticum projects allowed us to take the spaces they use, I'll make a pull request and we can discuss them

@eroux eroux added this to the 4.2 milestone Jul 10, 2016
eroux added a commit that referenced this issue Jul 10, 2016
@eroux eroux modified the milestones: 5.0, 4.2 Jul 12, 2016
@eroux eroux self-assigned this Jul 12, 2016
@eroux
Copy link
Contributor Author

eroux commented Nov 21, 2016

Here are the spaces used by the italian Antiphonale Project:

\grechangedim{annotationraise}{-2mm}{scalable}
\grechangedim{beforeinitialshift}{2mm}{scalable}%
\grechangedim{afterinitialshift}{2mm}{scalable}%
\greseteolhyphen{normal}

\grechangedim{moraadjustment}{0.05 cm}{scalable}%
\grechangedim{moraadjustmentbar}{0.05 cm}{scalable}%
%\grechangedim{alterationadjustmentbar}{0.07 cm}{scalable}%
\grechangedim{bar@virgula@standalone@notext}{0.2 cm}{scalable}%
\grechangedim{bar@minima@standalone@notext}{0.2 cm}{scalable}%
\grechangedim{bar@minor@standalone@notext}{0.2323 cm}{scalable}%
\grechangedim{bar@dominican@standalone@notext}{0.2323 cm}{scalable}%
\grechangedim{bar@maior@standalone@notext}{0.2323 cm}{scalable}%
\grechangedim{bar@finalis@standalone@notext}{0.2323 cm}{scalable}%
\grechangedim{bar@finalfinalis@standalone@notext}{0.29169 cm}{scalable}%
\grechangedim{bar@rubber}{0 cm plus 0.025cm minus 0.025 cm}{scalable}%
\grechangedim{bar@minima@short}{0.12 cm plus 0.05 cm minus 0.00469 cm}{scalable}%
\grechangedim{bar@virgula@short}{0.13 cm plus 0.05 cm minus 0.00469 cm}{scalable}%
%\grechangedim{bar@finalfinalis@standalone@notext}{0.2323 cm}{scalable}%
\grechangedim{interwordspacetext@bars@notext}{0.19 cm}{scalable}%
\grechangedim{interwordspacetext@bars@notext@euouae}{0.18 cm}{scalable}%
\grechangedim{interwordspacetext}{0.17 cm plus 0.05 cm minus 0.05 cm}{scalable}%
\grechangedim{interwordspacenotes@euouae}{0.23 cm plus 0.1 cm minus 0.05 cm}{scalable}%
\grechangedim{interwordspacenotes}{0.29 cm plus 0.05 cm minus 0.05 cm}{scalable}%
\grechangedim{interwordspacetext@euouae}{0.21 cm plus 0.1 cm minus 0.05 cm}{fixed}%
\grechangedim{maxbaroffsettextright}{0.15 cm}{scalable}%
\grechangedim{maxbaroffsettextleft}{0.3 cm}{scalable}%

\grechangedim{spaceafterlineclef}{0.23 cm plus 0 cm minus 0.01367 cm}{scalable}%
\grechangedim{spacebeforeeolcustos}{0.23 cm plus 0 cm minus 0 cm}{scalable}%
\grechangedim{shortspaceafterlineclef}{0.18 cm plus 0 cm minus 0.01367 cm}{scalable}%

\grechangedim{spaceabovelines}{0.45576 cm plus 0.45576 cm minus 0.20114 cm}{scalable}%
\grechangedim{emergencystretch}{1mm}{scalable}

\grechangedim{baselineskip}{11.5pt}{scalable}%

\gresetcustosalteration{invisible}

Now I'm not totally sure what to do with it... Although I think they are reasonable defaults, changing them in the test repository will have a big impact and it will be impossible to review the diff and know when a bug cannot be spotted anymore because of the change in spaces... so maybe having a gsp-pre5 with the current spaces (which we'd use in the test repository) and use these new ones in the gsp-default? Wdyt?

@rpspringuel
Copy link
Contributor

I'm not sure what the problem is. If done on its own branch then the changes caused by these spacing changes would be separate from any due to other changes already in progress.

@eroux
Copy link
Contributor Author

eroux commented Nov 21, 2016

My point is that there are certainly many tests that depend on some special circumstances, like some line breaks here or there, and if spaces are not the same, the original bug will not be shown anymore, and thus the test will be made irrelevant

@rpspringuel
Copy link
Contributor

That's an issue with any subsequent change. Granted, changing the spaces is more pervasive, but the change to the bar spacing algorithm was similarly pervasive.

✝✝✝✝✝✝✝✝✝✝✝✝✝✝✝✝✝✝✝✝✝✝✝✝
Br. Samuel, OSB
(R. Padraic Springuel)
PAX ☧ ΧΡΙΣΤΟΣ

On Nov 21, 2016, at 9:11 AM, Elie Roux notifications@github.com wrote:

My point is that there are certainly many tests that depend on some special circumstances, like some line breaks here or there, and if spaces are not the same, the original bug will not be shown anymore, and thus the test will be made irrelevant


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

@henryso
Copy link
Contributor

henryso commented Nov 21, 2016

I think we should do a best-effort pass through the tests to determine which ones look specifically at things like line endings and put comments in the tests themselves to call that out. Certainly, for the fix-XXXX tests, we can refer back to the issues themselves. Then, we take those identified tests and apply whatever technique necessary to keep them testing whatever they are testing and then just accept the rest using the new spacing configuration.

@eroux
Copy link
Contributor Author

eroux commented Nov 22, 2016

ok for that! I'm affraid I won't have time to do that any time soon though, end of this year will be quite busy...

@henryso
Copy link
Contributor

henryso commented Dec 19, 2016

#1266 (comment) indicates that there are a few more interline spacings that need to change, so I'm not closing this issue yet.

@eroux
Copy link
Contributor Author

eroux commented Jan 5, 2017

The values for horizontal spacing that the Italian Antiphonale is using are:

\grechangedim{spaceabovelines}{0 cm}{scalable}%
\grechangedim{emergencystretch}{1mm}{scalable}
\grechangedim{parskip}{1pt plus 1pt}{scalable}%
\grechangedim{lineskip}{0pt plus 1pt}{scalable}%
\grechangedim{baselineskip}{55pt plus 5pt minus 5pt}{scalable}%

the results are quite good...

@henryso
Copy link
Contributor

henryso commented Jan 5, 2017

Other than emergencystretch, you mean vertical spacing, no?

@eroux
Copy link
Contributor Author

eroux commented Jan 5, 2017

sorry, yes indeed! Well, maybe emergencystretch could be ignored here, I'm not entirely sure...

@henryso
Copy link
Contributor

henryso commented Jan 5, 2017

I'll leave emergencystretch as \the\emergencystretch, as described in #1266.

@henryso
Copy link
Contributor

henryso commented Jan 5, 2017

This is personal opinion, but looking at the results, I feel like a spaceabovelines of 0 cm is too small. Probably the old value (0.45576 cm plus 0.45576 cm minus 0.20114 cm) is too big, but setting this to 0 cm could mislead the eye (at least mine) to thinking the text goes with the line below.

image

@eroux
Copy link
Contributor Author

eroux commented Jan 5, 2017

I think you're getting this impression because most lines have very low notes, but on a more regular score it looks nice... professional books are printed with these settings so I don't think it's a problem... If you have books with scores with such a large ambitus, then changing it is probably a good idea, but I think 95% of users only have more "regular" scores... what do you think?

@henryso
Copy link
Contributor

henryso commented Jan 5, 2017

Well, I think that's a typical score, but this is only my opinion, so I'm willing to give up my argument.

@eroux
Copy link
Contributor Author

eroux commented Jan 5, 2017

Hmmm, that's not my experience (I'm currently proofreading the italian Antiphonale, which is quite big), but it certainly depends on the type of book, and maybe it should be indicated somewhere that if most scores have very low notes, spaceabovelines should be augmented... or maybe we should take the opposite approach? I don't know... anyway we should find a way to document it

@henryso
Copy link
Contributor

henryso commented Jan 5, 2017

I actually feel this way because of the high notes on the staff below, not because of the low notes on the staff above. But again, this is my opinion, and of course I can set my own personal value to whatever I want.

@eroux
Copy link
Contributor Author

eroux commented Jan 5, 2017

Yes, but both things count, the reason why it looks like the text is belonging to the notes below is also becaue it's far from the notes above, because of the low notes involving a big distance between the text and corresponding notes

@henryso
Copy link
Contributor

henryso commented Jan 5, 2017

Since my argument/opinion is not very strong, I'll drop it.

Regarding what you said earlier about documenting this: what is the best way to document this? I can add it to the GregorioRef PDF, but I don't think many people would find it easily there.

@eroux
Copy link
Contributor Author

eroux commented Jan 5, 2017

Well... that's certainly a good question... I think it would have its place in a medium-level tutorial, but we don't have that, and it's hard to fit it somewhere... maybe in the tips and tricks section, something about tuning the vertical spaces? It's really not an obvious part, I can write it myself if you want.

@henryso
Copy link
Contributor

henryso commented Jan 6, 2017

Because this affects all of the multi-line gabc-output tests, I am going to hold off on updating the expectations and creating the pull request for the new spaces until #1273 and #1274 are reviewed and merged (or rejected).

@henryso
Copy link
Contributor

henryso commented Jan 7, 2017

Leaving this open for @eroux to implement documentation as noted above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants