ok, this passes make but I get a lot of reg tests show up but I cannot see any diffs at all (i.e no 'green' shadows that indicate the changes). Either it's unbelievably subtle or something else is triggering the reg tests to show up. They are to big to post them all here but he diffs that show up are
After the diversion about stem direction affecting horizontal spacing this morning I've now returned to the real issue, which identifies a failing in Mike's patch caused by the stem direction. This is explained in http://codereview.appspot.com/4917046/.
Returning this issue to patch-needs_work.
Labels: -Patch-push Patch-needs_work
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
The follow-up patch, 70fd22ce, enables the code from this issue to print
"programming error: grob does not belong to a VerticalAlignment?"
on the regression test ‘dynamics-text-dynamics-context.ly’
Labels: -Fixed_2_15_14 Status: Started
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
This sorting can be deleted because the order in which contexts report their information to engravers is always top to bottom, so the sorting is unnecessary. I put that code in there in case one day someone undoes this ordering. However, as these programming erros show, it doesn't always work in the "Interpreting Music" phase, when grobs are not yet assured to have parents.
I think that if the top-to-bottom ordering of contexts is undone, it'll wreak havoc in many engravers, so this sorting of the vector is likely redundant and therefore unnecessary.
Labels: Patch-new
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Grob *
Grob::get_vertical_axis_group (Grob *g)
{
if (!g)
return 0;
if (!g->get_parent (Y_AXIS))
return 0;
if (Axis_group_interface::has_interface (g)
&& Align_interface::has_interface (g->get_parent (Y_AXIS)))
return g;
return get_vertical_axis_group (g->get_parent (Y_AXIS));
}
Is that correct? Why does the parent need an Align_interface but the grob itself only an Axis_group_interface? I don't claim to understand this code, but the name of the function is get_vertical_axis_group, so what's with the Align_interface?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
The code quoted in comment 24 recognizes a vertical axis group as something that
1) is an axis_group (which could also be a horizontal axis group) and
2) lives within something having the align_interface (Score, StaffGroup etc)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Originally posted by: pkx1...@gmail.com
ok, this passes make but I get a lot of reg tests show up but I cannot see any diffs at all (i.e no 'green' shadows that indicate the changes). Either it's unbelievably subtle or something else is triggering the reg tests to show up. They are to big to post them all here but he diffs that show up are
bar-extent.ly
tablature-tie-behaviour.ly
slur-cross-staff.ly
volta-multi-staff.ly
volta-multi-staff-inner-staff.ly
tablature-grace-notes.ly
span-bar-partial.ly
double-repeat.ly
spanner-alignment.ly
beam-cross-staff.ly
palm-mute.ly
bar-line-segno.ly
dead-notes.ly
context-nested-staffgroup.ly
tablature-glissando.ly
spacing-knee-compressed.ly
beam-cross-staff-slope.ly
slur-extreme.ly
note-line.ly
span-bar-break.ly
voice-follower.ly
cluster-cross-staff.ly
auto-change.ly
page-spacing-staff-group-nested.ly
tablature-harmonic-tie.ly
remove-empty-staves-auto-knee.ly
grace-staff-length.ly
span-bar-spacing.ly
arpeggio-span-collision.ly
etc.
Not all but a *lot*
So I don't know if this needs more work or not really, but am putting this as 'review' for those that know better than I.
Labels: -Patch-new Patch-review
Owner: mts...@gmail.com
Originally posted by: pkx1...@gmail.com
Mike has made a new patch..
Passes make and reg tests
Originally posted by: ColinPKC...@gmail.com
Counted down to 20110907.
Labels: -Patch-review Patch-push
Originally posted by: tdanielsmusic
After the diversion about stem direction affecting horizontal spacing this morning I've now returned to the real issue, which identifies a failing in Mike's patch caused by the stem direction. This is explained in http://codereview.appspot.com/4917046/.
Returning this issue to patch-needs_work.
Labels: -Patch-push Patch-needs_work
Originally posted by: percival.music.ca@gmail.com
Mike thinks this is ready now.
Labels: -Patch-needs_work Patch-review
Originally posted by: percival.music.ca@gmail.com
oops, should be countdown
Labels: -Patch-review Patch-countdown
Originally posted by: percival.music.ca@gmail.com
no wait, I have no evidence that the patch can pass a regtest comparison.
Labels: -Patch-countdown Patch-new
Originally posted by: pkx1...@gmail.com
Passes make, reg tests that get spewed out look identical, too many to attach but they are zipped here
http://lilypond-stuff.1065243.n5.nabble.com/http-code-google-com-p-lilypond-issues-detail-id-1846-td4837220.html
Labels: -Patch-new Patch-review
Originally posted by: ColinPKC...@gmail.com
(No comment was entered for this change.)
Labels: -Patch-review Patch-countdown
Originally posted by: ColinPKC...@gmail.com
Counted down to 20110928
Labels: -Patch-countdown Patch-push
Originally posted by: mts...@gmail.com
Improves horizontal spacing of axis groups that SpanBar grobs traverse.
http://codereview.appspot.com/4917046
Labels: Patch-new
Originally posted by: mts...@gmail.com
Fixed with [r20670d51f8d97fd390210dd239b3b2427f071e7c].
Status: Fixed
Originally posted by: ColinPKC...@gmail.com
(No comment was entered for this change.)
Labels: Fixed_2_15_14
Originally posted by: pkx1...@gmail.com
Mike re-verted and the re-pushed with a new commit
commit [r4f49b000d6e257724e311b406e2346b8388c1f0e]
Originally posted by: PhilEHol...@googlemail.com
Verified as pushed with that commitish
Status: Verified
Originally posted by: tdanielsmusic
(No comment was entered for this change.)
Labels: -Patch-new
Originally posted by: k-ohara5...@oco.net
The follow-up patch, 70fd22ce, enables the code from this issue to print
"programming error: grob does not belong to a VerticalAlignment?"
on the regression test ‘dynamics-text-dynamics-context.ly’
Labels: -Fixed_2_15_14
Status: Started
Originally posted by: mts...@gmail.com
Fix for comment 17 at http://codereview.appspot.com/5528081
This sorting can be deleted because the order in which contexts report their information to engravers is always top to bottom, so the sorting is unnecessary. I put that code in there in case one day someone undoes this ordering. However, as these programming erros show, it doesn't always work in the "Interpreting Music" phase, when grobs are not yet assured to have parents.
I think that if the top-to-bottom ordering of contexts is undone, it'll wreak havoc in many engravers, so this sorting of the vector is likely redundant and therefore unnecessary.
Labels: Patch-new
Originally posted by: lilypond...@gmail.com
Patchy the autobot says: LGTM.
Labels: Patch-review
Originally posted by: ColinPKC...@gmail.com
(No comment was entered for this change.)
Labels: -Patch-review Patch-countdown
Originally posted by: ColinPKC...@gmail.com
Counted down to 20120117, please push.
Labels: -Patch-countdown Patch-push
Originally posted by: mts...@gmail.com
Pushed as [rb667b7fe1bf651b7373014204edbe0e68f17326e]
Status: Fixed
Originally posted by: PhilEHol...@googlemail.com
Verified from committish
Status: Verified
Originally posted by: dak@gnu.org
Comment #17:
The code touched in commit 70fd22c reads:
Grob *
Grob::get_vertical_axis_group (Grob *g)
{
if (!g)
return 0;
if (!g->get_parent (Y_AXIS))
return 0;
if (Axis_group_interface::has_interface (g)
&& Align_interface::has_interface (g->get_parent (Y_AXIS)))
return g;
return get_vertical_axis_group (g->get_parent (Y_AXIS));
}
Is that correct? Why does the parent need an Align_interface but the grob itself only an Axis_group_interface? I don't claim to understand this code, but the name of the function is get_vertical_axis_group, so what's with the Align_interface?
Originally posted by: k-ohara5...@oco.net
The code quoted in comment 24 recognizes a vertical axis group as something that
1) is an axis_group (which could also be a horizontal axis group) and
2) lives within something having the align_interface (Score, StaffGroup etc)