lilypond-devel
[Top][All Lists]
Advanced

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

Re: Fixing issue 37 with extra position callback (issue3928041)


From: n . puttock
Subject: Re: Fixing issue 37 with extra position callback (issue3928041)
Date: Sun, 23 Jan 2011 17:46:58 +0000

Looks good, but there's still some excessive beam translation in
slur-scoring.ly.


http://codereview.appspot.com/3928041/diff/30001/input/regression/beam-collision.ly
File input/regression/beam-collision.ly (right):

http://codereview.appspot.com/3928041/diff/30001/input/regression/beam-collision.ly#newcode2
input/regression/beam-collision.ly:2: \header{
\header {

http://codereview.appspot.com/3928041/diff/30001/input/regression/beam-collision.ly#newcode3
input/regression/beam-collision.ly:3: texidoc="@cindex Beam Collisions
indent

http://codereview.appspot.com/3928041/diff/30001/input/regression/beam-collision.ly#newcode12
input/regression/beam-collision.ly:12: \time 4/4
indent

http://codereview.appspot.com/3928041/diff/30001/input/regression/beam-collision.ly#newcode13
input/regression/beam-collision.ly:13: << { s128 e [ s cis, ] } \\ { b''
[ s b ] } >> r8..
e[

etc.

http://codereview.appspot.com/3928041/diff/30001/lily/beam-collision-engraver.cc
File lily/beam-collision-engraver.cc (right):

http://codereview.appspot.com/3928041/diff/30001/lily/beam-collision-engraver.cc#newcode24
lily/beam-collision-engraver.cc:24: #include "item.hh"
resort alphabetically

http://codereview.appspot.com/3928041/diff/30001/lily/beam-collision-engraver.cc#newcode45
lily/beam-collision-engraver.cc:45: void process_acknowledged ();
remove

http://codereview.appspot.com/3928041/diff/30001/lily/beam-collision-engraver.cc#newcode52
lily/beam-collision-engraver.cc:52:
Beam_collision_engraver::stop_translation_timestep()
timestep ()

http://codereview.appspot.com/3928041/diff/30001/lily/beam-collision-engraver.cc#newcode54
lily/beam-collision-engraver.cc:54: for (vsize i=0; i <
covered_interior_grobs_.size (); i++)
i = 0 (etc.)

Why use a separate vector?  There's no difference in processing, so it
would make more sense to add all the grobs to covered_grobs_.

http://codereview.appspot.com/3928041/diff/30001/lily/beam-collision-engraver.cc#newcode70
lily/beam-collision-engraver.cc:70: in auto beaming, end beams are
signaled with their beams at a later timestep.
acknowledge manual beams only instead, then there's no need for this
hack

http://codereview.appspot.com/3928041/diff/30001/lily/beam-collision-engraver.cc#newcode96
lily/beam-collision-engraver.cc:96: active_beams_.erase
(active_beams_.begin() + j);
begin ()

http://codereview.appspot.com/3928041/diff/30001/lily/beam-collision-engraver.cc#newcode103
lily/beam-collision-engraver.cc:103:
Beam_collision_engraver::Beam_collision_engraver() {}
engraver ()

http://codereview.appspot.com/3928041/diff/30001/lily/beam-collision-engraver.cc#newcode106
lily/beam-collision-engraver.cc:106:
Beam_collision_engraver::process_acknowledged() {}
remove

http://codereview.appspot.com/3928041/diff/30001/lily/beam-collision-engraver.cc#newcode115
lily/beam-collision-engraver.cc:115:
Beam_collision_engraver::acknowledge_accidental (Grob_info i)
Since you're acknowledging noteheads, they already cache accidentals, so
you could extract them later.

http://codereview.appspot.com/3928041/diff/30001/lily/beam.cc
File lily/beam.cc (right):

http://codereview.appspot.com/3928041/diff/30001/lily/beam.cc#newcode969
lily/beam.cc:969: Stencil *stencil = unsmob_stencil
(covered_grob->get_property ("stencil"));
use

grob->is_live ()

then

grob->extent (common, axis)

instead of extracting stencil

http://codereview.appspot.com/3928041/diff/30001/lily/beam.cc#newcode974
lily/beam.cc:974: width = stencil->extent(X_AXIS)[RIGHT] -
stencil->extent(X_AXIS)[LEFT];
= extent (X_AXIS).length ()

http://codereview.appspot.com/3928041/diff/30001/lily/beam.cc#newcode979
lily/beam.cc:979: height = stencil->extent(Y_AXIS)[UP] -
stencil->extent(Y_AXIS)[DOWN];
= extent (Y_AXIS).length ()

http://codereview.appspot.com/3928041/diff/30001/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (right):

http://codereview.appspot.com/3928041/diff/30001/scm/define-grob-properties.scm#newcode972
scm/define-grob-properties.scm:972: (covered-grobs ,ly:grob-array? "Note
heads that could potentially
Grobs

http://codereview.appspot.com/3928041/diff/30001/scm/define-grobs.scm
File scm/define-grobs.scm (right):

http://codereview.appspot.com/3928041/diff/30001/scm/define-grobs.scm#newcode380
scm/define-grobs.scm:380: ly:beam::move-to-avoid-collisions
fix indent (tabs)

http://codereview.appspot.com/3928041/



reply via email to

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