Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(865)

Issue 4917046: Improves horizontal spacing of axis groups that SpanBar grobs traverse. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 7 months ago by MikeSol
Modified:
11 years, 3 months ago
Reviewers:
mike7
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Improves horizontal spacing of axis groups that SpanBar grobs traverse.

Patch Set 1 #

Total comments: 9

Patch Set 2 : Incorporates Joe's suggestions #

Patch Set 3 : Rebased against current master. #

Total comments: 36

Patch Set 4 : Responses to Neil and Joe's comments. #

Patch Set 5 : Rebase against current master. #

Patch Set 6 : Fix for 1846 rebased against current master #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+522 lines, -9 lines) Patch
A input/regression/span-bar-allow-span-bar.ly View 1 2 3 1 chunk +59 lines, -0 lines 0 comments Download
M lily/grob.cc View 1 2 3 1 chunk +76 lines, -0 lines 1 comment Download
M lily/include/grob.hh View 1 1 chunk +6 lines, -0 lines 0 comments Download
A lily/include/pure-from-neighbor-interface.hh View 1 2 3 1 chunk +34 lines, -0 lines 0 comments Download
A lily/pure-from-neighbor-engraver.cc View 1 2 3 1 chunk +104 lines, -0 lines 0 comments Download
A lily/pure-from-neighbor-interface.cc View 1 2 3 1 chunk +58 lines, -0 lines 0 comments Download
M lily/span-bar.cc View 1 chunk +0 lines, -2 lines 0 comments Download
M lily/span-bar-engraver.cc View 1 4 chunks +21 lines, -4 lines 0 comments Download
A lily/span-bar-stub-engraver.cc View 1 2 3 1 chunk +143 lines, -0 lines 0 comments Download
M ly/engraver-init.ly View 1 2 3 2 chunks +2 lines, -0 lines 0 comments Download
M scm/define-grob-properties.scm View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M scm/define-grobs.scm View 1 2 3 4 5 2 chunks +12 lines, -3 lines 0 comments Download
M scm/output-lib.scm View 1 2 3 4 2 chunks +6 lines, -0 lines 0 comments Download

Messages

