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#newcode619
lily/stem.cc:619: static vector<Real> available_flag_lengths[2][5];
On 2011/04/18 03:42:53, hanwenn wrote:
You cannot do this.
This will screw up if two .ly files use fonts of different design
sizes: the
settings of the first file will leak into the 2nd.
Hmm, i'm not sure what you mean. This:
\relative c'' {
\new CueVoice {
\voiceOne \autoBeamOff
a8 b c d e f g a
a,16 b c d e f g a
}
\new Voice {
\voiceOne \autoBeamOff
a,16 b c d e f g a
a,8 b c d e f g a
}
}
uses (i think) two different design sizes (one for cues, one for normal
notes) and yet everything is fine - change from regular flags to
shortened flags take place on note d both in cues and normal notes.
Also when i compile this and a copy of this file with
#(set-global-staff-size 15) the results are correct.
Perhaps i'm not understanding something?
http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode620
lily/stem.cc:620:
On 2011/04/17 10:23:59, MikeSol wrote:
It seems like this new stuff should be declared in stem.hh instead of
here.
Judging by pitch.cc and pitch.hh example I thought that deleting this
line in stem.cc and adding
private:
static vector<Real> available_flag_lengths[2][5];
before closing brace in stem.hh would do this, but make says
/home/janek/lilypond-git/lily/stem.cc:674: error:
'available_flag_lengths' was not declared in this scope
changing 'available_flag_lengths' into 'Stem::available_flag_lengths'
didn't make it work...
How this should be done?
http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode637
lily/stem.cc:637: already_computed = 1;
On 2011/04/17 10:23:59, MikeSol wrote:
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.
Ok, i'll add a comment.
http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode650
lily/stem.cc:650: /* examine available standard flags glyphs.
On 2011/04/18 03:42:53, hanwenn wrote:
this seems awfully kludgy. Can't we just export another list of
dimension
variables directly in the font? See gen-emmentaler-script.py and
mf/out/*table*
From what i understand gen-emmentaler-scripts.py it extracts some
variables, like glyph size, from mf files. That's quite not what we need
to do - the information that we need must be specified by font-designer
manually and separately from actual flag variables. (these numbers do
not appear in flag code mf variables nor can be calculated from them.)
Should i explain this in more detail?
http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode660
lily/stem.cc:660: glyph_name[8] !=NULL )
On 2011/04/17 10:23:59, MikeSol wrote:
isdigit (glyph_name[7])
You are referring to whitespace?
I was desperately trying to fit in 80 characters line width :)
Fixed now.
http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode669
lily/stem.cc:669:
On 2011/04/17 10:23:59, MikeSol wrote:
substr (7,1)
Done.
http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode673
lily/stem.cc:673: = ::atof(glyph_name.substr(9,string::npos).c_str());
On 2011/04/17 10:23:59, MikeSol wrote:
::atof (glyph_name.substr (0, string::npos).c_str ())
Done.
http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode757
lily/stem.cc:757: */
On 2011/04/17 10:23:59, MikeSol wrote:
How won't the current patch apply to user-overriden lengths?
I meant that compiling { \autoBeamOff f'8 \override Stem #'length = #5
f'8 } should result in the second flag being a shorter variant to better
fit shorter stem.
I suppose implementing this would mean changing the order in which
things are currently done in Lily?
Sounds like a nightmare...
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);
On 2011/04/17 10:23:59, MikeSol wrote:
get_property ("font-size")
Done.
http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode777
lily/stem.cc:777: - stem_length/2)
On 2011/04/17 10:23:59, MikeSol wrote:
while (abs (available_flag_lengths [dir == 'd' ? 1 : 0][log -
3][flag_variant_number]
Done.
http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode777
lily/stem.cc:777: - stem_length/2)
On 2011/04/17 10:23:59, MikeSol wrote:
stem_length / 2
Done.
http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode777
lily/stem.cc:777: - stem_length/2)
On 2011/04/17 10:23:59, MikeSol wrote:
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.
ok
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]
On 2011/04/17 10:23:59, MikeSol wrote:
same as above
Done.
http://codereview.appspot.com/4410049/diff/1/lily/stem.cc#newcode781
lily/stem.cc:781: flag_variant_number++;
On 2011/04/17 10:23:59, MikeSol wrote:
same as above
Done.
http://codereview.appspot.com/4410049/
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
