Issue 6778050: fix representation switching from line-position to staff-space
Descriptionfix representation switching from line-position to staff-space
Patch Set 1 #Patch Set 2 : compute middle-of-spaces the hard way; enhance regtest #
Total comments: 9
Patch Set 3 : do meaningful things for empty or zero-width staves; change delta to length #
Total comments: 2
Patch Set 4 : improved explanations #
MessagesTotal messages: 11
The regression tester looks for changed X/Y-extents, but gregorian.ly enlarges the extents of the BreathingSign so it can be used for these bar-line-like-things without letting notes overlap, so the regression tester does not notice these changes. http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc File lily/breathing-sign.cc (left): http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc#oldcode88 lily/breathing-sign.cc:88: int const int_dim = (int) ydim[i]; If you want to avoid ending the divisi at a fractional position, maybe just ydim[i] = int_dim; http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc File lily/breathing-sign.cc (right): http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc#newcod... lily/breathing-sign.cc:112: assert (line_pos.begin () < it); Can you assert this here, and similarly a few lines down? If someone uses a collapsed staff with all line-positions equal, I would have thought that lower_bound() would return a pointer to the first line-position, and similarly upper_bound() a pointer to one beyond the last line-position.
Sign in to reply to this message.
http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc File lily/breathing-sign.cc (left): http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc#oldcode88 lily/breathing-sign.cc:88: int const int_dim = (int) ydim[i]; On 2012/10/27 19:36:21, Keith wrote: > If you want to avoid ending the divisi at a fractional position, maybe just > ydim[i] = int_dim; Sounds like a bad idea since it truncates, leading to potentially unsymmetric results. http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc File lily/breathing-sign.cc (right): http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc#newcode99 lily/breathing-sign.cc:99: standard algorithms are suitable to find the upper line of Do we really need a complicated variable line arrangement algorithm here? Nobody complained before this was "fixed" 2.15-ish. It really looks like an overly complex solution to a self-made problem. I think that for most of those problems, the very old code looking only at the line-count property (even when it was overruled via line-positions for special effects) was producing exactly what people expected and wanted. All that magic and second-guessing does not appear to have resulted in an improvement for actual scores. Not in results, and most certainly not in predictability.
Sign in to reply to this message.
http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc File lily/breathing-sign.cc (left): http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc#oldcode88 lily/breathing-sign.cc:88: int const int_dim = (int) ydim[i]; On 2012/10/27 19:36:21, Keith wrote: > If you want to avoid ending the divisi at a fractional position, maybe just > ydim[i] = int_dim; this juggling was done only because Symbol_referencer::on_staff_line takes an int, whereas the staff positions are Real's. http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc File lily/breathing-sign.cc (right): http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc#newcode99 lily/breathing-sign.cc:99: standard algorithms are suitable to find the upper line of On 2012/10/27 19:51:49, dak wrote: > Do we really need a complicated variable line arrangement algorithm here? I want staves with line-positions like (-2 0 2 4) work. I could arrange for this a bit simpler, but the moral I drew from the line-count saga is that from time to time comes a user with tablature or line-positions (-4 4) or (-4 0 4) (I know, they are not using \divisioMaior, only repeat signs, bar lines, slurs, time signatures, etc.), and if I can cater for them, then I should do it. > Nobody complained before this was "fixed" 2.15-ish. I apologise, my original "fix" is plain silly. > It really looks like an overly complex solution to a self-made problem. of course I wouldn't mind anybody coming up with a more elegant solution, but the original code relying on the line-count property is unacceptable for me (not here, but at enough other places to make me hunt these down). > I think that for most of those problems, the very old code looking only at the > line-count property (even when it was overruled via line-positions for special > effects) was producing exactly what people expected and wanted. I started this series about a year ago because I couldn't make the old code produce what I wanted. see also your comment in http://code.google.com/p/lilypond/issues/detail?id=2783#c35 (BTW let me note that I'm working on rests as hinted at in comment 34.) > All that magic and second-guessing does not appear to have resulted in an > improvement for actual scores. Not in results, and most certainly not in > predictability. if "common" staves are not predictable, it's a bug and I'm willing to work on it. having said all this, I don't mind if this particular patch is rejected. http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc#newcod... lily/breathing-sign.cc:112: assert (line_pos.begin () < it); On 2012/10/27 19:36:21, Keith wrote: > Can you assert this here, and similarly a few lines down? If someone uses a > collapsed staff with all line-positions equal, I would have thought that > lower_bound() would return a pointer to the first line-position, and similarly > upper_bound() a pointer to one beyond the last line-position. you are right; what's more, assertions are futile in such cases, this condition must be checked (and perhaps I'll just do nothing).
Sign in to reply to this message.
On 2012/10/27 20:34:35, benko.pal wrote: > > I want staves with line-positions like (-2 0 2 4) work. Why would somebody specify (-2 0 2 4) with the expectation that the results should be identical to (-3 -1 1 3)? Why would he not specify (-3 -1 1 3) in the first place then? How is something "working" when it nullifies what the user is trying to do? I don't get it.
Sign in to reply to this message.
2012/10/27 <dak@gnu.org>: > On 2012/10/27 20:34:35, benko.pal wrote: > >> I want staves with line-positions like (-2 0 2 4) work. > Why would somebody specify (-2 0 2 4) with the expectation that the > results should be identical to (-3 -1 1 3)? Why would he not specify > (-3 -1 1 3) in the first place then? How is something "working" when > it nullifies what the user is trying to do? clefs: when specifying (-4 -2 0 2), you can use \clef alto or similar to get a c-clef on the third line. in other words: when I want to LilyPond-ize ancient music using four- or six-line staff, the expected representation is a standard staff reduced or expanded by a line. I may well imagine that for some drumming applications the best choice is a staff spaced by three, e.g. (-4 -1 2 5), because this way every pitch in the range used is unambiguously assigned to a staff line. symmetrizing such a staff of even lines won't work at all.
Sign in to reply to this message.
http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc File lily/breathing-sign.cc (left): http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc#oldcode85 lily/breathing-sign.cc:85: ydim.widen (-0.25 * ydim.delta ()); Use ydim.length() so the reader does not wonder if ydim is allowed to have reversed order. http://codereview.appspot.com/6778050/diff/1001/lily/breathing-sign.cc#oldcode88 lily/breathing-sign.cc:88: int const int_dim = (int) ydim[i]; On 2012/10/27 20:34:35, benko.pal wrote: > On 2012/10/27 19:36:21, Keith wrote: > > If you want to avoid ending the divisi at a fractional position, maybe just > > ydim[i] = int_dim; > > this juggling was done only because Symbol_referencer::on_staff_line takes an > int, whereas the staff positions are Reals. I suppose that by 'juggling' you mean creating an int version while continuing to use fractional positions. It seems that we always want to end of the divisi at a whole-number position, and then more specifically we want it on a staff position that is a space, not a line : ydim[i] = rint(ydim[i]); if (Staff_symbol_referencer::on_staff_line(me, (int) ydim[i]))) ydim[i] += 1.0 * i;
Sign in to reply to this message.
Benkő Pál <benko.pal@gmail.com> writes: > 2012/10/27 <dak@gnu.org>: >> On 2012/10/27 20:34:35, benko.pal wrote: >> >>> I want staves with line-positions like (-2 0 2 4) work. > >> Why would somebody specify (-2 0 2 4) with the expectation that the >> results should be identical to (-3 -1 1 3)? Why would he not specify >> (-3 -1 1 3) in the first place then? How is something "working" when >> it nullifies what the user is trying to do? > > clefs: when specifying (-4 -2 0 2), you can use \clef alto or similar > to get a c-clef on the third line. in other words: when I want to > LilyPond-ize ancient music using four- or six-line staff, the expected > representation is a standard staff reduced or expanded by a line. And? Why would looking at the line_count property then yield a wrong result? You are specifying a missing line in the line positions, but the overriden line count would still lead to the same positioning of clefs. Which would be exactly what was wanted. > I may well imagine that for some drumming applications the best > choice is a staff spaced by three, e.g. (-4 -1 2 5), because this way > every pitch in the range used is unambiguously assigned to a staff > line. A drum staff is not exactly going to need an alto clef. Or a divisioMaior line. I really have a hard time seeing just _what_ improvements we get over the 2.14 behavior. Is there any _real_ _existing_ problem that now works better than before? -- David Kastrup
Sign in to reply to this message.
2012/10/30 David Kastrup <dak@gnu.org>: > Benkő Pál <benko.pal@gmail.com> writes: > >> 2012/10/27 <dak@gnu.org>: >>> On 2012/10/27 20:34:35, benko.pal wrote: >>> >>>> I want staves with line-positions like (-2 0 2 4) work. >> >>> Why would somebody specify (-2 0 2 4) with the expectation that the >>> results should be identical to (-3 -1 1 3)? Why would he not specify >>> (-3 -1 1 3) in the first place then? How is something "working" when >>> it nullifies what the user is trying to do? >> >> clefs: when specifying (-4 -2 0 2), you can use \clef alto or similar >> to get a c-clef on the third line. in other words: when I want to >> LilyPond-ize ancient music using four- or six-line staff, the expected >> representation is a standard staff reduced or expanded by a line. > > And? Why would looking at the line_count property then yield a wrong > result? You are specifying a missing line in the line positions, but > the overriden line count would still lead to the same positioning of > clefs. Which would be exactly what was wanted. sorry, David, I'm lost. (-4 -2 0 2) and (-3 -1 1 3) yield different alto clefs, I expect that, and that's why I prefer overriding line-positions to line-count. >> I may well imagine that for some drumming applications the best >> choice is a staff spaced by three, e.g. (-4 -1 2 5), because this way >> every pitch in the range used is unambiguously assigned to a staff >> line. > > A drum staff is not exactly going to need an alto clef. Or a > divisioMaior line. agreed. I meant this to be an independent argument to handle asymmetric staves more gracefully, as such staves may need rests, time signatures, repeat signs, slurs, and those would look best like in tablature (= a staff with staff-space = 1.5). this patch is an independent element of a set of several such patches. (it started as one patch, then got reverted as a whole.) > I really have a hard time seeing just _what_ improvements we get over > the 2.14 behavior. Is there any _real_ _existing_ problem that now > works better than before? I never use divisioMaior, so can't speak about that. I had real existing problems with rests which launched me on this crusade against line_count a year ago. time signatures don't work the 2.14 way, as you admitted in http://code.google.com/p/lilypond/issues/detail?id=2783#c35 p
Sign in to reply to this message.
Just two things that are not quite clear to me. http://codereview.appspot.com/6778050/diff/9001/lily/breathing-sign.cc File lily/breathing-sign.cc (right): http://codereview.appspot.com/6778050/diff/9001/lily/breathing-sign.cc#newcode83 lily/breathing-sign.cc:83: high/low enough. i have no idea what "high/low enough" is, sorry :( http://codereview.appspot.com/6778050/diff/9001/lily/breathing-sign.cc#newcode99 lily/breathing-sign.cc:99: find the spaces at quarter and three quarters of the staff. you mean, of the staff height?
Sign in to reply to this message.
LGTM thanks!
Sign in to reply to this message.
On 2012/10/30 18:59:49, benko.pal wrote: > 2012/10/30 David Kastrup <dak@gnu.org>: > > Benkő Pál <mailto:benko.pal@gmail.com> writes: > > > >> 2012/10/27 <dak@gnu.org>: > >>> On 2012/10/27 20:34:35, benko.pal wrote: > >>> > >>>> I want staves with line-positions like (-2 0 2 4) work. > >> > >>> Why would somebody specify (-2 0 2 4) with the expectation that the > >>> results should be identical to (-3 -1 1 3)? Why would he not specify > >>> (-3 -1 1 3) in the first place then? How is something "working" when > >>> it nullifies what the user is trying to do? > >> > >> clefs: when specifying (-4 -2 0 2), you can use \clef alto or similar > >> to get a c-clef on the third line. in other words: when I want to > >> LilyPond-ize ancient music using four- or six-line staff, the expected > >> representation is a standard staff reduced or expanded by a line. > > > > And? Why would looking at the line_count property then yield a wrong > > result? You are specifying a missing line in the line positions, but > > the overriden line count would still lead to the same positioning of > > clefs. Which would be exactly what was wanted. > > sorry, David, I'm lost. (-4 -2 0 2) and (-3 -1 1 3) yield different alto > clefs, > I expect that, and that's why I prefer overriding line-positions to > line-count. With all the line_count changes, we have basically two use cases. a) Things that depend on a particular position of each line. Those include collision avoidance, micro-alignment to staff or ledger lines, basically pretty much every use case that would equally well hold for ledger lines as it would for "regular" lines. b) positioning relative to an entire Staff. This includes the positioning of clefs, positioning of keys, positioning of things like the divisio maior. In case of the divisio maior, actually both seem relevant: one would position according to b) and then might need to slightly adjust ends in case they hit actual stafflines in a bad way. What I am arguing is that it is a _mistake_ for use case b) to actually look at the position of each single staff line. Just using line-count rather than essentially (if (null? line-positions) line-count (length line-positions)) would make things much more predictable. Particularly in the case if irregularly spaced stafflines, it becomes impossible for the user to get a predictable positioning of elements in case he does not want to go with whatever LilyPond picks for him. > > I really have a hard time seeing just _what_ improvements we get over > > the 2.14 behavior. Is there any _real_ _existing_ problem that now > > works better than before? > > I never use divisioMaior, so can't speak about that. > I had real existing problems with rests which launched > me on this crusade against line_count a year ago. Rests are "case a)". That's different: of course things combining visually with actual lines need to look at the actual lines. No issue with that. > time signatures don't work the 2.14 way, as you admitted in > http://code.google.com/p/lilypond/issues/detail?id=2783#c35 We settled on "no shift at all" which was essentially the 2.14 way for most use cases. The 2.14 way for most use cases was basically established by two bugs/design errors cancelling each other, and when one got handled in 2.15, the other surfaced.
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
