Re: Lyrics break estimation of vertical spacing
Please forgive me for bumping this discussion, but I was wondering if Valentin, I am sorry I have disappeared from the Lilypond scene for a while. My work on Lilypond development has been temporarily put on the back burner. Right now, we are concentrating on something slightly different: we are working to secure a very large Lilypond-based contract, for a major series of critical editions by a major publisher (I'm not allowed to divulge any details yet including who the publisher is, but I am sure everyone on this list is familiar with the name). IF we are successful, it will mean a radical breakthrough in acceptance for Lilypond. Some time ago, I was talking about how I wanted to transform Lilypond from "a volunteer project with limited resources" (Graham's definition), into a professional open-source project where at least some of the core people can afford to spend non-trivial amounts of time on their passion. I'll come back as soon as I have something to announce (either good or bad). Boris ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Mon, Nov 15, 2010 at 08:40:26AM +0100, David Kastrup wrote: > > In commercial settings, stagnation often means death. With free > software, stagnation mostly means stagnation. > This is one of the strenghts of free software. Another is that over time it becomes tayored to actual users and actual potential users as opposed to those in some salesman's head. If lily had been a commercial product it might well have died years ago. It would be nice to have commercial backing but wheter or not this happens my guess is that lily will become the defacto standard for computer typesetting of music over the next 10 years or so. Bernard ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
Boris Shingarov writes: > My work on Lilypond development has been temporarily put on the back > burner. Right now, we are concentrating on something slightly > different: we are working to secure a very large Lilypond-based > contract, for a major series of critical editions by a major publisher > (I'm not allowed to divulge any details yet including who the > publisher is, but I am sure everyone on this list is familiar with the > name). IF we are successful, it will mean a radical breakthrough in > acceptance for Lilypond. Some time ago, I was talking about how I > wanted to transform Lilypond from "a volunteer project with limited > resources" (Graham's definition), into a professional open-source > project where at least some of the core people can afford to spend > non-trivial amounts of time on their passion. I'll come back as soon > as I have something to announce (either good or bad). Well, one nice thing with free software development is that you can't really announce something bad (except perhaps for your personal plans). If everything goes wrong with your endeavour, the project is not worse off than before. In commercial settings, stagnation often means death. With free software, stagnation mostly means stagnation. -- David Kastrup ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Wed, Nov 10, 2010 at 2:01 PM, Valentin Villenave wrote: > Please forgive me for bumping this discussion, but I was wondering if > your work on the Lyrics spacing had ever been implemented and merged > into LilyPond. Oh, never mind. Just found it: http://code.google.com/p/lilypond/issues/detail?q=1053 Sorry for the noise! Valentin. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Tue, Mar 30, 2010 at 2:08 PM, Boris Shingarov wrote: > Ok, it's fully functional now. I am going to format and upload a patch. Greetings Boris, Please forgive me for bumping this discussion, but I was wondering if your work on the Lyrics spacing had ever been implemented and merged into LilyPond. If not, do you want me to open a page in the tracker so we don't forget about it? (Unless I'm totally mistaken and it's already been done, in which case I apologize for having missed it.) Cheers, Valentin. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
On Thu, 22 Apr 2010 13:39:35 -0700, Joe Neeman wrote: Never mind, I just did that hunk manually and pushed. Then, I am marking bug 1053 as fixed. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
"Boris Shingarov" writes: > What makes me really depressed about the situation with pure-height, > is that we have fixed a number of "reasonable" bugs in this area > (intersystem begin/rest, overridden stem length, deprecated space, > padding of markup -- these are the ones that I did in the immediate > past, -- the slur fix from Joe yesterday, and I also recall a bunch of > other fixes in this area in the last few months), but we are not only > not closer to having reasonable trouble-free page layout, but starting > to look at page overfill/underfill problems which are very deeply > rooted in the nature of pure-height estimation. > > So much so that I am starting to think that sacrificing the benefit of > linebreaking/pagebreaking integration in the sake of always running > real (non-pure) height, would be the path to having a reasonable > layout for our book. That is, calculate the line breaks disregarding > page breaking; calculate tallness of all lines; then run the page > breaking algorithm. In case the line breaking algorithm is TeX-like, namely using a shortest graph traversal algorithm (Viterbi), it would be a considerable improvement if the first pass did not establish the optimal breaks, but rather the maximum and minimum heights of material. Then the page breaker would pick its goals (avoid widows, avoid extra pages and so on), and the line breaking would be done again, this time using optimal breaks while keeping the page break goals focused. A complete separation of line- and page breaking makes it infeasible to avoid things like widows and clubs (single lines on one page) by picking better line breaks. That actually is a rather severe limitation with TeX: while TeX has penalties for widows and clubs (and hyphens at the end of a page), they come into play only once line breaking is already completed. With a rigid layout, there is nothing you can do to fix the previous bad decision. You can only call it foul and have TeX emit a warning. I think we need to be better than that. -- David Kastrup ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
What makes me really depressed about the situation with pure-height, is that we have fixed a number of "reasonable" bugs in this area (intersystem begin/rest, overridden stem length, deprecated space, padding of markup -- these are the ones that I did in the immediate past, -- the slur fix from Joe yesterday, and I also recall a bunch of other fixes in this area in the last few months), but we are not only not closer to having reasonable trouble-free page layout, but starting to look at page overfill/underfill problems which are very deeply rooted in the nature of pure-height estimation. So much so that I am starting to think that sacrificing the benefit of linebreaking/pagebreaking integration in the sake of always running real (non-pure) height, would be the path to having a reasonable layout for our book. That is, calculate the line breaks disregarding page breaking; calculate tallness of all lines; then run the page breaking algorithm. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
On Wed, 21 Apr 2010 20:37:52 -0400, Boris Shingarov wrote: not closer to having reasonable trouble-free page layout, but starting > to look at page overfill/underfill problems which are very deeply > rooted in the nature of pure-height estimation. I meant Bug 1061 in particular. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
Ok, latest patch is uploaded now. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
Oops, I had missed it. Reviewed now. On Mon, 2010-04-19 at 19:48 -0400, Boris Shingarov wrote: > Hi Joe, > > Have you seen the last patchset I uploaded to Rietveld 3 days ago? Is > there anything expected from me at this point to further the progress > on this bug? > ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
Hi Joe, Have you seen the last patchset I uploaded to Rietveld 3 days ago? Is there anything expected from me at this point to further the progress on this bug? ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
Ah, you might want to install git-cl, which is a git interface to the > upload.py script Thanks -- this does make life a lot easier!!! ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
On Mon, Apr 12, 2010 at 1:43 PM, Boris Shingarov wrote: > >> If you use the same test branch as your original patch (which is > > > associated with a particular issue on Rietveld), you can just apply > > the revised patch and upload again (git-cl will ask you for a > > description which becomes the title of the new patch set on Rietveld.) > > I am not sure I am following, esp. the git-cl part. You need to install "git-cl". It's just a matter of cloning this repo and adding symlinks to git-cl and upload.py in your PATH. http://neugierig.org/software/git/index.cgi?url=git-cl/ -Patrick ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
Am Montag, 12. April 2010 22:43:30 schrieb Boris Shingarov: > Hi Neil, > > > If you use the same test branch as your original patch (which is > > > > associated with a particular issue on Rietveld), you can just apply > > the revised patch and upload again (git-cl will ask you for a > > description which becomes the title of the new patch set on Rietveld.) > > I am not sure I am following, esp. the git-cl part. The state of the > test branch right now is simply a clone of the HEAD. I had made some > changes, then ran the "upload.py" script to generate the first patch. Ah, you might want to install git-cl, which is a git interface to the upload.py script. It stores the issue for you current branch, and when you do a second upload on the same branch, it will reuse that issue number... > This was without any commits. Then, I made some corrections, and > re-ran "upload.py" again -- this is the revised patch. Sure I can do > a diff of the two patches, but how do I get that diff to Rietveld? You don't have to do that yourself. Just upload the new patch to the same issue on Rietveld, and the server will create the diff of the patches for you... Cheers, Reinhold -- -- Reinhold Kainhofer, reinh...@kainhofer.com, http://reinhold.kainhofer.com/ * Financial & Actuarial Math., Vienna Univ. of Technology, Austria * http://www.fam.tuwien.ac.at/, DVR: 0005886 * LilyPond, Music typesetting, http://www.lilypond.org ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
Hi Neil, If you use the same test branch as your original patch (which is > associated with a particular issue on Rietveld), you can just apply > the revised patch and upload again (git-cl will ask you for a > description which becomes the title of the new patch set on Rietveld.) I am not sure I am following, esp. the git-cl part. The state of the test branch right now is simply a clone of the HEAD. I had made some changes, then ran the "upload.py" script to generate the first patch. This was without any commits. Then, I made some corrections, and re-ran "upload.py" again -- this is the revised patch. Sure I can do a diff of the two patches, but how do I get that diff to Rietveld? ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
On 12 April 2010 21:03, Boris Shingarov wrote: > This is the whole patch. I understand your idea, but how do I add it to the > previous set? Git will not help here, because neither of the two were > committed yet. If you use the same test branch as your original patch (which is associated with a particular issue on Rietveld), you can just apply the revised patch and upload again (git-cl will ask you for a description which becomes the title of the new patch set on Rietveld.) Cheers, Neil ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
On Mon, 12 Apr 2010 20:57:00 0100, Neil Puttock wrote: On 12 April 2010 18:46, Boris Shingarov wrote: > > Ok, this patch is ready for code review: > > http://codereview.appspot.com/872044 > > Can you add this to the previous patch set so it's easier to see > what's changed since the last review? This is the whole patch. I understand your idea, but how do I add it to the previous set? Git will not help here, because neither of the two were committed yet. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
On 12 April 2010 18:46, Boris Shingarov wrote: > Ok, this patch is ready for code review: > http://codereview.appspot.com/872044 Can you add this to the previous patch set so it's easier to see what's changed since the last review? Thanks, Neil ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
Ok, this patch is ready for code review: http://codereview.appspot.com/872044 On Thu, 08 Apr 2010 23:47:15 -0700, Joe Neeman wrote: On Mon, 2010-04-05 at 03:00 -0400, Boris Shingarov wrote: > > > >Grob *alignment = get_vertical_alignment (); //TODO check for null > > > please check for null > > > > But what do we do if it's null? > > Maybe print a programming_error and return an empty extent? It doesn't > particularly matter what you do, just don't crash. > > Joe > > > ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
On Fri, 2010-04-09 at 02:55 -0400, Boris Shingarov wrote: > > Alternatively, you could just check explicitly for an > > empty result in System::part_of_line_pure_height. > > And if it's empty, then not do the translation at all, right? > Will do this right now and see how much better this > gets us. > > BTW, what is the correct (supposed) way to run the > regtests? I am just running Lilypond on all the files > from a bash script, as I can't seem to find the script > that would be supposed to run the suite -- if such a > beast exists. Checkout a known good commit (eg. origin/master) make test-baseline git checkout my-branch make && make test-clean && make check There's probably something in the contributor's guide about this... Joe ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
Alternatively, you could just check explicitly for an > empty result in System::part_of_line_pure_height. And if it's empty, then not do the translation at all, right? Will do this right now and see how much better this gets us. BTW, what is the correct (supposed) way to run the regtests? I am just running Lilypond on all the files from a bash script, as I can't seem to find the script that would be supposed to run the suite -- if such a beast exists. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
On Thu, 08 Apr 2010 23:47:15 -0700, Joe Neeman wrote: > But what do we do if it's null? > Maybe print a programming_error and return an empty extent? It doesn't > particularly matter what you do, just don't crash. But it's never null, on any input that I've seen. Not on any of the regtests nor on our book anyway. So ok, I'll do as you suggest, but I'll add a comment explaining that this is somewhat arbitrary. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
On Mon, 2010-04-05 at 03:00 -0400, Boris Shingarov wrote: > > >Grob *alignment = get_vertical_alignment (); //TODO check for null > > please check for null > > But what do we do if it's null? Maybe print a programming_error and return an empty extent? It doesn't particularly matter what you do, just don't crash. Joe ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
On Fri, 2010-04-09 at 02:30 -0400, Boris Shingarov wrote: > What I am stuck on now, is the function > Align_interface::get_minimum_translations(). > Could you explain the high-level design of what this is doing? The function builds a skyline for each staff/lyrics/etc, then compares the skylines to find the minimum amounts that each staff must be moved down so that no collisions occur. > In all cases in the book I am trying to typeset, everything appears > to work A-ok, but on the "hara-kiri-pianostaff" regtest, this now returns > an empty vector of offsets when called from > System::part_of_line_pure_height(), > but the corresponding vector of staves is non-empty, which causes > a segfault when trying to translate the 0th staff. get_skylines modifies its elems argument, removing all staves that are empty. If every staff is empty (which happens in hara-kiri-pianostaff, IIRC) then get_skylines doesn't return any skylines and hence get_minimum_translations (see align-interface.cc:226) returns an empty vector. I'm not actually sure why it does this (instead of returning, say, a vector of zeros) so if the regression tests pass, I'd be ok with changing this. Alternatively, you could just check explicitly for an empty result in System::part_of_line_pure_height. Cheers, Joe ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
What I am stuck on now, is the function Align_interface::get_minimum_translations(). Could you explain the high-level design of what this is doing? In all cases in the book I am trying to typeset, everything appears to work A-ok, but on the "hara-kiri-pianostaff" regtest, this now returns an empty vector of offsets when called from System::part_of_line_pure_height(), but the corresponding vector of staves is non-empty, which causes a segfault when trying to translate the 0th staff. On Sun, 04 Apr 2010 11:47:48 -0700, Joe Neeman wrote: On Wed, 2010-03-31 at 01:46 -0400, Boris Shingarov wrote: > > Wooo, hang on, my patch is sour! It segfaults if the system is not a > > group of staves -- for example, this is the case of markup. I'm > > fixing it. > > Ok, so I'll save the full review for later, but here are a few quick > things I noticed: > > > @@ -512,7 524,6 @@ Line_details::Line_details (Prob *pb, Output_def > > *paper) > > > >last_column_ = 0; > >force_ = 0; > > - extent_ = unsmob_stencil (pb->get_property ("stencil")) ->extent > > You'll need to do something better here, rather than just leaving all > the extent information out for markup blocks. > > > (Y_AXIS); > >bottom_padding_ = 0; > >space_ = 0.0; > >inverse_hooke_ = 1.0; > > @@ -530,3 541,33 @@ Line_details::Line_details (Prob *pb, Output_def > > *paper) > >SCM first_scm = pb->get_property ("first-markup-line"); > >first_markup_line_ = to_boolean (first_scm); > > } > > > > /* > > * I measure hanging from top of page. It is positive for everything > > * below the top of page. Lower things have bigger hanging. > > * NB!!! These hangings are artificial in that they do not take into > > * account any padding/spacing. > hangings need to take padding (but not spacing) into account, because > padding is non-stretchable space. See (the old version of) > Page_breaking::min_page_count, which uses padding. > > > They are as if systems were stacked > > * on top of each other; as such, hangings are only used/useful for > > the > > * calculation of ext_len in Page_breaking. > > */ > > void Line_details::compute_hangings (double previous_begin, double > > previous_rest) > we use Real rather than double (not quite sure why, but let's be > consistent). Also, the code style is > > void > Line_details::compute_hangings ... > > > { > >double a = begin_of_line_extent_[UP]; > >double b = rest_of_line_extent_[UP]; > >double midline_hanging = max (previous_begin a, previous_rest > > b); > >hanging_begin_ = midline_hanging - begin_of_line_extent_[DOWN]; > >hanging_rest_ = midline_hanging - rest_of_line_extent_[DOWN]; > > } > > > > double Line_details::hanging () > double > Line_details::hanging () > > > { > >return max (hanging_begin_, hanging_rest_); > > } > > > > double Line_details::full_height () > > { > >Interval ret; > >ret.unite(begin_of_line_extent_); > >ret.unite(rest_of_line_extent_); > >return ret.length(); > > } > > diff --git a/lily/include/constrained-breaking.hh > > b/lily/include/constrained-breaking.hh > > index e6d898e..1f0e952 100644 > > --- a/lily/include/constrained-breaking.hh > > b/lily/include/constrained-breaking.hh > > @@ -27,7 27,11 @@ > > struct Line_details { > >Grob *last_column_; > >Real force_; > > - Interval extent_; /* Y-extent of the system */ > >Interval begin_of_line_extent_; > >Interval rest_of_line_extent_; > >double hanging_begin_; > >double hanging_rest_; > >double y_extent_; /* Y-extent, adjusted according to > When do you set this? Doesn't full_height() remove the need for this > anyway? > > > begin/rest-of-line*/ > > > >Real padding_; /* compulsory space after this system (if we're not > > last on a page) */ > > @@ -78,6 82,16 @@ struct Line_details { > >} > > > >Line_details (Prob *pb, Output_def *paper); > > > >/* > > * Pure procedure. > > * Based on the arguments which indicate how low the previous > > system > > * hangs, and on the internal state (*_of_line_extents), store into > > * internal state (hanging_*) how low our own low margin hangs. > > */ > >void compute_hangings (double previous_begin, double > > previous_rest); > >double hanging (); > >double full_height (); > > }; > > > > /* > > diff --git a/lily/include/system.hh b/lily/include/system.hh > > index 509a65d..b8fa664 100644 > > --- a/lily/include/system.hh > > b/lily/include/system.hh > > @@ -65,9 65,15 @@ public: > >void typeset_grob (Grob *); > >void pre_processing (); > > > >Interval begin_of_line_pure_height (vsize start, vsize end); > >Interval rest_of_line_pure_height (vsize start, vsize end); > > > > protected: > >virtual void derived_mark () const; > >virtual Gro
Re: PATCH: Lyrics break estimation of vertical spacing
Hi Joe, You'll need to do something better here, rather than just leaving all > the extent information out for markup blocks. Right, this is exactly why that oldest version of the patch was segfaulting. The second version (attached to the bug and formatted on codreview.appspot, also linked to from the bug) has this corrected. > > * NB!!! These hangings are artificial in that they do not take into > > * account any padding/spacing. hangings need to take padding (but not spacing) into account, because > padding is non-stretchable space. See (the old version of) > Page_breaking::min_page_count, which uses padding. Correct; the code does do it, the meaning of the comment is to warn that it happens not in the hanging method but in its caller; this is ok because the hangings are only used to calculate the rod heights. > void Line_details::compute_hangings (double previous_begin, double > > previous_rest) > we use Real rather than double (not quite sure why, but let's be > consistent). I did put some thought into this; it appears that in some places we actually use double, and I *think* the pattern is, Real for allowing /-Inf, double when we want to specifilally allow proper finite values only. I this case, the method demands the value of the argument to be finite, this is why I chose double. Also, the code style is > > void > Line_details::compute_hangings ... Thanks, corrected. >double y_extent_; /* Y-extent, adjusted according to When do you set this? In min_page_count(). Doesn't full_height() remove the need for this > anyway? No, full_height() is different. full_height() is similar to the old unify() of the begin/rest; y_extent_ is the fine-adjusted depending on the surrounding lines (well, the previous line to speak correctly). >if (i > 0) > >{ > indent the brace: > if (i > 0) > { > foo; > } Thanks. > - rod_height_ = line.extent_.length (); > >rod_height_ = line.y_extent_; > You'll need to use the same hanging logic here as you do in > min_page_count. Otherwise, min_page_count will be able to pack pages > tighter than the page-spacer can space them, which will cause problems. > Same for space_systems_on_2_pages and space_systems_on_1_page. Thanks, I missed this! >Grob *alignment = get_vertical_alignment (); //TODO check for null please check for null But what do we do if it's null? Boris ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
On Wed, 2010-03-31 at 01:46 -0400, Boris Shingarov wrote: > Wooo, hang on, my patch is sour! It segfaults if the system is not a > group of staves -- for example, this is the case of markup. I'm > fixing it. Ok, so I'll save the full review for later, but here are a few quick things I noticed: > @@ -512,7 +524,6 @@ Line_details::Line_details (Prob *pb, Output_def > *paper) > >last_column_ = 0; >force_ = 0; > - extent_ = unsmob_stencil (pb->get_property ("stencil")) ->extent You'll need to do something better here, rather than just leaving all the extent information out for markup blocks. > (Y_AXIS); >bottom_padding_ = 0; >space_ = 0.0; >inverse_hooke_ = 1.0; > @@ -530,3 +541,33 @@ Line_details::Line_details (Prob *pb, Output_def > *paper) >SCM first_scm = pb->get_property ("first-markup-line"); >first_markup_line_ = to_boolean (first_scm); > } > + > +/* > + * I measure hanging from top of page. It is positive for everything > + * below the top of page. Lower things have bigger hanging. > + * NB!!! These hangings are artificial in that they do not take into > + * account any padding/spacing. hangings need to take padding (but not spacing) into account, because padding is non-stretchable space. See (the old version of) Page_breaking::min_page_count, which uses padding. > They are as if systems were stacked > + * on top of each other; as such, hangings are only used/useful for > the > + * calculation of ext_len in Page_breaking. > + */ > +void Line_details::compute_hangings (double previous_begin, double > previous_rest) we use Real rather than double (not quite sure why, but let's be consistent). Also, the code style is void Line_details::compute_hangings ... > +{ > + double a = begin_of_line_extent_[UP]; > + double b = rest_of_line_extent_[UP]; > + double midline_hanging = max (previous_begin + a, previous_rest + > b); > + hanging_begin_ = midline_hanging - begin_of_line_extent_[DOWN]; > + hanging_rest_ = midline_hanging - rest_of_line_extent_[DOWN]; > +} > + > +double Line_details::hanging () double Line_details::hanging () > +{ > + return max (hanging_begin_, hanging_rest_); > +} > + > +double Line_details::full_height () > +{ > + Interval ret; > + ret.unite(begin_of_line_extent_); > + ret.unite(rest_of_line_extent_); > + return ret.length(); > +} > diff --git a/lily/include/constrained-breaking.hh > b/lily/include/constrained-breaking.hh > index e6d898e..1f0e952 100644 > --- a/lily/include/constrained-breaking.hh > +++ b/lily/include/constrained-breaking.hh > @@ -27,7 +27,11 @@ > struct Line_details { >Grob *last_column_; >Real force_; > - Interval extent_; /* Y-extent of the system */ > + Interval begin_of_line_extent_; > + Interval rest_of_line_extent_; > + double hanging_begin_; > + double hanging_rest_; > + double y_extent_; /* Y-extent, adjusted according to When do you set this? Doesn't full_height() remove the need for this anyway? > begin/rest-of-line*/ > >Real padding_; /* compulsory space after this system (if we're not > last on a page) */ > @@ -78,6 +82,16 @@ struct Line_details { >} > >Line_details (Prob *pb, Output_def *paper); > + > + /* > + * Pure procedure. > + * Based on the arguments which indicate how low the previous > system > + * hangs, and on the internal state (*_of_line_extents), store into > + * internal state (hanging_*) how low our own low margin hangs. > + */ > + void compute_hangings (double previous_begin, double > previous_rest); > + double hanging (); > + double full_height (); > }; > > /* > diff --git a/lily/include/system.hh b/lily/include/system.hh > index 509a65d..b8fa664 100644 > --- a/lily/include/system.hh > +++ b/lily/include/system.hh > @@ -65,9 +65,15 @@ public: >void typeset_grob (Grob *); >void pre_processing (); > > + Interval begin_of_line_pure_height (vsize start, vsize end); > + Interval rest_of_line_pure_height (vsize start, vsize end); > + > protected: >virtual void derived_mark () const; >virtual Grob *clone () const; > + > +private: > + Interval part_of_line_pure_height (vsize start, vsize end, bool > begin); > }; > > void set_loose_columns (System *which, Column_x_positions const > *posns); > diff --git a/lily/page-breaking.cc b/lily/page-breaking.cc > index 7dbac44..1741f5c 100644 > --- a/lily/page-breaking.cc > +++ b/lily/page-breaking.cc > @@ -101,8 +101,6 @@ compress_lines (const vector &orig) > Line_details compressed = orig[i]; > Real padding = orig[i].title_ ? old.title_padding_ : > old.padding_; > > - compressed.extent_[DOWN] = old.extent_[DOWN]; > - compressed.extent_[UP] = old.extent_[UP] + > orig[i].extent_.length () + padding; > compressed.space_ += old.space_; > compressed.inverse_hooke_ += old.inverse_hooke_; > > @@ -853,11 +851,27 @@ Page_breaking::min_page_count (vsize > configuration, vsize first_page_num) > >fo
Re: Lyrics break estimation of vertical spacing [now: Issue 1053]
1) what's your gmail account address? 2) try logging out and then back in. When you click on the "Add a comment and make changes" box at the bottom, it should give you a number of options. Cheers, - Graham On Thu, Apr 1, 2010 at 7:51 AM, Boris Shingarov wrote: > Ok, I changed the Owner, but I don't see how I am supposed to change the > Status, etc. Becoming blind? > > On Thu, 1 Apr 2010 07:42:08 0100, Graham Percival wrote: > On Thu, Apr 1, 2010 at 5:23 AM, Boris Shingarov wrote: > > > Ok, so I created Issue 1053. The next logical step would be to assign > this > > > to myself, indicating that I am working on the code to fix it, and link > from > > > the bug report to the patch. Can I do this? > > > > You're now a member of the googlecode lilypond project, so yes. > Please > make it type-enhancement, priority-medium, status-started, > > owner shingarov. > > > Cheers, > > - Graham > > > > > > > ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing [now: Issue 1053]
Ok, I changed the Owner, but I don't see how I am supposed to change the Status, etc. Becoming blind? On Thu, 1 Apr 2010 07:42:08 0100, Graham Percival wrote: On Thu, Apr 1, 2010 at 5:23 AM, Boris Shingarov wrote: > > Ok, so I created Issue 1053. The next logical step would be to assign this > > to myself, indicating that I am working on the code to fix it, and link from > > the bug report to the patch. Can I do this? > > You're now a member of the googlecode lilypond project, so yes. > Please make it type-enhancement, priority-medium, status-started, > owner shingarov. > > Cheers, > - Graham > > ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing [now: Issue 1053]
On Thu, Apr 1, 2010 at 5:23 AM, Boris Shingarov wrote: > Ok, so I created Issue 1053. The next logical step would be to assign this > to myself, indicating that I am working on the code to fix it, and link from > the bug report to the patch. Can I do this? You're now a member of the googlecode lilypond project, so yes. Please make it type-enhancement, priority-medium, status-started, owner shingarov. Cheers, - Graham ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing [now: Issue 1053]
Ok, so I created Issue 1053. The next logical step would be to assign this to myself, indicating that I am working on the code to fix it, and link from the bug report to the patch. Can I do this? On Wed, 31 Mar 2010 03:47:27 -0400, Boris Shingarov wrote: Hi Dmytro, > > > where it is a bug and where it is not. But if you can provide a minimal > > example, it will be easier. > > The idea definitely is to provide a minimal example. Otherwise, it > does not count as a bug report, and shouldn't really even be posted on > the -bug list. > > The problem is that our understanding of a bug evolves with time -- it > is usually very unclear at the beginning. > > So was the case with this particular bug. When I first started > looking at this behavior to try to understand what was causing it, I > only knew that the low-hanging lyrics were somehow responsible. So I > named the email, "Lyrics break estimation of vertical spacing". We > now know exactly what the bug is caused by, and we know that this case > with lyrics, is only one scenario. The file "lyrics.ly" is one > minimal example. But the critical ingredient of the bug, is not > lyrics, but anything hanging very low under the staff. It can be > lyrics, or it can be notes on ledger lines: this is what my second > example ("no-lyrics.ly") illustrates. Now, is this name a good name > for the bug report? No, it's not a good name. But I didn't change > the subject line on the thread, because it was just a continuation of > the technical discussion about how to fix the bug. > > Boris > > > > > > ___ > bug-lilypond mailing list > bug-lilypond@gnu.org > http://lists.gnu.org/mailman/listinfo/bug-lilypond > > ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
У ср, 2010-03-31 у 03:47 -0400, Boris Shingarov пише: > Hi Dmytro, Hi Boris, thank you for you explanation. If you will not add this issue at the tracker (as Graham mentioned), i guess i will. Anyway --- "invalid" issue report is not a huge problem, too .) As for me. > > where it is a bug and where it is not. But if you can provide a minimal > > example, it will be easier. > > The idea definitely is to provide a minimal example. Otherwise, it > does not count as a bug report, and shouldn't really even be posted on > the -bug list. > > The problem is that our understanding of a bug evolves with time -- it > is usually very unclear at the beginning. > > So was the case with this particular bug. When I first started > looking at this behavior to try to understand what was causing it, I > only knew that the low-hanging lyrics were somehow responsible. So I > named the email, "Lyrics break estimation of vertical spacing". We > now know exactly what the bug is caused by, and we know that this case > with lyrics, is only one scenario. The file "lyrics.ly" is one > minimal example. But the critical ingredient of the bug, is not > lyrics, but anything hanging very low under the staff. It can be > lyrics, or it can be notes on ledger lines: this is what my second > example ("no-lyrics.ly") illustrates. Now, is this name a good name > for the bug report? No, it's not a good name. But I didn't change > the subject line on the thread, because it was just a continuation of > the technical discussion about how to fix the bug. > > Boris -- Dmytro O. Redchuk ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
On Wed, 31 Mar 2010 01:46:12 -0400, Boris Shingarov wrote: > Here is the patch for the "begin/rest-of-line" problem. Wooo, hang on, my patch is sour! It segfaults if the system is not a group of staves -- for example, this is the case of markup. I'm fixing it. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
У ср, 2010-03-31 у 04:29 -0400, Boris Shingarov пише: > Hi Dmytro, Hi Boris, > > If you will not add this issue at the tracker (as Graham mentioned), i > > guess i will. > > I'll do it, but I'll ask you qustions please do :-) > if I stumble upon something > unclear, because I am new to the Lilypond bug procedure. me too ;-) > Boris -- Dmytro O. Redchuk ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
Hi Dmytro, where it is a bug and where it is not. But if you can provide a minimal > example, it will be easier. The idea definitely is to provide a minimal example. Otherwise, it does not count as a bug report, and shouldn't really even be posted on the -bug list. The problem is that our understanding of a bug evolves with time -- it is usually very unclear at the beginning. So was the case with this particular bug. When I first started looking at this behavior to try to understand what was causing it, I only knew that the low-hanging lyrics were somehow responsible. So I named the email, "Lyrics break estimation of vertical spacing". We now know exactly what the bug is caused by, and we know that this case with lyrics, is only one scenario. The file "lyrics.ly" is one minimal example. But the critical ingredient of the bug, is not lyrics, but anything hanging very low under the staff. It can be lyrics, or it can be notes on ledger lines: this is what my second example ("no-lyrics.ly") illustrates. Now, is this name a good name for the bug report? No, it's not a good name. But I didn't change the subject line on the thread, because it was just a continuation of the technical discussion about how to fix the bug. Boris ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
Hi Dmytro, If you will not add this issue at the tracker (as Graham mentioned), i > guess i will. I'll do it, but I'll ask you qustions if I stumble upon something unclear, because I am new to the Lilypond bug procedure. Boris ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
Hi Graham, I'm doing everything I can to increase the number of people involved > in lilypond, including the bug squad. If you would like to volunteer > to help with this, I'd love to have you on board. My involvement with Lilypond is pragmatic. I am enabling the publication of a top-quality, musicologically definitive, academic edition. (Not pretending to be a Berenreiter, it is, nevertheless, definitely a book of status). I need to do whatever I have to do to make sure this book happens. Therefore, I am interested in working on issues (bugs / missing / new functionality) that stand in the way of this book. Right now, the are many code issues. I need to have those resolved, and of course contribute these resoltions back to the community. This needs to happen according to proper software discipline. This is why I do care about properly documenting this particular bug in the tracker. you permission to add items directly to the tracker. If you'd like to do this, please send me the name of your gmail account (creating a new > one if necessary). It's "shingarov". I'll probably ask Dmytro for some direction with filing this bug, as I am new to the Lilypond bug procedure. Boris ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
On Wed, Mar 31, 2010 at 7:05 AM, Boris Shingarov wrote: > Hi Graham, > > > ... I > > would expect somebody to add it to the tracker. I'm not certain > > if that has always happened, though. > So in other words, not all bugs have bug reports in the tracker? It depends on how well the Bug Squad have been doing their job. I'd estimate that between 90% and 100% of bugs have been correctly entered into the tracker. I'm doing everything I can to increase the number of people involved in lilypond, including the bug squad. If you would like to volunteer to help with this, I'd love to have you on board. As an aside to the general policy discussion, I'm comfortable giving you permission to add items directly to the tracker. If you'd like to do this, please send me the name of your gmail account (creating a new one if necessary). > If the discussion takes a technical turn from the very beginning, > then we just discuss the patch through into the codebase, but > no trail in the tracker remains? Correct. Again, help acepted. > Hmm. I'd find it useful if we documented and kept trail of all > bugs in the tracker. You know, putting tags on pushes in the > form "fix for bug #XYZ", or if someone asks "what does this > code do" you can reply "see bug #XYZ", that kind of stuff. Well, the mailing list archives *are* available. But again, if we had more people on the Bug Squad, especially more technically-minded people, we could increase the scope of their coverage. Cheers, - Graham ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
У ср, 2010-03-31 у 02:05 -0400, Boris Shingarov пише: > Hi Graham, > > > If you send a minimal example to this list, and no developer > > responds within a few days to begin discussing a patch, then I > > would expect somebody to add it to the tracker. I'm not certain > > if that has always happened, though. > > So in other words, not all bugs have bug reports in the tracker? > If the discussion takes a technical turn from the very beginning, > then we just discuss the patch through into the codebase, but > no trail in the tracker remains? Hi, Boris! As one of persons who ideally should track all these discussions and submit proper bugreports, i can say that i can not decide in every case where it is a bug and where it is not. But if you can provide a minimal example, it will be easier. See also: http://lilypond.org/doc/v2.13/Documentation/web/bug-reports I understand that probably not every case may be illustrated by minimal example --- that is the difficulty for Bug Squad and for developers, i guess. ps. In this particular case --- i read every post, i assume there can be a bug somewhere, but for me --- it's hard to say *what* is a bug, *where* is it. You say: "Lyrics break estimation of vertical spacing" --- so, no-lyrics.ly should contain no "unwanted" vertical space? But it does, with 2.13.16. Or i've missed something? Next, i've commented out everything in \paper {} block (left paper-size alone) and problem disappeared. So, how this bug should be titled? "Lyrics... breaks.. *if* " or like that? I'd *love* to submit a bugreport :O) But i can not, sorry :-( pps. "Melody" in your example can be minimized to \repeat "unfold" 392 { f e } It's quote good for minimal example, i believe. Thank You! > Hmm. I'd find it useful if we documented and kept trail of all > bugs in the tracker. You know, putting tags on pushes in the > form "fix for bug #XYZ", or if someone asks "what does this > code do" you can reply "see bug #XYZ", that kind of stuff. -- Dmytro O. Redchuk ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
Hi Graham, If you send a minimal example to this list, and no developer > responds within a few days to begin discussing a patch, then I > would expect somebody to add it to the tracker. I'm not certain > if that has always happened, though. So in other words, not all bugs have bug reports in the tracker? If the discussion takes a technical turn from the very beginning, then we just discuss the patch through into the codebase, but no trail in the tracker remains? Hmm. I'd find it useful if we documented and kept trail of all bugs in the tracker. You know, putting tags on pushes in the form "fix for bug #XYZ", or if someone asks "what does this code do" you can reply "see bug #XYZ", that kind of stuff. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: PATCH: Lyrics break estimation of vertical spacing
On Wed, Mar 31, 2010 at 01:46:12AM -0400, Boris Shingarov wrote: > Btw, how does one file a proper file report? > The bug tracker says scary things to the effect of "never create > a bug report directly in this tracker, or risk being permanently > killed; instead, write to the -bug maillist." Well at least this is > my (mis-?)understanding of it. I have written numeruos reports > here, but never actually saw it result in a tracked report. The people who handle bug reports are normal users, and there's a natural tendency to be intimidated by discussions like this one. If a technical discussion is going on between a contributor (you) and a developer (Joe), we just tend to leave it alone. If you send a minimal example to this list, and no developer responds within a few days to begin discussing a patch, then I would expect somebody to add it to the tracker. I'm not certain if that has always happened, though. Cheers, - Graham ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
PATCH: Lyrics break estimation of vertical spacing
Here is the patch for the "begin/rest-of-line" problem. (Sorry, couldn't upload to appspot for various reasons). I have also attached two score examples illustrating the bug. We should probably augment the test suite, too, for the bug to be considered done done. Btw, how does one file a proper file report? The bug tracker says scary things to the effect of "never create a bug report directly in this tracker, or risk being permanently killed; instead, write to the -bug maillist." Well at least this is my (mis-?)understanding of it. I have written numeruos reports here, but never actually saw it result in a tracked report. What is the correct procedure? -- Boris Shingarov Work on Lilypond under grant from Sonus Paradisi / Jiri Zurek (Prague), Czech Science Foundation, Project No. 401/09/0419 diff --git a/lily/constrained-breaking.cc b/lily/constrained-breaking.cc index 0db98cc..78079ef 100644 --- a/lily/constrained-breaking.cc +++ b/lily/constrained-breaking.cc @@ -458,7 +471,8 @@ Constrained_breaking::fill_line_details (Line_details *const out, vsize start, v int start_rank = Paper_column::get_rank (all_[breaks_[start]]); int end_rank = Paper_column::get_rank (all_[breaks_[end]]); System *sys = pscore_->root_system (); - Interval extent = sys->pure_height (sys, start_rank, end_rank); + Interval begin_of_line_extent = sys->begin_of_line_pure_height (start_rank, end_rank); + Interval rest_of_line_extent = sys->rest_of_line_pure_height (start_rank, end_rank); Grob *c = all_[breaks_[end]]; out->last_column_ = c; @@ -476,20 +490,18 @@ Constrained_breaking::fill_line_details (Line_details *const out, vsize start, v out->turn_permission_ = min_permission (out->page_permission_, out->turn_permission_); - // TODO: see the hack regarding begin_of_line and - // rest_of_line extents in align-interface. Perhaps we - // should do the same thing here so that the effect extends - // between systems as well as within systems. It isn't as - // crucial here, however, because the effect is largest when - // dealing with large systems. - out->extent_ = (extent.is_empty () - || isnan (extent[LEFT]) - || isnan (extent[RIGHT])) -? Interval (0, 0) : extent; + out->begin_of_line_extent_ = (begin_of_line_extent.is_empty () + || isnan (begin_of_line_extent[LEFT]) + || isnan (begin_of_line_extent[RIGHT])) +? Interval (0, 0) : begin_of_line_extent; + out->rest_of_line_extent_ = (rest_of_line_extent.is_empty () + || isnan (rest_of_line_extent[LEFT]) + || isnan (rest_of_line_extent[RIGHT])) +? Interval (0, 0) : rest_of_line_extent; out->padding_ = between_system_padding_; out->title_padding_ = before_title_padding_; out->space_ = between_system_space_; - out->inverse_hooke_ = extent.length () + between_system_space_; + out->inverse_hooke_ = out->full_height () + between_system_space_; } Real @@ -512,7 +524,6 @@ Line_details::Line_details (Prob *pb, Output_def *paper) last_column_ = 0; force_ = 0; - extent_ = unsmob_stencil (pb->get_property ("stencil")) ->extent (Y_AXIS); bottom_padding_ = 0; space_ = 0.0; inverse_hooke_ = 1.0; @@ -530,3 +541,33 @@ Line_details::Line_details (Prob *pb, Output_def *paper) SCM first_scm = pb->get_property ("first-markup-line"); first_markup_line_ = to_boolean (first_scm); } + +/* + * I measure hanging from top of page. It is positive for everything + * below the top of page. Lower things have bigger hanging. + * NB!!! These hangings are artificial in that they do not take into + * account any padding/spacing. They are as if systems were stacked + * on top of each other; as such, hangings are only used/useful for the + * calculation of ext_len in Page_breaking. + */ +void Line_details::compute_hangings (double previous_begin, double previous_rest) +{ + double a = begin_of_line_extent_[UP]; + double b = rest_of_line_extent_[UP]; + double midline_hanging = max (previous_begin + a, previous_rest + b); + hanging_begin_ = midline_hanging - begin_of_line_extent_[DOWN]; + hanging_rest_ = midline_hanging - rest_of_line_extent_[DOWN]; +} + +double Line_details::hanging () +{ + return max (hanging_begin_, hanging_rest_); +} + +double Line_details::full_height () +{ + Interval ret; + ret.unite(begin_of_line_extent_); + ret.unite(rest_of_line_extent_); + return ret.length(); +} diff --git a/lily/include/constrained-breaking.hh b/lily/include/constrained-breaking.hh index e6d898e..1f0e952 100644 --- a/lily/include/constrained-breaking.hh +++ b/lily/include/constrained-breaking.hh @@ -27,7 +27,11 @@ struct Line_details { Grob *last_column_; Real force_; - Interval extent_; /* Y-extent of the system */ + Interval begin_of_line_extent_; + Interval rest_of_line_extent_; + double hanging_begin_; + double hanging_rest_; + double y_extent_; /* Y-extent, adjusted according to begin/rest-of-line*/ Real padding_; /* compulsory space after this system (if we're not
Re: Lyrics break estimation of vertical spacing
On Tue, 30 Mar 2010 08:08:41 -0400, Boris Shingarov wrote: Ok, it's fully functional now. > I am going to format and upload a patch. Hmmm... still overestimates on my "real-life" manuscript, even though several different scenarios (with and withou lyrics) which were restimating, are A-ok now... ok, digging into my hole again to see what breaks this time around... ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
Ok, it's fully functional now. I am going to format and upload a patch. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
dynamic_cast? (or maybe my mind is corrupted by 15 years > of Smalltalk, and this is a standard C quirk?) Oops, keyboard failure, obviously meant "C quirk"! ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
Replace constrained-breaking.cc:461 by > > Interval begin_extent = sys->begin_of_line_extent (start, end); > Interval rest_extent = sys->rest_of_line_extent (start, end); Ok, experimenting with this, I am inclined to *add* instead of *replacing*. The extent_ member of Line_details is touched in many other places, not just Page_breaking (e.g. the spacer also uses it). I wanted to replace it everywhere first, but now I think it's probably too intrusive. All I need is access to the two Intervals in Page_breaking::min_page_count(). Interval System::begin_of_line_pure_height(Grob *me, vsize start, vsize end) > { > System *sys = dynamic_cast (me); This I have been always wondering about, it's happening throughout all the Grob code. Why not just take "this" as the argument? I *am* in class System to bgin with, so why the dynamic_cast? (or maybe my mind is corrupted by 15 years of Smalltalk, and this is a standard C quirk?) ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Mon, 2010-03-29 at 21:25 -0400, Boris Shingarov wrote: > Hi Joe, > > > Replace constrained-breaking.cc:461 by > > > > Interval begin_extent = sys->begin_of_line_extent (start, end); > > Interval rest_extent = sys->rest_of_line_extent (start, end); > > > > and replace constrained-breaking.cc:485 by something analogous. > > Btw, it's not clear to me what the inverse_hooke_ should be in this > case -- I am setting it to max of the two extents, it should probably > be calculated later in the game, when we stack the systems on top of > each other at which point we know how long the actual rod is. We'll > see if we get any head ache because of inverse_hooke_. ::shrug:: I doubt it. I don't think any of the spring forces are really significant here. Getting a better height estimate is much more important. Cheers, Joe ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Mon, 2010-03-29 at 20:49 -0400, Boris Shingarov wrote: > > I'd rather discuss this point now, because I don't like the extension of > > Interval (and I'd rather you didn't spend a whole lot of time getting it > > to work if there is a nicer way) > > No kidding, neither do I like diluting Interval. One, it is an extremely > hacky way to do things. Two, it *does* introduce problems all over the > rest of the code: various idiosyncrasies of C showing up here and > there, ending up having a donzen of different places fixed already, but > no one knows how many more will surface. A situation definitely NOT > to be happy about. > > > Replace constrained-breaking.cc:461 by > > > > Interval begin_extent = sys->begin_of_line_extent (start, end); > > Interval rest_extent = sys->rest_of_line_extent (start, end); > > Yes, but how do I implement begin_of_line_extent()? > Grob::pure_height() simply looks up the value of the "Y-extent" property, > which is a Scheme function, and applies it. The problem that I've been > struggling with during the past several days, is that this is generic. > > Oh wait. Are you saying that System has Axis_group_interface? Yes. > So that I can call Axis_group_interface::*_of_line_pure_height() on it? > Let me try it right now. No, because *_of_line_pure_height only works properly for VerticalAxisGroup. What you'll need to do is to write System:*_of_line_pure_height. For example (typed directly into my mail client, so it probably won't even compile): Interval System::begin_of_line_pure_height(Grob *me, vsize start, vsize end) { System *sys = dynamic_cast (me); Grob *alignment = me->get_vertical_alignment (); // check for null extract_grob_set (alignment, "elements", staves); vector offsets = Align_interface::get_minimum_translations (alignment, staves, Y_AXIS, true, start, end); Interval ret; for (vsize i = 0; i < staves.size(); ++i) { Interval iv = Axis_group_interface::begin_of_line_pure_height(staves[i], start); iv.translate (offsets[i]); ret.unite (iv); } return ret; } For more code reuse, it might be better to write one function which computes both begin_of_line_ and rest_of_line_pure_height. Cheers, Joe ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
Hi Joe, Replace constrained-breaking.cc:461 by > > Interval begin_extent = sys->begin_of_line_extent (start, end); > Interval rest_extent = sys->rest_of_line_extent (start, end); > > and replace constrained-breaking.cc:485 by something analogous. Btw, it's not clear to me what the inverse_hooke_ should be in this case -- I am setting it to max of the two extents, it should probably be calculated later in the game, when we stack the systems on top of each other at which point we know how long the actual rod is. We'll see if we get any head ache because of inverse_hooke_. ::shrug:: ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
I'd rather discuss this point now, because I don't like the extension of > Interval (and I'd rather you didn't spend a whole lot of time getting it > to work if there is a nicer way) No kidding, neither do I like diluting Interval. One, it is an extremely hacky way to do things. Two, it *does* introduce problems all over the rest of the code: various idiosyncrasies of C showing up here and there, ending up having a donzen of different places fixed already, but no one knows how many more will surface. A situation definitely NOT to be happy about. Replace constrained-breaking.cc:461 by > > Interval begin_extent = sys->begin_of_line_extent (start, end); > Interval rest_extent = sys->rest_of_line_extent (start, end); Yes, but how do I implement begin_of_line_extent()? Grob::pure_height() simply looks up the value of the "Y-extent" property, which is a Scheme function, and applies it. The problem that I've been struggling with during the past several days, is that this is generic. Oh wait. Are you saying that System has Axis_group_interface? So that I can call Axis_group_interface::*_of_line_pure_height() on it? Let me try it right now. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Mon, 2010-03-29 at 17:04 -0400, Boris Shingarov wrote: > > Do we really need this to be included in every Interval? I'd have > > thought that the only data structure that needs to change is > > Line_details. > > Right, but how do you get the actual value in there? Replace constrained-breaking.cc:461 by Interval begin_extent = sys->begin_of_line_extent (start, end); Interval rest_extent = sys->rest_of_line_extent (start, end); and replace constrained-breaking.cc:485 by something analogous. > Hang on a bit, I am debugging the patch, so I can post it and then we > can look at actual code instead of drawing pictures in the air. I'd rather discuss this point now, because I don't like the extension of Interval (and I'd rather you didn't spend a whole lot of time getting it to work if there is a nicer way): intervals are used all over the code and I think they should really remain intervals, rather than hiding additional complexity. If you really need to carry all of the double-intervals around, I think you should create a separate class for them, so they are only used where we want to use them. Cheers, Joe ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Mon, 29 Mar 2010 12:04:42 -0700, Joe Neeman wrote: but I think you don't need to store an > extra version of rod-height. Just define rod-height to measure the > bottom of the last staff (whether that bottom comes from > begin_of_line_extent or rest_of_line_extent). Each time you add a staff, > you just need to see how far it needs to be from the last staff (this is > where the two separate extents come in) and then you increment > rod_height by > > rod_height = distance_between_staves > - min(cur_staff.begin_extent_[DOWN], cur_staff.rest_of_extent_[DOWN]) > min(last_staff.begin_extent_[DOWN], last_staff.rest_of_extent_[DOWN]); Right -- the important point that we need to keep two values indicating how low the last system hangs; whether these two are kept in last_staff.*_extent_, or in two rods, is an implementation detail. Hang on a bit, I am debugging the patch, so I can post it and then we can look at actual code instead of drawing pictures in the air. Boris ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
Do we really need this to be included in every Interval? I'd have > thought that the only data structure that needs to change is > Line_details. Right, but how do you get the actual value in there? It's filled from calling the Y-extent Scheme function, which can be anything, and in most cases it calls back into C land, but my point is that all these APIs are completely generic, you can't statically tell in which places in code you need the extra interval. My answer to this is an Interval that can be augmented by another Interval, but is in all other respects just an Interval. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Mon, 2010-03-29 at 15:53 -0400, Boris Shingarov wrote: > On Mon, 29 Mar 2010 08:48:35 -0700, Joe Neeman wrote: > > > Keep in mind that a lyric line is the same as a staff to the > > page-breaker. Don't your one-staffers have lyrics? If so, they are > > really two-staffers and so the problem could still be in the > > within-system estimation. > > My one-staffers do have lyrics, but I have overwhelmingly convincing > proof the problem is the between-system estimation. (Also, the > bug exhibits itself even when there are no lyrics, just music on > low ledgers -- the estimator does not know that they go below > the top point of the treble clef on the next line; and because these > are one-liners, the error is very significant (30% of the page is > blank)). > > I've already spent some time writing a patch, and it's almost ready. > The difficult problem is not in the estimator itself -- it's just simple > arithmetic -- but in intervalues crossing the libGuile FFI boundary, > there is a lot of places where the API we are traversing is generic, > and we don't know whether in this instance we are dealing with > a single Interval or with a (begin, rest) pair. > What I did was I generalized "Interval" to be augmentable with > a "rest-of-line" interval, but only if need be. Do we really need this to be included in every Interval? I'd have thought that the only data structure that needs to change is Line_details. Cheers, Joe ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Mon, 29 Mar 2010 08:48:35 -0700, Joe Neeman wrote: Keep in mind that a lyric line is the same as a staff to the > page-breaker. Don't your one-staffers have lyrics? If so, they are > really two-staffers and so the problem could still be in the > within-system estimation. My one-staffers do have lyrics, but I have overwhelmingly convincing proof the problem is the between-system estimation. (Also, the bug exhibits itself even when there are no lyrics, just music on low ledgers -- the estimator does not know that they go below the top point of the treble clef on the next line; and because these are one-liners, the error is very significant (30% of the page is blank)). I've already spent some time writing a patch, and it's almost ready. The difficult problem is not in the estimator itself -- it's just simple arithmetic -- but in intervalues crossing the libGuile FFI boundary, there is a lot of places where the API we are traversing is generic, and we don't know whether in this instance we are dealing with a single Interval or with a (begin, rest) pair. What I did was I generalized "Interval" to be augmentable with a "rest-of-line" interval, but only if need be. This is hacky in that it touches the template parametrization in interval.hh: typedef Interval_t Interval; becomes: typedef Interval_t IntervalBase; and then the class used by everyone is derived from IntervalBase: class Interval : public IntervalBase { ... } I'll post the patch shortly. Boris ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Wed, 2010-03-24 at 22:01 -0400, Boris Shingarov wrote: > First of all, thank you Joe for all the explanations -- now that it > has dawned on me what the source of confusion was, it all > makes sense. Thanks!! > > > I am now thinking about how to implement this TODO best. > > One idea that immediately comes to mind, is to replace the > > unified heights with begin- and rest-of-line pairs, dragging them > > Looking at Page_breaking::min_page_count(), I am thinking > about the correct generalization of the concept of "rod_height", > so that we would still have one "current position", but two > "current bottom edges" (so the next current position is calculated > by juxtaposing the next system's two top edges against these > current bottom edges). > The spring length, will also remain one. > > Do you agree with this line of thinking, or am I way off with my idea? I don't think you're way off, but I think you don't need to store an extra version of rod-height. Just define rod-height to measure the bottom of the last staff (whether that bottom comes from begin_of_line_extent or rest_of_line_extent). Each time you add a staff, you just need to see how far it needs to be from the last staff (this is where the two separate extents come in) and then you increment rod_height by rod_height += distance_between_staves - min(cur_staff.begin_extent_[DOWN], cur_staff.rest_of_extent_[DOWN]) + min(last_staff.begin_extent_[DOWN], last_staff.rest_of_extent_[DOWN]); The minimums and subtraction looks a bit confusing, but you need to remember that the [DOWN] part of an interval is the smaller number, so the distance from the middle of a staff to the bottom of the staff is -min(begin_extent_[DOWN], rest_of_extent_[DOWN]) Cheers, Joe ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Wed, 2010-03-24 at 19:42 -0400, Boris Shingarov wrote: > On Wed, 24 Mar 2010 18:15:45 -0400, Boris Shingarov wrote: > > Hmm, let me understand this better. > > Does it mean that Constrained_breaking::fill_line_details() gets > > called before the line breaks are calculated, and then again after > > Oh, now I understand why!!! The hack addresses the space > between staves within a system. > Bien sûr it's not doing anything to the space between systems > (mine are one-staffers, so it *does* cause the estimation to be > off *a lot*). > The TODO at constrained-breaking.cc:479 clearly explains it. Keep in mind that a lyric line is the same as a staff to the page-breaker. Don't your one-staffers have lyrics? If so, they are really two-staffers and so the problem could still be in the within-system estimation. > I am now thinking about how to implement this TODO best. > One idea that immediately comes to mind, is to replace the > unified heights with begin- and rest-of-line pairs, dragging them > through all the calls, and unite them at the very end, that is in > constrained-breaking. For one-staff systems, this should be > trivial, but I am not sure how difficult it will be to not break the > general case of multi-staff systems. You would need to separate Line_details.extent_ into Line_details.begin_of_line_extent_ and Line_details.rest_of_line_extent_. I'd suggest writing a System::begin_of_line_extent and System::rest_of_line_extent, which finds the VerticalAlignment, calculates the pure-offsets for the staves and then unifies the appropriate xxx_of_line_extents, with each one at the appropriate offset. Cheers, Joe ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
Looking at Page_breaking::min_page_count(), I am thinking > about the correct generalization of the concept of "rod_height", > so that we would still have one "current position", but two > "current bottom edges" (so the next current position is calculated > by juxtaposing the next system's two top edges against these > current bottom edges). Or maybe, we could get by with just adjusting the mutual positions of the two extents when they are unite()'d... trying to think of what this simplification might break: ok, why do we care about the extent, with its [UP] and [DOWN] parts, instead of just the length? - because some of the variables in the layout are relative to the middle line, not the edges of the bounding box. If we vertically shift the begin- and rest- extents for the systems against each other, we no longer have a clear definition of middle line, so we are broken. Am I missing a way to solve this, in a way which is easier than a full implementation of two rods? ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
First of all, thank you Joe for all the explanations -- now that it has dawned on me what the source of confusion was, it all makes sense. Thanks!! I am now thinking about how to implement this TODO best. > One idea that immediately comes to mind, is to replace the > unified heights with begin- and rest-of-line pairs, dragging them Looking at Page_breaking::min_page_count(), I am thinking about the correct generalization of the concept of "rod_height", so that we would still have one "current position", but two "current bottom edges" (so the next current position is calculated by juxtaposing the next system's two top edges against these current bottom edges). The spring length, will also remain one. Do you agree with this line of thinking, or am I way off with my idea? ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Wed, 24 Mar 2010 18:15:45 -0400, Boris Shingarov wrote: Hmm, let me understand this better. > Does it mean that Constrained_breaking::fill_line_details() gets > called before the line breaks are calculated, and then again after Oh, now I understand why!!! The hack addresses the space between staves within a system. Bien sûr it's not doing anything to the space between systems (mine are one-staffers, so it *does* cause the estimation to be off *a lot*). The TODO at constrained-breaking.cc:479 clearly explains it. I am now thinking about how to implement this TODO best. One idea that immediately comes to mind, is to replace the unified heights with begin- and rest-of-line pairs, dragging them through all the calls, and unite them at the very end, that is in constrained-breaking. For one-staff systems, this should be trivial, but I am not sure how difficult it will be to not break the general case of multi-staff systems. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
Right, the positions of the staff lines relative to each other are not > yet computed, so each extent is relative to its own staff. Hmm, let me understand this better. Does it mean that Constrained_breaking::fill_line_details() gets called before the line breaks are calculated, and then again after they are known? and at that later stage, sys->pure_height() (constrained-breaking.cc:461) should return a smaller value? but no, it's the "pure" one, i.e. by definition pre-linebreak... ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Wed, 24 Mar 2010 14:19:21 -0700, Joe Neeman wrote: Right, but in align-interface.cc:118, we throw away that unite()d extent > in favour of a begin_of_line_extent and a rest_of_line_extent. But that, IIUC, is only happening one level (of the grob tree) above, when it's too late, no? That is, inside Grob::pure_relative_y_coordinate() (grob.cc:456), which follows the Guile call, the returned value from which is already the unify()'ed value, and then we translate *that* value (already too big). I suspect that *may* be where the bug is, but I am not sure. The end result of severe overestimation of height when low-hanging rest-of-line elements are present (such as lyrics or low notes), has been there for a very long time. To me, the whole mechanism of beggining-vs-rest-of-line, is exactly about it, so that means that the whole mechanism is broken (or is there an example where it is working?) But I find it hard to believe that we have been content with living with it for so long. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Wed, 2010-03-24 at 16:08 -0400, Boris Shingarov wrote: > > The skylines are used as part of VerticalAlignment, to figure out how > > far apart the children of VerticalAlignment (ie. VerticalAxisGroups for > > staves, lyrics, etc) need to be. Once we compute the translations of the > > children of VerticalAlignment, we can compute the height of > > VerticalAlignment (and thus the height of System). > > I understand that this is the general idea how it is supposed to be. > What happens in reality, is that pure_height() of the VerticalAxisGroup, > calling ly:hara-kiri-group-spanner::y-extent in Scheme land, > calling back Hara_kiri_group_spanner::pure_height(), ultimately > ends up in Axis_group_interface::cached_pure_height(). > That function, unite()'s the begin_of_line_pure_height() and > the rest_of_line_pure_height(), Right, but in align-interface.cc:118, we throw away that unite()d extent in favour of a begin_of_line_extent and a rest_of_line_extent. > which are simply measured from > the middle line of the staff, no translations. Right, the positions of the staff lines relative to each other are not yet computed, so each extent is relative to its own staff. Cheers, Joe ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Wed, 24 Mar 2010 16:08:51 -0400, Boris Shingarov wrote: What happens in reality, is that pure_height() of the VerticalAxisGroup, > calling ly:hara-kiri-group-spanner::y-extent in Scheme land, > calling back Hara_kiri_group_spanner::pure_height(), ultimately > ends up in Axis_group_interface::cached_pure_height(). > That function, unite()'s the begin_of_line_pure_height() and > the rest_of_line_pure_height(), which are simply measured from > the middle line of the staff, no translations. And this is not a caching bug: it doesn't go away if I comment out the first three lines of Axis_group_interface::relative_pure_height(). ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
The skylines are used as part of VerticalAlignment, to figure out how > far apart the children of VerticalAlignment (ie. VerticalAxisGroups for > staves, lyrics, etc) need to be. Once we compute the translations of the > children of VerticalAlignment, we can compute the height of > VerticalAlignment (and thus the height of System). I understand that this is the general idea how it is supposed to be. What happens in reality, is that pure_height() of the VerticalAxisGroup, calling ly:hara-kiri-group-spanner::y-extent in Scheme land, calling back Hara_kiri_group_spanner::pure_height(), ultimately ends up in Axis_group_interface::cached_pure_height(). That function, unite()'s the begin_of_line_pure_height() and the rest_of_line_pure_height(), which are simply measured from the middle line of the staff, no translations. This is the exact point where I am most confused -- what is the supposed result of these translations? So far in the debugger I only see values relative to the middle line; what is the idea what these translations are supposed to be? ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Tue, 2010-03-23 at 22:51 -0400, Boris Shingarov wrote: > > The estimated height for the whole system _should_ take into account the > > fact that the lyrics can be squeezed between the systems. Have a look at > > the comment in align-interface.cc:104 (which should get called as a > > result of sys->pure_height()). The begin_of_line_extent should be zero > > for lyrics, which should allow the lines to be close together in the > > height-estimation procedure. > > Ok, I just got one step forward in the dichotomy of where the bug is. > The begin_of_line_extent is zero. Moreover, the problem is NOT > specific to lyrics, and is observed also if there are low notes. So, I am > concentrating my attention on what's happening in the callers of > get_skylines(). (Need to understand now why these skylines are > part of "translation" and not "height"). The skylines are used as part of VerticalAlignment, to figure out how far apart the children of VerticalAlignment (ie. VerticalAxisGroups for staves, lyrics, etc) need to be. Once we compute the translations of the children of VerticalAlignment, we can compute the height of VerticalAlignment (and thus the height of System). Cheers, Joe ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Tue, 2010-03-23 at 19:57 -0400, Boris Shingarov wrote: > > The estimated height for the whole system _should_ take into account the > > fact that the lyrics can be squeezed between the systems. Have a look at > > the comment in align-interface.cc:104 (which should get called as a > > result of sys->pure_height()) > > Trying to make head or tail of which child is which, and why lyrics do > increase the pure_height by much, I am now thouroughly confused by > something else. The height is calculated by unite()'ing the heights > of all items and all spanners. I started with a trivial example with > some repeating notes, and no lyrics and nothing else. There is an > item amounting to [-0.9:0.3], and a spanner amounting to [-7:0]. With the attached .gdbinit file, you can do ps grob->self_scm_ to see the type of a grob, or pg grob to see its type and its properties. That may help you get a better understanding of what is going on. > Ok, I step inside the calculation of pure_height for it, and find another > (one) spanner. Putting a breakpoint inside the for() loop for the items, I > never hit it. I'd appreciate a three-liner briefly explaining the design of > this. (The "pure" vs "non-pure" height also seems of mysterious design). Until the line-breaking is computed, we can't calculate the horizontal positioning of any grob. pure-height is an _estimate_ of the real height of the grob ("pure" because it can be computed without causing side-effects, whereas the actual height usually can't). Cheers, Joe def ps call ly_display_scm($arg0) end def pg ps $arg0->self_scm_ ps $arg0->immutable_property_alist_ ps $arg0->mutable_property_alist_ end def pgv pg $arg0 ps $arg0->object_alist_ end___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
The estimated height for the whole system _should_ take into account the > fact that the lyrics can be squeezed between the systems. Have a look at > the comment in align-interface.cc:104 (which should get called as a > result of sys->pure_height()). The begin_of_line_extent should be zero > for lyrics, which should allow the lines to be close together in the > height-estimation procedure. Ok, I just got one step forward in the dichotomy of where the bug is. The begin_of_line_extent is zero. Moreover, the problem is NOT specific to lyrics, and is observed also if there are low notes. So, I am concentrating my attention on what's happening in the callers of get_skylines(). (Need to understand now why these skylines are part of "translation" and not "height"). ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
> The estimated height for the whole system _should_ take into account the > fact that the lyrics can be squeezed between the systems. Have a look at > the comment in align-interface.cc:104 (which should get called as a > result of sys->pure_height()) Trying to make head or tail of which child is which, and why lyrics do increase the pure_height by much, I am now thouroughly confused by something else. The height is calculated by unite()'ing the heights of all items and all spanners. I started with a trivial example with some repeating notes, and no lyrics and nothing else. There is an item amounting to [-0.9:0.3], and a spanner amounting to [-7:0]. Ok, I step inside the calculation of pure_height for it, and find another (one) spanner. Putting a breakpoint inside the for() loop for the items, I never hit it. I'd appreciate a three-liner briefly explaining the design of this. (The "pure" vs "non-pure" height also seems of mysterious design). ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Fri, 19 Mar 2010 17:15:08 -0400, Boris Shingarov wrote: > As to the call to get_skylines(), now I am really confused. All this > (get_property("springs-and-rods") resulting in > Spacing_spanner::set_springs() and all the other things down the road > being called), happens much earlier than thewhole page layout stage -- > I guess the Y-extent property is stored inside the grob at this earlier > stage? Ok, now I understand what's going on here: the get_skylines() does get called much earlier in the game, but it also is called during Constrained_breaking::fill_line_details(), the Y-extent function of the system is the one from Axis_group_interface; that iterates over all spanners calculating their pure_height(), that goes through another layer of Axis_group_interface, and that one finaly reaches the Align_interface. What this tree of grobs looks like, and what is each grob, I am not sure how to tell. ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
On Fri, 19 Mar 2010 13:39:36 -0700, Joe Neeman wrote: The begin_of_line_extent should be zero > for lyrics, which should allow the lines to be close together in the > height-estimation procedure. The difficulty of debugging this, is how do I tell which grob is which. They are Scheme pointers, so it is not easy to tell if the current current iteration of the "for" cycle is the one about the lyrics grob. Boris ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
Hi Joe, Thanks for the tips, and have a nice and safe trip! > The estimated height for the whole system _should_ take into account the > fact that the lyrics can be squeezed between the systems. Have a look at > the comment in align-interface.cc:104 (which should get called as a > result of sys->pure_height()) Apparently, the estimated height does not take it into account. As to the call to get_skylines(), now I am really confused. All this (get_property("springs-and-rods") resulting in Spacing_spanner::set_springs() and all the other things down the road being called), happens much earlier than thewhole page layout stage -- I guess the Y-extent property is stored inside the grob at this earlier stage? > The begin_of_line_extent should be zero > for lyrics, which should allow the lines to be close together in the > height-estimation procedure. It looks to me like the correction based on begin_of_line_extent, is only about the case when the whole previous line is lyrics? I'll step through get_skylines() in the debugger right now to see what's going on. Boris ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
Unfortunately, I'm about to go out of town for the weekend, so I don't have time right now to follow this up properly. But I've left a couple of hints below. On Fri, 2010-03-19 at 11:05 -0400, Boris Shingarov wrote: > On Wed, 13 Jan 2010 19:37:14 1100, Joe Neeman wrote: > > > It's nice to see someone else touching the page-breaking code > > The more I dig into it, the more questions I have. > Back in January, we talked about vertical space estimation in the case of > Prob (i.e. markup); now I am investigating vertical space estimation for the > case of Grob (i.e. score). The problem is that when the user tries to fit > more systems on the page by decreasing the spacing/padding, the final page > layout does take it into account, but it has no effect on the estimations at > the page-breaking stage. The visual result is that the lines do get closer > together, but the number of lines per page still remains the same, leaving a > big empty gap at the bottom of the page. > > This problem is caused by the calculation of ext_len in the "for (i=...)" > cycle in Page_breaking::min_page_count(). > For illustration, I have attached two examples: simple-nolyrics.ly, and the > same score with added lyrics, simple-lyrics.ly. When there are no lyrics, > the estimation works as expected. The ext_len in this case is 7.947, and the > specified padding of 1.2 causes the systems to be nicely squeezed together. > Now if we add lyrics, the final spacing and the estimation start to diverge > dramatically. In the final layout of the page, the lyrics do not cause a > significant change to the distance between the systems: the lyrics just sit > in the gap which was already there. But the estimator, during the page > breaking, get confused: for each line, the rod is lengthened by ext_len > padding, and ext_len in this case is as much as 10.203. So the page breaker > thinks that the systems are much taller than they will finally prove to be > when the final spacing is done. The estimated height for the whole system _should_ take into account the fact that the lyrics can be squeezed between the systems. Have a look at the comment in align-interface.cc:104 (which should get called as a result of sys->pure_height()). The begin_of_line_extent should be zero for lyrics, which should allow the lines to be close together in the height-estimation procedure. > Trying to do something about this ext_len problem, I traced the origin of > the ext-len back to the Y-ext of the Grob (fill_line_details() calls > sys->pure_height(), which calls get_property_data("Y-extent")). Bien sûr, > the Y-extent includes the whole height of the lyrics. > > I am not sure what can be done to reconcile the estimator with the actual > spacing, so that users can get tight spacing. My understanding is that the > final spacing actually tries to avoid skyline collisions; the page breaker > ignores the actual shape of the skyline and treats the systems as solid > rectangular blocks. So the only possible workarond would be to manually > indicate the ext_len to the page breaker (something like, a block variable in > the score layout) -- extremely ugly, and I would only opt for that if I knew > for sure that all other options were exhausted. There is a workaround, but it's hacky: do something like \paper { page-breaking-between-system-spacing #'padding = #'-5.0 } The page-breaking-between-system-spacing variable was added exactly for this situation, so that you can override the page breaker's estimations in case they're off. But you only get to plug in a single value for the whole \book. Cheers, Joe ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond
Re: Lyrics break estimation of vertical spacing
Here are the two illustrative examples. On Fri, 19 Mar 2010 11:05:30 -0400, Boris Shingarov wrote: On Wed, 13 Jan 2010 19:37:14 1100, Joe Neeman wrote: > > > It's nice to see someone else touching the page-breaking code > > The more I dig into it, the more questions I have. > Back in January, we talked about vertical space estimation in the case of Prob (i.e. markup); now I am investigating vertical space estimation for the case of Grob (i.e. score). The problem is that when the user tries to fit more systems on the page by decreasing the spacing/padding, the final page layout does take it into account, but it has no effect on the estimations at the page-breaking stage. The visual result is that the lines do get closer together, but the number of lines per page still remains the same, leaving a big empty gap at the bottom of the page. > > This problem is caused by the calculation of ext_len in the "for (i=...)" cycle in Page_breaking::min_page_count(). > For illustration, I have attached two examples: simple-nolyrics.ly, and the same score with added lyrics, simple-lyrics.ly. When there are no lyrics, the estimation works as expected. The ext_len in this case is 7.947, and the specified padding of 1.2 causes the systems to be nicely squeezed together. Now if we add lyrics, the final spacing and the estimation start to diverge dramatically. In the final layout of the page, the lyrics do not cause a significant change to the distance between the systems: the lyrics just sit in the gap which was already there. But the estimator, during the page breaking, get confused: for each line, the rod is lengthened by ext_len padding, and ext_len in this case is as much as 10.203. So the page breaker thinks that the systems are much taller than they will finally prove to be when the final spacing is done. > > Trying to do something about this ext_len problem, I traced the origin of the ext-len back to the Y-ext of the Grob (fill_line_details() calls sys->pure_height(), which calls get_property_data("Y-extent")). Bien sûr, the Y-extent includes the whole height of the lyrics. > > I am not sure what can be done to reconcile the estimator with the actual spacing, so that users can get tight spacing. My understanding is that the final spacing actually tries to avoid skyline collisions; the page breaker ignores the actual shape of the skyline and treats the systems as solid rectangular blocks. So the only possible workarond would be to manually indicate the ext_len to the page breaker (something like, a block variable in the score layout) -- extremely ugly, and I would only opt for that if I knew for sure that all other options were exhausted. > > Boris > > > > > > ___ > bug-lilypond mailing list > bug-lilypond@gnu.org > http://lists.gnu.org/mailman/listinfo/bug-lilypond > > \version "2.12.2" \paper { #(set-paper-size "b5") indent = 0.0 tagline = ##f between-system-spacing = #'((space . 1.20) (padding . 1.20) (minimum-distance . 1.20)) ragged-bottom=##t ragged-last-bottom=##t %min-systems-per-page=12 } \book { \score { << \new Voice = "cantus" { f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' } >> } %score } %book \version "2.12.2" \paper { #(set-paper-size "b5") indent = 0.0 tagline = ##f between-system-spacing = #'((space . 1.20) (padding . 1.20) (minimum-distance . 1.20)) ragged-bottom=##t ragged-last-bottom=##t %min-systems-per-page=12 } \book { \score { << \new Voice = "cantus" { f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g' f' g' a' b' a' b' f' g
Lyrics break estimation of vertical spacing
On Wed, 13 Jan 2010 19:37:14 1100, Joe Neeman wrote: > It's nice to see someone else touching the page-breaking code The more I dig into it, the more questions I have. Back in January, we talked about vertical space estimation in the case of Prob (i.e. markup); now I am investigating vertical space estimation for the case of Grob (i.e. score). The problem is that when the user tries to fit more systems on the page by decreasing the spacing/padding, the final page layout does take it into account, but it has no effect on the estimations at the page-breaking stage. The visual result is that the lines do get closer together, but the number of lines per page still remains the same, leaving a big empty gap at the bottom of the page. This problem is caused by the calculation of ext_len in the "for (i=...)" cycle in Page_breaking::min_page_count(). For illustration, I have attached two examples: simple-nolyrics.ly, and the same score with added lyrics, simple-lyrics.ly. When there are no lyrics, the estimation works as expected. The ext_len in this case is 7.947, and the specified padding of 1.2 causes the systems to be nicely squeezed together. Now if we add lyrics, the final spacing and the estimation start to diverge dramatically. In the final layout of the page, the lyrics do not cause a significant change to the distance between the systems: the lyrics just sit in the gap which was already there. But the estimator, during the page breaking, get confused: for each line, the rod is lengthened by ext_len padding, and ext_len in this case is as much as 10.203. So the page breaker thinks that the systems are much taller than they will finally prove to be when the final spacing is done. Trying to do something about this ext_len problem, I traced the origin of the ext-len back to the Y-ext of the Grob (fill_line_details() calls sys->pure_height(), which calls get_property_data("Y-extent")). Bien sûr, the Y-extent includes the whole height of the lyrics. I am not sure what can be done to reconcile the estimator with the actual spacing, so that users can get tight spacing. My understanding is that the final spacing actually tries to avoid skyline collisions; the page breaker ignores the actual shape of the skyline and treats the systems as solid rectangular blocks. So the only possible workarond would be to manually indicate the ext_len to the page breaker (something like, a block variable in the score layout) -- extremely ugly, and I would only opt for that if I knew for sure that all other options were exhausted. Boris ___ bug-lilypond mailing list bug-lilypond@gnu.org http://lists.gnu.org/mailman/listinfo/bug-lilypond