On Mon, Jan 24, 2011 at 9:42 AM, Mike Solomon mike...@ufl.edu wrote:
Fixed, although I have no idea what the desired output is in this case (I
have a feeling it's Hey...you asked for it...).
IMO, in the last case, the stems should be shortened so there is no
collision; why is there a collision
I had just reverted it to the old behavior (I think...).
The question is: when we have a collision like the third example, how much do
we shorten the stems before it becomes ridiculous? We could shorten the stems
to avoid the collision there, but if the note were a perfect 5th lower, it'd
On Mon, Jan 24, 2011 at 10:49 AM, Mike Solomon mike...@ufl.edu wrote:
I had just reverted it to the old behavior (I think...).
The question is: when we have a collision like the third example, how much do
we shorten the stems before it becomes ridiculous? We could shorten the
stems to
On Mon, Jan 24, 2011 at 11:22 AM, Han-Wen Nienhuys hanw...@gmail.com wrote:
Let me whip up a prototype for you to see how it works.
See attached (it applies on top of issue3928041_55001.diff)
It is imperfect in several ways,
- see TODOs in the code.
- 4th beat 1st line: should go below
- 5th
On 24 January 2011 14:28, Han-Wen Nienhuys hanw...@gmail.com wrote:
See attached (it applies on top of issue3928041_55001.diff)
+ common[a] = common_refpoint_of_array (covered_grobs, me, Axis (a));
I suspect this line needs changing, otherwise it junks the calculated
refpoint for the
On Mon, Jan 24, 2011 at 9:40 PM, Neil Puttock n.putt...@gmail.com wrote:
+ common[a] = common_refpoint_of_array (covered_grobs, me, Axis (a));
I suspect this line needs changing, otherwise it junks the calculated
refpoint for the stems (which means we no longer get the correct
refpoint
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):
I noticed another nitpick.
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#newcode1
input/regression/beam-collision.ly:1:
I made a lot of changes to get things looking more lilypond-y.
I also now have it better handling scenarios where the subdivisions are
wacky. Check out:
\relative c' { { b32 [ c,16 c'8 d,64 e'8. c,32 e'' ] } \\ { f8 [ c32
d'32 bes,,64 c'64 c64 c'128 ] } }
The low D is snug as a bug in a
New patch set uploaded.
http://codereview.appspot.com/3928041/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Sorry, I got the author incorrect.
Thanks for looking into this, Mike!
http://codereview.appspot.com/3928041/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Hi Carl,
thanks for diving into this!
I love the idea of handling beam collisions, but I think the design of
the backend code is flawed. See my comments.
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
File lily/beam.cc (right):
http://codereview.appspot.com/3928041/diff/30001/lily/beam.cc#newcode1014
lily/beam.cc:1014: pos[RIGHT] += offset;
also, these offsets disregard carefully computed beam quantizations.
Even though these are
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#newcode1015
lily/beam.cc:1015: return ly_interval2scm (pos);
On 2011/01/23 18:05:39, hanwenn wrote:
it looks like this only handles the 1st
On Mon, Jan 24, 2011 at 12:58 AM, mts...@gmail.com wrote:
acknowledge manual beams only instead, then there's no need for this
hack
OK - but as far as I know, there is no way to have DECLARE_ACKNOWLEDGER
(manual_beam) . What would be the appropriate way to do this?
Look at the cause of
Mike, I tried Chopin preludes Opus 28 No. 1 (where I thought there might
be trouble because beams need to cross stems there) and No. 2 (the
classic example that benefits from this fix).
These, and the large score + parts I use for informal checking, look
good.
Hope you can keep weaving it in to
Regtest comparison looks fine, builds the docs from scratch. Let's push
it in 24 hours if there's no complaints.
http://codereview.appspot.com/3928041/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
Reviewers: MikeSol,
Message:
Looks great, Mike!
You have some code indentation issues that don't match our style. Other
than that, I think it's good to go.
Of course, we do need a regression test as well.
Thanks,
Carl
Carl,
Thanks for the comments!
I've integrated all but one of them - when you say Also add to TabStaff, what
do you mean? I was under the impression that TabStaff would inherit this
engraver because it inherits from Staff.
Cheers,
MS
On Jan 8, 2011, at 1:38 PM, carl.d.soren...@gmail.com
Carl,
Thanks for the comments!
I've integrated all but one of them - when you say Also add to TabStaff, what
do you mean? I was under the impression that TabStaff would inherit this
engraver because it inherits from Staff.
Cheers,
MS
On Jan 8, 2011, at 1:38 PM, carl.d.soren...@gmail.com
All is fixed. Attached is the original patch plus one with Carl's suggested
formatting changes.
I ran all of the regtests and nothing breaks.
I'd like one other person to test these patches out w/ the attached .ly
document to confirm. Then, if there are no other other comments, please push
Mike Solomon mike...@ufl.edu writes:
All is fixed. Attached is the original patch plus one with Carl's
suggested formatting changes.
I ran all of the regtests and nothing breaks.
I'd like one other person to test these patches out w/ the attached
.ly document to confirm. Then, if there
On 8 January 2011 19:48, Mike Solomon mike...@ufl.edu wrote:
I ran all of the regtests and nothing breaks.
This is not good enough.
You *must* do a proper regression test comparison.
Withough doing make check, I can already see it will fail since you
haven't documented covering-note-heads in
On 1/8/11 12:48 PM, Mike Solomon mike...@ufl.edu wrote:
All is fixed. Attached is the original patch plus one with Carl's suggested
formatting changes.
Latest patch is posted on Rietveld, as
http://codereview.appspot.com/3928041/
Thanks,
Carl
On 1/8/11 12:51 PM, David Kastrup d...@gnu.org wrote:
Mike Solomon mike...@ufl.edu writes:
All is fixed. Attached is the original patch plus one with Carl's
suggested formatting changes.
I ran all of the regtests and nothing breaks.
I'd like one other person to test these patches
On 1/8/11 12:52 PM, Neil Puttock n.putt...@gmail.com wrote:
On 8 January 2011 19:48, Mike Solomon mike...@ufl.edu wrote:
I ran all of the regtests and nothing breaks.
This is not good enough.
You *must* do a proper regression test comparison.
As defined in the CG, this means:
Run
On 1/8/11 12:48 PM, Mike Solomon mike...@ufl.edu wrote:
All is fixed. Attached is the original patch plus one with Carl's suggested
formatting changes.
I ran all of the regtests and nothing breaks.
I'd like one other person to test these patches out w/ the attached .ly
document to
Mike,
I like it on small scores.
I let your patch compile a symphony and got several unneeded stem
lengthenings per page. I started carving out a reasonable-sized .ly to
send for you to test, but that might actually take longer than you
recognizing the problem from the output. Three images
Hey Keith,
I believe the new patch set fixes that problem.
Cheers,
MS
On Jan 8, 2011, at 9:01 PM, Keith OHara wrote:
Mike,
I like it on small scores.
I let your patch compile a symphony and got several unneeded stem
lengthenings per page. I started carving out a reasonable-sized .ly to
Latest patch set uploaded with full side-by-side-diffs.
Thanks,
Carl
http://codereview.appspot.com/3928041/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM.
Don't forget to fix your copyright on beam-collision-engraver.
One set of braces to be removed.
Thanks,
Carl
http://codereview.appspot.com/3928041/diff/17001/lily/beam-collision-engraver.cc
File lily/beam-collision-engraver.cc (right):
Done - attached is a fresh patch set. The first three are what's already on
Rietveld, and the 4th is Carl's formatting copyright changes.
Cheers,
MS
0004-Changes-suggested-by-Carl.patch
Description: Binary data
0003-Intermediary-37-patch.patch
Description: Binary data
New patches pushed.
Thanks,
Carl
http://codereview.appspot.com/3928041/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
I only re-checked that one score, after the patch to overcome the
autobeam confusion. Only unnecessary lengthening now is some stems on
cue notes, but it really sticks out. I could only reduce it to four
required ingredients: manual beams, 32nd notes, dots, and CueVoice.
\music = \relative c''
Original Message
Only unnecessary lengthening now is some stems on
cue notes, but it really sticks out.
Well, as they say, pictures or it didn't happen The cue from the
trumpets looks desperate for attention.
I could only reduce it to four
required ingredients: manual
35 matches
Mail list logo
