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

[address@hidden]

On Feb 20, 2012, at 1:58 AM, Joe Neeman wrote:

> 
>  
> I think you're going about things in the wrong order. I think that the first 
> goal should be to make the code clear and correct. (In fact, if your code 
> were "done" but slow, you could push it to master but with skylining disabled 
> for most grobs by default. That way, people could test it and give feedback. 
> They could even enable skylining on more grobs if they didn't mind the 
> performance hit.)

This idea was kicked around the list in various vicissitudes, but it seemed 
that the majority of people favored LilyPond making the best result possible 
out of the box.

> Then you should profile and benchmark it to figure out which parts need to be 
> faster (use ./configure --enable-profiling). For example, do you know that 
> making special cases for beams and key signatures actually helps?
> 

I've only used `time' on large scores to get an idea, as with small scores it's 
difficult to tell.  I did a few runs again this morning and have gotten rid of 
the exception for key signatures (it does not help at all).  I think that beams 
help, but I'm honestly not even sure...it fluctuates a lot and I'm having 
trouble getting a clean read.  I am going to delete all of the exceptions (save 
slurs, which does help).

> Having said that, there are two places that I can think of which *might* 
> speed things up:
>  - you return a lot of vector<Box> in stencil-intergrate.cc. This might mean 
> that a lot of things get copied (not sure about this -- c++11 is supposed to 
> help here). It might speed things up if you pass around const vector<Box>& 
> instead.

I did this.  I'm not sure if it results in better performance, but it can't 
hurt & looks cleaner.

>  - the skylines you generate are a lot more complicated than they need to be. 
> For example, you approximate beams with 11 horizontal segments. If you were 
> to implement my suggestion about turning line segments into skylines, you 
> could represent a beam with 1 sloped segment.
> 

This is true.  I have a test case with 1200 hairpins (12 staves, 100 hairpins a 
piece) and this brings the compilation time down from about 19.9' to 19.5' on 
average.  So there's definitely a performance increase.  The issue I'm having 
is figuring out a good way to implement this so that it maintains a meaningful 
notion of what "horizon padding" is.  For example, if two skylines are merged 
with different horizon paddings, do they retain the horizon paddings of the old 
grobs (harder to code) or take the horizon padding of the new grob (easier to 
code)?  Or both - do they retain the old and tack on the new?  These ideas are 
currently ill defined but would need to be solidified.  I'm an advocate of the 
horizon padding of the new grob (or prob) only ever kicking in and forgetting 
the previous ones, operating under the assumption that if someone wants to 
hardcode an earlier horizon padding upstream for a given skyline, it'd be 
better to tack the padding onto the boxes that make the skyline and not the 
skyline itself.  Thoughts?

> I would encourage you to do the second part _before_ profiling, since I think 
> it would make the code cleaner. The first part, though, you definitely 
> shouldn't do unless you know that it's a problem.

Too late...I should finish reading e-mails first.  Meh.  Took me 4 minutes, no 
biggie.

> 
> By the way, could you update your git branch please?
> 

Will do once we finish this exchange & I get a clean version up.

Cheers,
MS