Total messages: 27
MikeSol
Hey all, I've been thinking about Neil's comment regarding a better interface for cross-staff stems. ...
12 years, 7 months ago (2011-08-20 20:51:18 UTC) #1
MikeSol
Hey all, I just realized that in this patch, the Span_bar_stub_engraver could be bumped down ...
12 years, 7 months ago (2011-08-21 09:13:32 UTC) #2
pkx166h
ok, this passes make but I get a lot of reg tests show up but ...
12 years, 7 months ago (2011-08-27 08:40:07 UTC) #3
mikesol_ufl.edu
On Aug 27, 2011, at 10:40 AM, pkx166h@gmail.com wrote: > ok, this passes make but ...
12 years, 7 months ago (2011-08-27 12:47:42 UTC) #4
hanwenn
I read through this patch, but I can't tell what it does or is supposed ...
12 years, 7 months ago (2011-08-27 14:23:15 UTC) #5
mikesol_ufl.edu
On Aug 27, 2011, at 4:23 PM, hanwenn@gmail.com wrote: > I read through this patch, ...
12 years, 7 months ago (2011-08-27 14:43:00 UTC) #6
MikeSol
Thanks Han-Wen! Cheers, MS http://codereview.appspot.com/4917046/diff/1/lily/align-interface.cc File lily/align-interface.cc (right): http://codereview.appspot.com/4917046/diff/1/lily/align-interface.cc#newcode399 lily/align-interface.cc:399: Align_interface::vertical_sort (Grob *g1, Grob *g2) ...
12 years, 7 months ago (2011-08-30 07:10:00 UTC) #7
joeneeman
Correct me if I'm wrong, but this is my understanding: wherever there's a SpanBar, you're ...
12 years, 7 months ago (2011-08-30 15:53:16 UTC) #8
mikesol_ufl.edu
On Aug 30, 2011, at 5:53 PM, joeneeman@gmail.com wrote: > Correct me if I'm wrong, ...
12 years, 7 months ago (2011-08-30 16:28:28 UTC) #9
joeneeman
On Tue, Aug 30, 2011 at 8:58 AM, Mike Solomon <mikesol@ufl.edu> wrote: > On Aug ...
12 years, 7 months ago (2011-08-30 21:26:50 UTC) #10
MikeSol
Hey all, I'm not sure if my previous message went through, so sorry for a ...
12 years, 7 months ago (2011-08-31 14:39:04 UTC) #11
MikeSol
Hey all, So that people can confirm the visual output of this, I'm going to ...
12 years, 7 months ago (2011-09-01 16:01:50 UTC) #12
Bertrand Bordage
This requires an update. The patch fails to apply. Bertrand
12 years, 7 months ago (2011-09-01 16:12:32 UTC) #13
mikesol_ufl.edu
On Sep 1, 2011, at 6:12 PM, bordage.bertrand@gmail.com wrote: > This requires an update. > ...
12 years, 7 months ago (2011-09-01 16:14:25 UTC) #14
pkx166h
new patch passes make and reg tests
12 years, 7 months ago (2011-09-02 22:51:03 UTC) #15
joeneeman
lgtm http://codereview.appspot.com/4917046/diff/18001/lily/grob.cc File lily/grob.cc (right): http://codereview.appspot.com/4917046/diff/18001/lily/grob.cc#newcode593 lily/grob.cc:593: return get_maybe_root_vertical_alignment (g, 0); I still think this ...
12 years, 7 months ago (2011-09-02 23:27:44 UTC) #16
Trevor Daniels
Hi Mike I found a visual difference with one of my scores. I finally tracked ...
12 years, 6 months ago (2011-09-09 17:09:23 UTC) #17
mikesol_ufl.edu
On Sep 9, 2011, at 7:09 PM, tdanielsmusic@googlemail.com wrote: > Hi Mike > I found ...
12 years, 6 months ago (2011-09-17 15:15:19 UTC) #18
Neil Puttock
http://codereview.appspot.com/4917046/diff/18001/input/regression/span-bar-allow-span-bar.ly File input/regression/span-bar-allow-span-bar.ly (right): http://codereview.appspot.com/4917046/diff/18001/input/regression/span-bar-allow-span-bar.ly#newcode2 input/regression/span-bar-allow-span-bar.ly:2: \version "2.15.10" 2.15.12 http://codereview.appspot.com/4917046/diff/18001/input/regression/span-bar-allow-span-bar.ly#newcode5 input/regression/span-bar-allow-span-bar.ly:5: texidoc = "The SpanBarStub ...
12 years, 6 months ago (2011-09-17 19:01:38 UTC) #19
Neil Puttock
On 17 September 2011 15:45, Mike Solomon <mikesol@ufl.edu> wrote: > The problem comes not from ...
12 years, 6 months ago (2011-09-17 20:03:14 UTC) #20
MikeSol
Hey all, Responses to Neil and Joe below, and a new patchset posted. I'd like ...
12 years, 6 months ago (2011-09-18 05:55:03 UTC) #21
pkx166h
Passes make, reg tests that get spewed out look identical, too many to attach but ...
12 years, 6 months ago (2011-09-24 19:31:45 UTC) #22
mikesol_ufl.edu
On Sep 24, 2011, at 9:31 PM, pkx166h@gmail.com wrote: > Passes make, reg tests that ...
12 years, 6 months ago (2011-09-25 03:41:35 UTC) #23
Keith
On 2011/08/30 16:28:28, mikesol_ufl.edu wrote: > On Aug 30, 2011, at 5:53 PM, mailto:joeneeman@gmail.com wrote: ...
12 years, 4 months ago (2011-11-10 07:19:13 UTC) #24
mikesol_ufl.edu
On Nov 9, 2011, at 11:19 PM, k-ohara5a5a@oco.net wrote: > On 2011/08/30 16:28:28, mikesol_ufl.edu wrote: ...
12 years, 4 months ago (2011-11-10 14:58:25 UTC) #25
Keith
https://codereview.appspot.com/4917046/diff/34001/lily/grob.cc File lily/grob.cc (right): https://codereview.appspot.com/4917046/diff/34001/lily/grob.cc#newcode647 lily/grob.cc:647: g1->programming_error ("could not place this grob in its axis ...
11 years, 3 months ago (2012-12-22 21:14:30 UTC) #26
mike7
11 years, 3 months ago (2012-12-22 23:06:54 UTC) #27

On 22 déc. 2012, at 23:14, k-ohara5a5a@oco.net wrote:

> 
> https://codereview.appspot.com/4917046/diff/34001/lily/grob.cc
> File lily/grob.cc (right):
> 
> https://codereview.appspot.com/4917046/diff/34001/lily/grob.cc#newcode647
> lily/grob.cc:647: g1->programming_error ("could not place this grob in
> its axis group");
> It sounds like you're mixing together "axis groups" and "Axis group
> engraver." The axis group engraver creates VerticalAxisGroups, which
> implement the axis-group-interface.  There are, however, numerous grobs
> that implement the axis-group-interface that are not created in the
> Axis_group_engraver.
> 
> https://codereview.appspot.com/4917046/

Couldn't have said it better myself! Wait a minute...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b