Re: shortened stems and flags (issue4134041)

[Jan Warchoł]

2011/2/6  <address@hidden>:
> Great work!

Thank you!

> I can't really speak to the font stuff.

Why not? Every one has his own personal preferences.
And the font stuff is what matters the most for me :)
Of course some changes are very subtle. It's best to open pdfs called
"flag testing" side-by-side on a big resolution screen (i have 24",
1920x1080 screen), zoomed to 300-400%, but it can be done on smaller
displays too.
Here are the pdfs made with my commit: http://www.sendspace.com/file/j9dq5t
Here are pdfs made with 2.13.47 for comparison:
http://www.sendspace.com/file/ogl8rk
Here are the .ly files: http://www.sendspace.com/file/gjh6ng

> You can make your C++ code a
> lot shorter by using arrays.

I've made it shorter in some places by using grob properties, but i
was unable to upload that new patch. It should be available soon.

> And I see that you've wiped a lot of the
> tabbing in the original code.  You may want to start by copying a fresh
> version of the file into your git repository and then copying and
> pasting the new code into that to save you the headache of dealing w/
> all of those tabs.

Frankly, i really do not know what is happening here. Files before and
after the change look the same to me.
I'm using lilydev, but the default text editor wasn't to my taste, so
i edited the files outside lilydev, in Notepad++. I set Notepad++ to
write all tabs as spaces (if i remember correctly, our policy for the
code says that all indentation should be done with spaces?) and it
looked ok...

> Other than that, arrays are your friend.
> Everything else looks good!
>
> http://codereview.appspot.com/4134041/diff/1/lily/stem.cc
> File lily/stem.cc (right):
>
> http://codereview.appspot.com/4134041/diff/1/lily/stem.cc#newcode92
> lily/stem.cc:92: Staff_symbol_referencer::get_position (e[UP]));
> Try to keep the same indentation from the previous file (here, there
> were 2 tabs).
>
> http://codereview.appspot.com/4134041/diff/1/lily/stem.cc#newcode190
> lily/stem.cc:190: while (flip (&d) != DOWN);
> Ditto.
>
> http://codereview.appspot.com/4134041/diff/1/lily/stem.cc#newcode231
> lily/stem.cc:231: }
> Ditto.
>
> http://codereview.appspot.com/4134041/diff/1/lily/stem.cc#newcode246
> lily/stem.cc:246: {
> Ditto.  I'll stop "dittoing," but check out the side by side diff to
> rectify this :)

As i've said, i fail to see how it was different :(
there is the same amount of whitespace, and when i copy a fragment of
the file to some external editor, it behaves all the same (like if all
indentation was done with spaces).
If it weren't for these tiny » in side-by-side diff, i wouldn't know
what you are talking about :(

> http://codereview.appspot.com/4134041/diff/1/lily/stem.cc#newcode350
> lily/stem.cc:350: Direction dir = get_grob_direction (me);
> Try:
> Real potential_lengths[] = {7,7,7,7,8.5,10,11.5};
> length = potential_lengths[durlog-1];

This one i improved with grob properties.

> http://codereview.appspot.com/4134041/diff/1/lily/stem.cc#newcode373
> lily/stem.cc:373: }
> Same approach as above - you can use an array here.
>
> http://codereview.appspot.com/4134041/diff/1/lily/stem.cc#newcode388
> lily/stem.cc:388: length -= shorten;
> You can also use an array here.
>
> Real potential_shortenings[] = {0.833,0.666,0.555,0.333,0.166};
> shorten *= potential_shortenings[(int)(head_positions (me)[dir])];

It didn't work, i got stems with length 0 and general mess :(
i'll investigate tomorrow.

> http://codereview.appspot.com/4134041/diff/1/lily/stem.cc#newcode715
> lily/stem.cc:715: }
> Ditto - use arrays.
>
> http://codereview.appspot.com/4134041/diff/1/lily/stem.cc#newcode728
> lily/stem.cc:728: }
> Same - arrays can be used here (see above for how to handle else if).
>
> http://codereview.appspot.com/4134041/

Thanks for help!