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

Issue 7768043: rewrite Self_alignment_interface

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 6 months ago by janek
Modified:
5 years, 10 months ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! I strongly recommend to view changes in a git branch, not here (you'll be able to see individual commits and follow the logic of changes instead of viewing one big diff). To do that, go to your lilypond git repository and type git log --patch --reverse --color=always \ master..origin/dev/janek/cleaned-alignment thanks!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! 66a8ff7 use X-parents to align MMRNumbers, MMRTexts and PercentRepeatCounters 12ab158 Remove x_centered_on_y_parent from Self_alignment_interface. 7d793a9 Self_alignment_interface: introduce align_grob (issue 3239). 4d7318f If a grob's parent is a PaperColumn, align using NoteColumn extent. 03b1173 Allow separate alignment "factors" for me and parent. 8424e5b Calculate dynamics alignment in a sane, configurable way. 20a9f06 Calculate fingerings alignment in a sane, configurable way. 2f5b7b3 Remove add_offset_callback, set_center_parent and set_align_self. c626752 Remove special case in DynamicText alignment. 10e4327 Remove centered_on_note_columns from Self_alignment_interface. Use align-grob in define-grobs.scm: [...] (several commits) eafb4ee Remove centered_on_._parent from Self_alignment_interface. Use align-grob in define-grobs.scm: [...] (several commits) 15de696 allow to use nonstandard extents for alignment

Patch Set 1 #

Patch Set 2 : return 0 if him is a PaperColumn to avoid regressions #

Patch Set 3 : rewrite. replaces all three methods in self-alignment-interface with one #

Patch Set 4 : change explanation comment #

Patch Set 5 : don't report additional programming errors #

Total comments: 7

Patch Set 6 : rewrite comments and interface description #

Patch Set 7 : fix silly typo #

Patch Set 8 : lots of stuff. Please review in dev/janek-alignment branch #

Total comments: 2

Patch Set 9 : some progress. #

Total comments: 4

Patch Set 10 : simplify things a bit. #

Total comments: 4

Patch Set 11 : add regtest suggested by Trevor #

Patch Set 12 : align MMR and perc.rep. stuff using xparents #

Unified diffs Side-by-side diffs Delta from patch set Stats (+296 lines, -264 lines) Patch
M input/regression/text-spanner-attachment-alignment.ly View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
A input/regression/unassociated-lyrics-alignment.ly View 1 2 3 4 5 6 7 1 chunk +24 lines, -0 lines 0 comments Download
A input/regression/unattached-lyrics-alignment.ly View 1 2 3 4 5 6 7 8 9 10 1 chunk +20 lines, -0 lines 0 comments Download
M lily/dynamic-engraver.cc View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -1 line 0 comments Download
M lily/fingering-engraver.cc View 1 2 3 4 5 6 7 8 2 chunks +18 lines, -8 lines 0 comments Download
M lily/flag.cc View 1 2 3 4 5 6 7 2 chunks +0 lines, -10 lines 0 comments Download
M lily/grob-closure.cc View 1 2 3 4 5 6 7 1 chunk +0 lines, -35 lines 0 comments Download
M lily/include/grob.hh View 1 2 3 4 5 6 7 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/paper-column.hh View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M lily/include/self-alignment-interface.hh View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +3 lines, -7 lines 0 comments Download
M lily/multi-measure-rest-engraver.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +3 lines, -0 lines 0 comments Download
M lily/new-dynamic-engraver.cc View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -1 line 0 comments Download
M lily/new-fingering-engraver.cc View 1 2 3 4 5 6 7 8 2 chunks +10 lines, -4 lines 0 comments Download
M lily/paper-column.cc View 1 2 3 4 5 6 7 1 chunk +16 lines, -0 lines 0 comments Download
M lily/percent-repeat-engraver.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +3 lines, -0 lines 0 comments Download
M lily/self-alignment-interface.cc View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +85 lines, -104 lines 0 comments Download
M lily/spanner.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +35 lines, -2 lines 0 comments Download
M ly/engraver-init.ly View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -7 lines 0 comments Download
M scm/define-grob-properties.scm View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +32 lines, -6 lines 0 comments Download
M scm/define-grobs.scm View 1 2 3 4 5 6 7 8 9 10 11 27 chunks +43 lines, -75 lines 0 comments Download
M scm/output-lib.scm View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 31
janek
Please review. You'll find an illustration in the issue tracker: http://code.google.com/p/lilypond/issues/detail?id=3239#c2
6 years, 6 months ago (2013-03-12 22:09:10 UTC) #1
janek
return 0 when him is a PaperColumn to avoid regressions
6 years, 6 months ago (2013-03-14 20:05:55 UTC) #2
janek
return 0 if him is a PaperColumn to avoid regressions
6 years, 6 months ago (2013-03-14 20:07:12 UTC) #3
janek
ok, now i see that i've unnecessarily mixed two different things: rewriting self-alignment-interface (this patch) ...
6 years, 6 months ago (2013-03-17 18:01:20 UTC) #4
janek
rewrite. replaces all three methods in self-alignment-interface with one
6 years, 6 months ago (2013-03-17 23:31:16 UTC) #5
janek
change explanation comment
6 years, 6 months ago (2013-03-18 07:44:06 UTC) #6
janek
don't report additional programming errors
6 years, 6 months ago (2013-03-18 12:43:18 UTC) #7
MikeSol
https://codereview.appspot.com/7768043/diff/19001/lily/self-alignment-interface.cc File lily/self-alignment-interface.cc (right): https://codereview.appspot.com/7768043/diff/19001/lily/self-alignment-interface.cc#newcode128 lily/self-alignment-interface.cc:128: /* LilyPond positions every grob relative to its parent ...
6 years, 6 months ago (2013-03-19 15:28:45 UTC) #8
janek
Hi Mike & all, On 2013/03/19 15:21:37, MikeSol wrote: > I like where you're going ...
6 years, 6 months ago (2013-03-19 21:04:16 UTC) #9
janek
rewrite comments and interface description
6 years, 6 months ago (2013-03-19 21:04:48 UTC) #10
janek
fix silly typo
6 years, 6 months ago (2013-03-19 21:33:45 UTC) #11
mike7
On 19 mars 2013, at 22:04, janek.lilypond@gmail.com wrote: > Hi Mike & all, > > ...
6 years, 6 months ago (2013-03-19 22:41:32 UTC) #12
janek
Hi, On Tue, Mar 19, 2013 at 11:40 PM, mike@mikesolomon.org <mike@mikesolomon.org> wrote: > The main ...
6 years, 6 months ago (2013-03-19 23:28:14 UTC) #13
mike7
On 20 mars 2013, at 00:27, Janek Warchoł <janek.lilypond@gmail.com> wrote: > Hi, > > On ...
6 years, 6 months ago (2013-03-20 04:24:17 UTC) #14
janek
Hi all, after some thinking i've come to a conclusion that we should keep this ...
6 years, 6 months ago (2013-03-20 22:18:40 UTC) #15
janek
lots of stuff. Please review in dev/janek-alignment branch
6 years, 5 months ago (2013-03-30 18:06:29 UTC) #16
MikeSol
In general, I see a lot of reassigning of parents. What is the goal with ...
6 years, 5 months ago (2013-03-30 18:15:11 UTC) #17
janek
On 2013/03/30 18:15:11, MikeSol wrote: > In general, I see a lot of reassigning of ...
6 years, 5 months ago (2013-03-30 18:25:23 UTC) #18
janek
some progress.
6 years, 5 months ago (2013-03-31 22:26:30 UTC) #19
MikeSol
I'd recommend adding a sort of padding property to the self-alignment-interface to get it completely ...
6 years, 5 months ago (2013-04-01 05:52:00 UTC) #20
janek
Hi all, On Mon, Apr 1, 2013 at 7:52 AM, <mtsolo@gmail.com> wrote: > I'd recommend ...
6 years, 5 months ago (2013-04-01 11:13:38 UTC) #21
mike7
On 1 avr. 2013, at 14:13, Janek Warchoł <janek.lilypond@gmail.com> wrote: > Hi all, > > ...
6 years, 5 months ago (2013-04-01 11:20:40 UTC) #22
janek
On Mon, Apr 1, 2013 at 1:20 PM, mike@mikesolomon.org <mike@mikesolomon.org> wrote: > X-extent is used ...
6 years, 5 months ago (2013-04-01 21:51:18 UTC) #23
mike7
On 2 avr. 2013, at 00:51, janek.lilypond@gmail.com wrote: > On Mon, Apr 1, 2013 at ...
6 years, 5 months ago (2013-04-02 04:26:29 UTC) #24
janek
On 2013/04/02 04:26:29, mike7 wrote: > On 2 avr. 2013, at 00:51, mailto:janek.lilypond@gmail.com wrote: > ...
6 years, 5 months ago (2013-04-02 11:42:14 UTC) #25
janek
simplify things a bit.
6 years, 5 months ago (2013-04-02 21:43:55 UTC) #26
janek
On 2013/04/02 21:43:55, janek wrote: > simplify things a bit. Oops, sorry, wrong description. This ...
6 years, 5 months ago (2013-04-02 22:48:52 UTC) #27
Trevor Daniels
Code not tested or even eye-balled. https://codereview.appspot.com/7768043/diff/76001/input/regression/text-spanner-attachment-alignment.ly File input/regression/text-spanner-attachment-alignment.ly (right): https://codereview.appspot.com/7768043/diff/76001/input/regression/text-spanner-attachment-alignment.ly#newcode21 input/regression/text-spanner-attachment-alignment.ly:21: \repeat unfold 2 ...
6 years, 5 months ago (2013-04-03 17:12:14 UTC) #28
janek
Thanks for the review! On 2013/04/03 17:12:14, Trevor Daniels wrote: > Code not tested or ...
6 years, 5 months ago (2013-04-03 18:00:37 UTC) #29
janek
add regtest suggested by Trevor
6 years, 5 months ago (2013-04-04 14:23:34 UTC) #30
janek
6 years, 5 months ago (2013-04-08 23:46:31 UTC) #31
align MMR and perc.rep. stuff using xparents
Sign in to reply to this message.

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