Menu

#1846 Improves horizontal spacing of axis groups that SpanBar grobs traverse

Verified
nobody
None
Enhancement
2015-09-19
2011-08-26
Anonymous
No

Originally created by: *anonymous

Originally created by: percival.music.ca@gmail.com
Originally owned by: mts...@gmail.com

http://codereview.appspot.com/4917046/

Related

Issues: #2065
Issues: #2272

Discussion

1 2 > >> (Page 1 of 2)
  • Google Importer

    Google Importer - 2011-08-27

    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

     
  • Google Importer

    Google Importer - 2011-09-02

    Originally posted by: pkx1...@gmail.com

    Mike has made a new patch..

    Passes make and reg tests

     
  • Google Importer

    Google Importer - 2011-09-07

    Originally posted by: ColinPKC...@gmail.com

    Counted down to 20110907.

    Labels: -Patch-review Patch-push

     
  • Google Importer

    Google Importer - 2011-09-09

    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

     
  • Google Importer

    Google Importer - 2011-09-23

    Originally posted by: percival.music.ca@gmail.com

    Mike thinks this is ready now.

    Labels: -Patch-needs_work Patch-review

     
  • Google Importer

    Google Importer - 2011-09-23

    Originally posted by: percival.music.ca@gmail.com

    oops, should be countdown

    Labels: -Patch-review Patch-countdown

     
  • Google Importer

    Google Importer - 2011-09-23

    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

     
  • Google Importer

    Google Importer - 2011-09-26

    Originally posted by: ColinPKC...@gmail.com

    (No comment was entered for this change.)

    Labels: -Patch-review Patch-countdown

     
  • Google Importer

    Google Importer - 2011-09-28

    Originally posted by: ColinPKC...@gmail.com

    Counted down to 20110928

    Labels: -Patch-countdown Patch-push

     
  • Google Importer

    Google Importer - 2011-09-29

    Originally posted by: mts...@gmail.com

    Improves horizontal spacing of axis groups that SpanBar grobs traverse.

    http://codereview.appspot.com/4917046

    Labels: Patch-new

     
  • Google Importer

    Google Importer - 2011-09-29

    Originally posted by: mts...@gmail.com

    Fixed with [r20670d51f8d97fd390210dd239b3b2427f071e7c].

    Status: Fixed

     
  • Google Importer

    Google Importer - 2011-09-30

    Originally posted by: ColinPKC...@gmail.com

    (No comment was entered for this change.)

    Labels: Fixed_2_15_14

     
  • Google Importer

    Google Importer - 2011-10-01

    Originally posted by: pkx1...@gmail.com

    Mike re-verted and the re-pushed with a new commit

    commit    [r4f49b000d6e257724e311b406e2346b8388c1f0e]

     
  • Google Importer

    Google Importer - 2011-10-09

    Originally posted by: PhilEHol...@googlemail.com

    Verified as pushed with that commitish

    Status: Verified

     
  • Google Importer

    Google Importer - 2011-10-16

    Originally posted by: tdanielsmusic

    (No comment was entered for this change.)

    Labels: -Patch-new

     
  • Google Importer

    Google Importer - 2012-01-11

    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

     
  • Google Importer

    Google Importer - 2012-01-12

    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

     
  • Google Importer

    Google Importer - 2012-01-13

    Originally posted by: lilypond...@gmail.com

    Patchy the autobot says: LGTM.

    Labels: Patch-review

     
  • Google Importer

    Google Importer - 2012-01-15

    Originally posted by: ColinPKC...@gmail.com

    (No comment was entered for this change.)

    Labels: -Patch-review Patch-countdown

     
  • Google Importer

    Google Importer - 2012-01-17

    Originally posted by: ColinPKC...@gmail.com

    Counted down to 20120117, please push.

    Labels: -Patch-countdown Patch-push

     
  • Google Importer

    Google Importer - 2012-01-17

    Originally posted by: mts...@gmail.com

    Pushed as [rb667b7fe1bf651b7373014204edbe0e68f17326e]

    Status: Fixed

     
  • Google Importer

    Google Importer - 2012-01-29

    Originally posted by: PhilEHol...@googlemail.com

    Verified from committish

    Status: Verified

     
  • Google Importer

    Google Importer - 2015-07-15

    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?

     
  • Google Importer

    Google Importer - 2015-07-17

    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)

     
1 2 > >> (Page 1 of 2)
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.