Re: shortened stems and flags (issue4134041)

[address@hidden]

On Feb 6, 2011, at 6:19 PM, Jan Warchoł wrote:

> 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!

I just looked over your code again - the bit I'm talking about doesn't really 
deal w/ intervals.

Try:

Real factors[] = {1,2,3,4,5,basic_length / incraments[log]};
string numbers[] = {"1","2","3","4","5","6"};
for (int i = 0; i < 6; i++)
  if (length > (basic_length - factors[i] * increments[log]))
  {
    font_char += numbers[i];
    break;
  }

I'm not sure if the above is 100% correct, but it or something like it should 
get the job done!

Cheers,
MS