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

Issue 4465049: Implements beam collision rest avoidance. (Closed)

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

Description

Implements beam collision rest avoidance.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -0 lines) Patch
M lily/beam-collision-engraver.cc View 3 chunks +8 lines, -0 lines 0 comments Download
M scm/define-grobs.scm View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 6
MikeSol
I can already see that this may lead to circular dependencies, but (miraculously) it passes ...
12 years, 11 months ago (2011-05-05 16:30:28 UTC) #1
Neil Puttock
On 2011/05/05 16:30:28, MikeSol wrote: > (a) People to confirm that the circular dependency I ...
12 years, 11 months ago (2011-05-05 20:44:36 UTC) #2
Neil Puttock
On 2011/05/05 20:44:36, Neil Puttock wrote: > On 2011/05/05 16:30:28, MikeSol wrote: > > > ...
12 years, 11 months ago (2011-05-05 20:50:43 UTC) #3
mike_apollinemike.com
On May 5, 2011, at 1:50 PM, n.puttock@gmail.com wrote: > On 2011/05/05 20:44:36, Neil Puttock ...
12 years, 11 months ago (2011-05-05 23:51:18 UTC) #4
hanwenn
On Thu, May 5, 2011 at 5:44 PM, <n.puttock@gmail.com> wrote: > > Did you do ...
12 years, 11 months ago (2011-05-06 02:43:52 UTC) #5
mike_apollinemike.com
12 years, 10 months ago (2011-05-10 13:48:21 UTC) #6
On May 5, 2011, at 7:51 PM, mike@apollinemike.com wrote:

> On May 5, 2011, at 1:50 PM, n.puttock@gmail.com wrote:
> 
>> On 2011/05/05 20:44:36, Neil Puttock wrote:
>>> On 2011/05/05 16:30:28, MikeSol wrote:
>> 
>>>> (a) People to confirm that the circular dependency I fear (beam
>> placement
>>>> relying on rest placement relying on beam placement relying on...)
>> does not
>>>> exist.
>> 
>>> Did you do a regtest run with an unoptimised binary?
>> 
>>> I get cyclic dependency errors in three tests.  Here's an example from
>>> beam-collision-basic.ly:
>> 
>>> @@ -4,6 +4,48 @@
>>> Preprocessing graphical objects...
>>> Calculating line breaks...
>>> Drawing systems...
>>> +beam-collision-basic.ly:16:25: programming error: cyclic dependency:
>>> calculation-in-progress encountered for #'quantized-positions (Beam)
>>> +    \repeat unfold 8 { c8
>>> +                         [ c] }
>>> +beam-collision-basic.ly:16:25: programming error: cyclic dependency:
>>> calculation-in-progress encountered for #'quantized-positions (Beam)
>>> +    \repeat unfold 8 { c8
>>> +                         [ c] }
>>> +beam-collision-basic.ly:16:27: programming error: cyclic dependency:
>>> calculation-in-progress encountered for #'stem-end-position (Stem)
>>> +    \repeat unfold 8 { c8[
>>> +                           c] }
>>> +beam-collision-basic.ly:16:25: programming error: cyclic dependency:
>>> calculation-in-progress encountered for #'quantized-positions (Beam)
>>> +    \repeat unfold 8 { c8
>>> +                         [ c] }
>>> +beam-collision-basic.ly:16:25: programming error: cyclic dependency:
>>> calculation-in-progress encountered for #'quantized-positions (Beam)
>>> +    \repeat unfold 8 { c8
>>> +                         [ c] }
>>> +beam-collision-basic.ly:16:27: programming error: cyclic dependency:
>>> calculation-in-progress encountered for #'stem-end-position (Stem)
>>> +    \repeat unfold 8 { c8[
>>> +                           c] }
>>> +beam-collision-basic.ly:16:27: programming error: cyclic dependency:
>>> calculation-in-progress encountered for #'Y-extent (Stem)
>>> +    \repeat unfold 8 { c8[
>>> +                           c] }
>>> +beam-collision-basic.ly:16:27: programming error: cyclic dependency:
>>> calculation-in-progress encountered for #'Y-extent (Stem)
>>> +    \repeat unfold 8 { c8[
>>> +                           c] }
>>> +beam-collision-basic.ly:16:25: programming error: cyclic dependency:
>>> calculation-in-progress encountered for #'quantized-positions (Beam)
>>> +    \repeat unfold 8 { c8
>>> +                         [ c] }
>>> +beam-collision-basic.ly:16:25: programming error: cyclic dependency:
>>> calculation-in-progress encountered for #'quantized-positions (Beam)
>>> +    \repeat unfold 8 { c8
>>> +                         [ c] }
>>> +beam-collision-basic.ly:16:27: programming error: cyclic dependency:
>>> calculation-in-progress encountered for #'stem-end-position (Stem)
>>> +    \repeat unfold 8 { c8[
>>> +                           c] }
>>> +beam-collision-basic.ly:16:25: programming error: cyclic dependency:
>>> calculation-in-progress encountered for #'quantized-positions (Beam)
>>> +    \repeat unfold 8 { c8
>>> +                         [ c] }
>>> +beam-collision-basic.ly:16:25: programming error: cyclic dependency:
>>> calculation-in-progress encountered for #'quantized-positions (Beam)
>>> +    \repeat unfold 8 { c8
>>> +                         [ c] }
>>> +beam-collision-basic.ly:16:27: programming error: cyclic dependency:
>>> calculation-in-progress encountered for #'stem-end-position (Stem)
>>> +    \repeat unfold 8 { c8[
>>> +                           c] }
>>> Writing header field `texidoc' to `./53/lily-2d77f62d.texidoc'...
>>> Writing ./53/lily-2d77f62d-1.signature
>>> Writing ./53/lily-2d77f62d-2.signature
>> 
>>> This is probably related to the existing rest translation code which
>> chains a
>>> Y-offset callback (ly:beam::rest-collision-callback) for Rest when
>> there's a
>>> rest under a manual beam.
>> 
>> Actually, that's not the case for this snippet, though it is probably
>> responsible for a similar warning in dot-rest-beam-trigger.ly:
>> 
>> +dot-rest-beam-trigger.ly:13:21: programming error: cyclic dependency:
>> calculation-in-progress encountered for #'positions (Beam)
>> +    { \time 12/16 c16
>> +                     [ b a r  b g] }
>> 
>> http://codereview.appspot.com/4465049/
>> 
> 
> I see on line 1682 of beam.cc:
> 
>  Drul_array<Real> pos (robust_scm2drul (beam->get_property ("positions"),
>                                         Drul_array<Real> (0,0)));
> 
> My question is: in the old code sans rest collision avoidance, why does the
rest not get the correct quantized positions of the beam and shift accordingly?
> 
> 

OK - after reading more, my old question is answered but I have a new one.

It seems like rests are only moved with respect to the beam to which their stem
is attached (line 1672, bem.cc).  However, in the beam collision engraver, lines
114-117 in this patch would have in theory skipped over this type of rest, which
means it would never be part of the covered grobs list.

Any guesses as to where else the cyclic dependency (dependencies) may be coming
from?  Or am I reading the code wrong?

Cheers,
MS
Sign in to reply to this message.

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