On Mon, Aug 8, 2011 at 3:21 PM, <mtsolo@gmail.com> wrote: > Reviewers: , > > ...
13 years, 8 months ago
(2011-08-12 19:27:20 UTC)
#4
On Mon, Aug 8, 2011 at 3:21 PM, <mtsolo@gmail.com> wrote:
> Reviewers: ,
>
> Message:
> This fixes Issue 509 and passes regtests.
>
> Cheers,
> MS
>
> Description:
> Prevents nested tuplets from colliding.
>
> Please review this at
http://codereview.appspot.com/**4808082/<http://codereview.appspot.com/4808082/>
>
> Affected files:
> M input/regression/tuplet-nest.**ly <http://tuplet-nest.ly>
> M lily/tuplet-bracket.cc
> M scm/define-grob-properties.scm
> M scm/define-grobs.scm
>
This is very much appreciated, btw.
:-)
Trevor.
--
Trevor Bača
trevorbaca@gmail.com
On Aug 11, 2011, at 11:40 PM, n.puttock@gmail.com wrote: > Hi Mike, > > Have ...
13 years, 8 months ago
(2011-08-14 16:24:34 UTC)
#5
On Aug 11, 2011, at 11:40 PM, n.puttock@gmail.com wrote:
> Hi Mike,
>
> Have you tested this with broken tuplets? I've tried adding breaks at
> random in tuplet-nest.ly and get collisions in some cases.
>
> Cheers,
> Neil
Thanks Neil!
New patchset uploaded that fixes the line breaking problem.
I haven't gotten around to making the other changes you pointed out yet, but I
will either tonight or tomorrow.
Cheers,
MS
> I haven't gotten around to making the other changes you pointed out yet, but ...
13 years, 8 months ago
(2011-08-17 09:47:28 UTC)
#6
> I haven't gotten around to making the other changes you pointed out yet, but I
> will either tonight or tomorrow.
>
> Cheers,
> MS
Changes are up and ready for review.
Cheers,
MS
On Aug 19, 2011, at 11:58 PM, n.puttock@gmail.com wrote: > > > http://codereview.appspot.com/4808082/diff/13002/lily/tuplet-bracket.cc#newcode283 > lily/tuplet-bracket.cc:283: ...
13 years, 8 months ago
(2011-08-20 08:30:07 UTC)
#8
On Aug 19, 2011, at 11:58 PM, n.puttock@gmail.com wrote:
>
>
>
http://codereview.appspot.com/4808082/diff/13002/lily/tuplet-bracket.cc#newco...
> lily/tuplet-bracket.cc:283: SCM scm_x_span = me->get_property
> ("X-positions");
> I seem to recall we discussed the option of splitting control-points
> into separate X/Y properties (can't remember exactly which grob it was
> for :).
Beams (I think...).
> My main concern was the naming since 'positions should be
> changed to Y-positions, but this would be disruptive for other grobs.
>
This could be done in a separate patch.
>
http://codereview.appspot.com/4808082/diff/13002/lily/tuplet-bracket.cc#newco...
> lily/tuplet-bracket.cc:669: // have to re_run numbers to check for
> number-on-number collisions
> This is getting a bit complicated. Do you think it's feasible to have a
> grob which would collect the colliding brackets and do the collision
> avoidance as a separate positioning-done callback?
>
This would certainly avoid the guestimation of the numbers' positions...
I'll do some digging and get back to you!
Cheers,
MS
On Aug 20, 2011, at 10:30 AM, mike@apollinemike.com wrote: > >> http://codereview.appspot.com/4808082/diff/13002/lily/tuplet-bracket.cc#newcode669 >> lily/tuplet-bracket.cc:669: // ...
13 years, 8 months ago
(2011-08-21 15:09:11 UTC)
#9
On Aug 20, 2011, at 10:30 AM, mike@apollinemike.com wrote:
>
>>
http://codereview.appspot.com/4808082/diff/13002/lily/tuplet-bracket.cc#newco...
>> lily/tuplet-bracket.cc:669: // have to re_run numbers to check for
>> number-on-number collisions
>> This is getting a bit complicated. Do you think it's feasible to have a
>> grob which would collect the colliding brackets and do the collision
>> avoidance as a separate positioning-done callback?
>>
>
> This would certainly avoid the guestimation of the numbers' positions...
> I'll do some digging and get back to you!
>
So the digging has been done and the results are almost there. I have a new
(broken) patch set up that shows the state of things.
The problem is a circular dependency: in
Tuplet_bracket::calc_position_and_height, there is a variable my_offset that
tries to look up the Y-offset of the grob. This triggers positioning-done in
the Y-parent, which already needs to know the positions that will eventually be
furnished by calc_position_and_height.
If anyone sees an obvious workaround, let me know! I'll let it simmer for a
couple days and see if I can come up with something as well.
Cheers,
MS
Hey all, Sorry to not have pushed this yet, but I recently rebased this against ...
13 years, 7 months ago
(2011-08-29 11:14:52 UTC)
#11
Hey all,
Sorry to not have pushed this yet, but I recently rebased this against current
master and it looks like my stem work actually fixed a good deal of what some of
this patch was fixing. I've gotten rid of about half the code from this patch
and posted a new version.
For now, this is just a sketch - it passes regtests, but it does not allow for
some of the fine tuning that was in the previous patch (tuplet numbers now push
tuplet brackets much higher than they used to, invalidating the need for all
that skyline stuff from previous patches, thus the deletions). I'm not sure yet
why this fine tuning doesn't happen, but I'll figure it out in a couple weeks.
In any case, moxy nested tuplets won't be part of LilyPond for at least another
month, 3-4 weeks, but when they are, they'll be fully integrated with the new
Stem work.
Cheers,
MS
On 2011/08/30 21:57:44, J_lowe wrote: > Mike, passes make but during reg test check i ...
13 years, 7 months ago
(2011-09-01 13:15:44 UTC)
#13
On 2011/08/30 21:57:44, J_lowe wrote:
> Mike, passes make but during reg test check i get
>
>
Yikes! Sorry about that - I forgot to commit the deletion of that file (the
property it tests no longer exists).
New patchset uploaded.
Cheers,
MS
On Sep 6, 2011, at 9:41 AM, tdanielsmusic@googlemail.com wrote: > Mike says this is not ...
13 years, 7 months ago
(2011-09-17 10:20:20 UTC)
#16
On Sep 6, 2011, at 9:41 AM, tdanielsmusic@googlemail.com wrote:
> Mike says this is not yet ready for pushing in comment #11. We should
> not push while he is away without his confirmation.
>
> Trevor
>
Hey all,
I'm pretty sure that the differences I'm noticing come from more accurate
Y-extents of stems. However, if these Y-extents are in fact less accurate then
we're in trouble. All of my tests have come back reporting them as more
accurate, but then again, I designed my own tests (like the Y-extent printer I
sent out to the list back when I was working on stems). If people could report
back perceived changes in how new stem Y-extents are effecting layout (good or
bad), this'd help me evaluate if there are any flaws in the Stem code. For the
moment, I don't see any.
I'm comfortable with pushing this version of the patch in its current form
(after I re-run regtests on it and after I push other stem fixes being tossed
around - these fixes may have an impact on this patch). However, if people
think the gaps between the nested tuplets needs to be controlled via a separate
property, a new property can be added that controls the distance between nested
TupletBracket and TupletNumber grobs (currently, there is a
one-property-fits-all property called "padding"). It may be a good idea,
however, to get this pushed first and then to work on that later.
>
> http://codereview.appspot.com/4808082/diff/45009/lily/tuplet-bracket.cc
> File lily/tuplet-bracket.cc (right):
>
>
http://codereview.appspot.com/4808082/diff/45009/lily/tuplet-bracket.cc#newco...
> lily/tuplet-bracket.cc:285: if (!scm_is_pair (scm_x_span) ||
> !scm_is_pair (scm_positions))
> Should issue programming error if user has specified invalid value for
> 'positions.
>
Should there also be a programming error if X-positions is incorrectly set?
Also, wouldn't this erroneous setting be blocked with a property check
(positions is number-pair? in define-grob-properties.scm).
Cheers,
MS
Issue 4808082: Prevents nested tuplets from colliding.
(Closed)
Created 13 years, 8 months ago by MikeSol
Modified 13 years, 6 months ago
Reviewers: pkx166h, Neil Puttock, trevorbaca_gmail.com, mike_apollinemike.com, Trevor Daniels
Base URL:
Comments: 15
