On Mar 5, 2011, at 9:26 AM, Joe Neeman wrote:
On Sat, Mar 5, 2011 at 11:57 PM, Mike Solomon <address@hidden>
On Mar 5, 2011, at 7:19 AM, Joe Neeman wrote:
On Sat, Mar 5, 2011 at 10:18 PM, Mike Solomon <address@hidden>
After a bit of back and forth w/ Han Wen, I have drummed up a way to split this up such that it can be part of LilyPond in two phases. It follows his suggestion to push all of the non-balloon-related stuff first, and to push that second. It will set the stencil property of FootnoteItem and FootnoteSpanner to #f in define-grobs.scm. This means that whatever people write for the annotation part of the footnote will be swallowed up and not used, whereas the note part on the bottom of the page will be printed.
If anyone has arguments for pushing this whole thing in its entirety, let me know. I am in favor of that for the reasons I stated in my previous e-mail, but I also realize that big chunks of code pushed all at once can make bug tracking interminable.
If not, the plan is to push the non-balloon-related stuff today, let it simmer for a few days, and then push the balloon related stuff used for the in-document annotations. I will make a nice long commit message detailing this so that, hopefully, experimental users don't waste hours figuring out what happened to their non-appearing annotations. Alternatively, if anyone feels that I should split this up but that I am splitting it up the wrong way, please let me know.
That sounds fine to me. Do you have a patch showing the part you are going to push first? I was happy with the spacing-related code up until your break-visibility changes, but if you're going to include anything since then, I'd like to have another quick look (I have to catch a plane in 12 hours; if you put up your patch before then, I'll have a look right away).
Patch attached. The stuff that comes from your comments regarding break-visibility is implemented in Balloon_interface::is_visible.
I find the return value of Balloon_interface::is_visible strange (first of all, the name suggests that it should just be bool). For example, it returns BEGINNING_OF_LINE if the footnote is visible and in the middle of the line (but only if the parent has a break-visibility property). I might be missing something, but it looks to me like you can get rid of this function, and just set FootnoteItem 'break-visibility to
(let ((parent (ly:grob-parent grob X))
(par-vis (ly:grob-property parent 'break-visibility))
(parent-visibility (if (vector? par-vis) par-vis #(#t #t #t))))
(vector-ref parent-visibility 0)
(vector-ref parent-visibility 1)
(vector-ref parent-visibility 2)
(not (vector-ref parent-visibility 0))))))
(There's still a corner case here, because you need the footnote to be visible if it is in the first column of the score...)
I ultimately incorporated a suggestion from Neil. It seems more streamlined & LilyPondish. That said, it does lead to double printing for stuff like time signatures. Perhaps what you're proposing could be part of the docs? I originally thought that what you're proposing should be default behavior, but more and more, I think that it's minimally annoying just to ask the user to override break-visible for FootnoteItem, as I'm not sure what the best default behavior is given the needs of users.
I don't like the interpolation stuff for spanner-placement. Why not just restrict the valid values to -1 and 1? It seems that any other value doesn't really have user-predictable behaviour anyway...
It doesn't, but like I said in my previous e-mail, I am reticent to disafford the user the ability to place the footnote on a long spanner (i.e. on a crescendo that spans 8 lines, it may be important to indicate that on the 5th line, the crescendo could potentially cut off...I know that sounds kinda bizarre, but I want to propose the most robust solution possible, and I think that it'd be better to warn the user in the docs to set this property to -1 or 1 for predictable behavior rather than to limit them to only this.
I think you have a bug in system.cc:281, because there is no guaranteed order between the grobs whose rank is the ending column. In particular, the one that appears at the beginning of the next line might appear first, and then you will break prematurely. Maybe something like:
bool end_of_line_visible = true;
if (Spanner *s ...)
if (Item *item ...)
end_of_line_visible = (LEFT == item->break_status_dir ());
if (pos < (int)start)
if (pos > (int)end)
if (pos == (int)end && !end_of_line_visible)
Thank you for all of your help! A new patch set is attached to this e-mail: please disregard the previous one.
Please let me know what you think - I'm flying in 6ish hours, so I can make any necessary changes and/or push before then. After that my internet access will be more spotty.