Re: Gets vertical skylines from grob stencils (issue 5626052)

[Joe Neeman]

On Tue, Feb 14, 2012 at 1:55 AM, address@hidden <address@hidden> wrote:

On Feb 14, 2012, at 9:44 AM, Joe Neeman wrote:

On Mon, Feb 13, 2012 at 4:48 AM, address@hidden <address@hidden> wrote:

On Feb 13, 2012, at 5:41 AM, Joe Neeman wrote:

On Thu, Feb 9, 2012 at 9:50 AM, address@hidden <address@hidden> wrote:

On Feb 7, 2012, at 6:47 PM, address@hidden wrote:

I did some experiments with caching that are up on:
dev/skylines-cached

Hey all,

Fresh branch up at dev/skylines-cached.  This patch should only increase compilation time of a LilyPond score by 1-2 seconds for every minute.

 

Wow, that's quite some work you've done. I haven't read it carefully yet, but here are some broad suggestions/comments:

You can thank inclement weather and late trains!

Regarding your comment in axis-group-interface.cc:802, if it gets called twice, there's a bug (usually caused because we've asked for the extent of a cross-staff grob before line-breaking has happened). I'm fine with trying to detect the bug and continue, but I think there should be a programming_error (at least for NDEBUG builds; see the way cyclic dependencies are reported in grob-property.cc).

The reason the function is called twice is because it'd be called through Axis_group_interface::calc_skylines and then again via Hara_kiri_group_spanner::calc_skylines, potentially before one of these finishes to execute.  It doesn't seem like this could be related to a line breaking problem.

In my experience, it usually is. What happens is that calc_skylines asks for the y-extent of some cross-staff grob. In order for that grob to find its extent, it needs to know how far apart the staves are, which depends on the height of the axis-group, and so you have a cyclic dependency.

Actually, before I respond fully to your e-mail, I just want to make sure we're talking about the same comment.  I'm looking at "Finding extents may have trigged another skyline lookup, which will have done the shifting.  In this case, we simply add the boxes."

Is that it?

Actually, I was referring to a comment that has since disappeared:

 

-        because this loop results in the looking up of extents, it may trigger

-        the calling of this function a second time before the first one finishes

-        to execute.  thus, the outside-staff-priority may be set to false.  that

-        is why this check is necessary.

but actually I preferred your older code (plus a programming_error) to the current one. A cyclic dependency is a bug, so better to catch it early than to allow it and try to correct afterwards.

Cheers,
Joe