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

Issue 5992073: Correctly positions dots on rests that share a column with a beamed grob.

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

Description

Correctly positions dots on rests that share a column with a beamed grob.

Patch Set 1 #

Patch Set 2 : Gets rid of vestigial code #

Patch Set 3 : Gets rid of valgrind #

Patch Set 4 : Makes Dots::calc_y_offset a true callback #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+123 lines, -28 lines) Patch
A input/regression/dots-beamed-rest.ly View 1 chunk +10 lines, -0 lines 0 comments Download
M lily/beam-collision-engraver.cc View 5 chunks +16 lines, -0 lines 0 comments Download
M lily/dot-column.cc View 1 5 chunks +24 lines, -13 lines 3 comments Download
M lily/dots.cc View 1 2 3 2 chunks +54 lines, -0 lines 0 comments Download
M lily/include/dot-column.hh View 1 chunk +1 line, -0 lines 0 comments Download
M lily/include/dots.hh View 1 chunk +6 lines, -0 lines 0 comments Download
M lily/include/staff-symbol-referencer.hh View 1 2 3 2 chunks +1 line, -2 lines 0 comments Download
M lily/rest.cc View 1 chunk +1 line, -0 lines 0 comments Download
M lily/staff-symbol-referencer.cc View 1 2 3 1 chunk +5 lines, -13 lines 0 comments Download
M scm/define-grob-properties.scm View 2 chunks +2 lines, -0 lines 0 comments Download
M scm/define-grobs.scm View 3 chunks +3 lines, -0 lines 0 comments Download

Messages

Total messages: 1
Keith
7 years, 2 months ago (2012-04-15 06:37:30 UTC) #1
I got about halfway through, but then it got too complicated with the various
ways to compute the Dots' Y-offset under various conditions.

http://codereview.appspot.com/5992073/diff/5001/lily/dot-column.cc
File lily/dot-column.cc (right):

http://codereview.appspot.com/5992073/diff/5001/lily/dot-column.cc#newcode163
lily/dot-column.cc:163: are due to the fact that this callback is called before
line breaking
"this callback *may be* called"
There seems to be nothing enforcing that dot-positioning is done at any
particular time.

http://codereview.appspot.com/5992073/diff/5001/lily/dot-column.cc#newcode195
lily/dot-column.cc:195: int p = before_line_breaking &
Dots::in_dot_column_with_beam_and_rest (dp.dot_)
This looks suspicious at first. But it seems that whenever this specific
condition is true, get_rounded_position() would return 0 anyway.

http://codereview.appspot.com/5992073/diff/5001/lily/dot-column.cc#newcode203
lily/dot-column.cc:203: 
It seems we want to just skip the collision resolution for dots on rests, and
let them ride with the rest, one position higher than the center of the rest.
      if (Rest::has_interface (note))
        {
          cfg[p+1] = dp;
          continue;
        }
Sign in to reply to this message.

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