lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: shortened flags: choosing appropriate flag (issue4410049)


From: mtsolo
Subject: Re: shortened flags: choosing appropriate flag (issue4410049)
Date: Sun, 17 Apr 2011 10:23:59 +0000

Please add a stemful regtest with several overrides just to make sure
that this works completely.


http://codereview.appspot.com/4410049/diff/1/lily/stem.cc
File lily/stem.cc (right):

http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode620
lily/stem.cc:620:
It seems like this new stuff should be declared in stem.hh instead of
here.

http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode637
lily/stem.cc:637: already_computed = 1;
In general, this seems to be hardcoding a lot of aspects of the names of
fonts.  This is not problematic so long as these names don't change, but
perhaps it'd be wise to add a comment in the metafont file warning
people to revamp this code if they ever change the name of fonts.

http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode660
lily/stem.cc:660: glyph_name[8] !=NULL )
isdigit (glyph_name[7])

http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode669
lily/stem.cc:669:
substr (7,1)

http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode673
lily/stem.cc:673: = ::atof(glyph_name.substr(9,string::npos).c_str());
::atof (glyph_name.substr (0, string::npos).c_str ())

http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode757
lily/stem.cc:757: */
How won't the current patch apply to user-overriden lengths?

http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode768
lily/stem.cc:768: = pow (2, (robust_scm2double
(me->get_property("font-size"), 0.0)) / 6);
get_property ("font-size")

http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode777
lily/stem.cc:777: - stem_length/2)
while (abs (available_flag_lengths [dir == 'd' ? 1 : 0][log -
3][flag_variant_number]

http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode777
lily/stem.cc:777: - stem_length/2)
stem_length / 2

http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode777
lily/stem.cc:777: - stem_length/2)
Hardcoding the use of duration log in arrays and vectors happens a
couple times in the source - I'd say keep it here, but this type of
coding should be revisited post 2.14 in case, for whatever reason,
duration log changes one day.

http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode779
lily/stem.cc:779: abs (available_flag_lengths
[(dir=='d')?1:0][log-3][flag_variant_number+1]
same as above

http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode781
lily/stem.cc:781: flag_variant_number++;
same as above

http://codereview.appspot.com/4410049/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]