https://codereview.appspot.com/325470043/diff/20001/lily/lyric-hyphen.cc File lily/lyric-hyphen.cc (right):
https://codereview.appspot.com/325470043/diff/20001/lily/lyric-hyphen.cc#newcode125 lily/lyric-hyphen.cc:125: Box b (Interval (0, dash_length), Interval (h, h + th)); On 2017/09/16 19:53:56, dak wrote:
It seems wasteful to put these in the loop, particularly since
dash_mol is then
additionally copied before shifting. Maybe pull if
(Text_interface::is_markup
...) outside of the loop and have a separate loop for each branch?
There is no
shared loop code outside of if/else in the current implementation
anyway. Well, you are right. https://codereview.appspot.com/325470043/diff/20001/lily/lyric-hyphen.cc#newcode126 lily/lyric-hyphen.cc:126: Stencil dash_mol (Lookup::round_filled_box (b, 0.8 * lt)); Shouldn't the hardcoded 0.8 be replaced by a property? https://codereview.appspot.com/325470043/diff/20001/scm/define-grobs.scm File scm/define-grobs.scm (right): https://codereview.appspot.com/325470043/diff/20001/scm/define-grobs.scm#newcode1403 scm/define-grobs.scm:1403: (text . '()) On 2017/09/16 19:53:56, dak wrote:
That's the default when unset. For properties that may optionally be
set, in
particular when this default formally does not match the predicate, I
think we
tend not to explicitly specify it.
Well, omitting it here means that there is no indication in the internals reference that LyricHyphen uses that property. https://codereview.appspot.com/325470043/ _______________________________________________ lilypond-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/lilypond-devel
