On Feb 6, 2011, at 6:19 PM, Jan Warchoł wrote:
> 2011/2/6 <mts...@gmail.com>:
>> 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
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
