#3385 Ottava with cross-staff messes up beams

[Anonymous]

Originally created by: *anonymous

Originally created by: jean-m...@nimzowitsch.ch
Originally owned by: k-ohara5...@oco.net

Dominic reports in http://lilypond.1069038.n5.nabble.com/2-17-19-Ottava-with-cross-staff-messed-up-beams-tp146480.html

%  On the stable 2.16.2, this is fine, but on the unstable 2.17.19,
%  the following short example results in extremely corrupted /
%  messed up stems. Bug?

chUp = \change Staff = "upper"
chDn = \change Staff = "lower"
\new PianoStaff
<<
    \new Staff = "upper"
        { \relative c' { \ottava #1 c'8 \chDn c \chUp c \chDn c \chUp c2 } }
    \new Staff = "lower" { s1 }
>>

I've found that starting with 2.17.14 the log warns about an error:

programming error: Grob direction requested while calculation in progress.
continuing, cross fingers

but the output still looks ok as in previous versions.

with 2.17.19 the output is weird

[Google Importer]

Originally posted by: dak@gnu.org

It's worthwhile to note that this also produces error messages:
xxx.ly:6:45: programming error: cyclic dependency: calculation-in-progress encountered for #'direction (Beam)
        { \relative c' { \ottava #1
                                            c'8 \chDn c \chUp c \chDn c \chUp c2 } }
xxx.ly:6:45: continuing, cross fingers
xxx.ly:6:45: programming error: cyclic dependency: calculation-in-progress encountered for #'direction (Stem)
        { \relative c' { \ottava #1
                                            c'8 \chDn c \chUp c \chDn c \chUp c2 } }
xxx.ly:6:45: continuing, cross fingers
programming error: Grob direction requested while calculation in progress.
continuing, cross fingers
xxx.ly:6:63: programming error: cyclic dependency: calculation-in-progress encountered for #'direction (Stem)
        { \relative c' { \ottava #1 c'8 \chDn c \chUp
                                                              c \chDn c \chUp c2 } }
xxx.ly:6:63: continuing, cross fingers
programming error: Grob direction requested while calculation in progress.
continuing, cross fingers
xxx.ly:6:63: programming error: cyclic dependency: calculation-in-progress encountered for #'direction (Stem)
        { \relative c' { \ottava #1 c'8 \chDn c \chUp
                                                              c \chDn c \chUp c2 } }
xxx.ly:6:63: continuing, cross fingers
programming error: Grob direction requested while calculation in progress.
continuing, cross fingers

Bisecting now.

Summary: Ottava with cross-staff messes up beams

[Google Importer]

Originally posted by: dak@gnu.org

[r1c893a4e3651018adcc13ea2c8f1c19b19faa722] is the first bad commit
commit [r1c893a4e3651018adcc13ea2c8f1c19b19faa722]
Author: Keith OHara <k-ohara5a5a@oco.net>
Date:   Tue May 14 00:26:55 2013 -0700

    side-position-interface: use real positions of cross-staff grobs; issue 3363

:040000 040000 [reaca92f40e8eb33bd8d92678fe3f0dc9d31cd273] [rb487cf159bf2a644c8cc15332099d885ffbecfee] M    input
:040000 040000 [r7440095c707627ffcc8a14e487d43a98da855bc3] [r4286d076c5030e0125c7e2afe2b4da811b81b1db] M    lily

It is worth noting that circular dependency errors keep getting thrown out both before and after this commit, it is just that the output gets worse after this commit.

[Google Importer]

Originally posted by: dak@gnu.org

And the commit that _started_ the circular dependency errors (just not with similarly bad results) has been
[r74e4d219b24ec6d6f28d663c0285418e6c8e122e] is the first bad commit
commit [r74e4d219b24ec6d6f28d663c0285418e6c8e122e]
Author: Mike Solomon <mike@apollinemike.com>
Date:   Tue Mar 5 21:03:55 2013 +0100

    Uses only unpure-pure containers to articulate unpure-pure relationships (issue 3199)
   
    Previously, LilyPond used several different lists in define-grobs.scm
    to define relationships between unpure and pure functions.  This patch
    eliminates these lists, using unpure-pure containers to articulate
    these relationships.
   
    The modifications required to implement this change are described below:
   
    -) axis-group-interface.cc
    A Scheme function is no longer needed to determine pure relevant grobs.
    All grobs can have their Y-extents meaningfully pure-evaluated now. The
    worst-case scenario is that they evaluate to false. Dead grobs, on the
    other hand, are never pure relevant. The calls to is_live are the only
    holdovers from the old pure-relevant? scheme function.
   
    -) grob-closure.cc
    We allow unpure-pure containers in simple closures.
   
    -) grob-property.cc
    call_pure_function no longer looks up a Scheme module. Because there are
    no hard-coded lists in Scheme any more, the function is smaller and
    writing it in C++ gets slight efficiency gains.
   
    -) grob.cc
    pure_stencil_height used to be a Scheme function in define-grobs.scm.
    Because it is much simpler (it no longer makes references to lists defined
    in Scheme), it can be implemented in C++.
   
    -) pure-from-neighbor-engraver.cc
    Similar to axis-group-interface.cc, the pure-relevant distinction is
    no longer important.
   
    -) side-position-interface.cc
    In order to avoid issues with alterBroken, we only check pure properties
    before line breaking.
   
    -) simple-closure.cc
    Simple closures were incorrectly evaluated when containing unpure-pure
    containers. This rectifies that.
   
    -) stencil-integral.cc
    Several pure equivalent functions needed to be written for skylines.
   
    -) define-grobs.scm
    Multiple overrides must be changed to unpure-pure containers. Previous
    hard-coded lists are all deleted and several functions moved to C++ (see
    above).
   
    -) output-lib.scm
    Several common unpure-pure containers used in define-grobs.scm are
    defined here. Several functions from define-grobs.scm pertaining to
    grob offsets are moved to this file.

:040000 040000 [r69f10e267f63d1c3315e91557e9a61658a04c53b] [r7f47f318dd89f2e1768860293c1ed86c1fa23114] M    input
:040000 040000 [r7d0fce689cff49901f19d3ba182f253f6c27338b] [r4739327adefbb80fee3aa1e3c5b36a1ade66f595] M    lily
:040000 040000 [r1c9e860a272a4b38f36526023369319afd97820d] [r3ff043eb2e0349161f551d615ddbc2ee13387a2c] M    ly
:040000 040000 [r2cfacbdbf1649347fd715b20a1d3afbdc15deb8e] [rdcc4ac6d62edc606de038cac934da023971574d1] M    scm

This is in version 2.17.14

Labels: Type-Critical

[Google Importer]

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

concerning comment nr. 1: I didn't get all these messages about

cyclic dependency: calculation-in-progress encountered for #'direction...

do you have special settings?

[Google Importer]

Originally posted by: dak@gnu.org

No special settings as far as I can see.

[Google Importer]

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

interesting - all I get is:

\version "2.17.14"

for future compatibility
Interpreting music...
Preprocessing graphical objects...
programming error: Grob direction requested while calculation in progress.
continuing, cross fingers
programming error: Grob direction requested while calculation in progress.
continuing, cross fingers
programming error: Grob direction requested while calculation in progress.
continuing, cross fingers
Finding the ideal number of pages...
Fitting music on 1 page...
Drawing systems...
Layout output to `2.17.14.test2.ps'...
Converting to `./2.17.14.test2.pdf'...
Success: compilation successfully completed

with windows 7-64

[Google Importer]

Originally posted by: dak@gnu.org

Uh, where is the contradiction?  Your output is for version 2.17.14 where error messages started appearing.  The output I quoted is for version 2.17.19 where the error symptom is acerbated and more programming errors get reported.

[Google Importer]

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

here it is:

GNU LilyPond 2.17.19
Processing `test2.ly'
Parsing...
Interpreting music...
Preprocessing graphical objects...
programming error: Grob direction requested while calculation in progress.
continuing, cross fingers
programming error: Grob direction requested while calculation in progress.
continuing, cross fingers
programming error: Grob direction requested while calculation in progress.
continuing, cross fingers
Finding the ideal number of pages...
Fitting music on 1 page...
Drawing systems...
Layout output to `test2.ps'...
Converting to `./test2.pdf'...
Success: compilation successfully completed

[Google Importer]

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

2.17.19 via installer returns only several:
  programming error: Grob direction requested while calculation in progress.

Self-compiled 2.17.20 (from latest master) returns David's result.

[Google Importer]

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

(No comment was entered for this change.)

Labels: Regression
Status: Accepted

[Google Importer]

Originally posted by: k-ohara5...@oco.net

Previous versions let the beam cross the ottava bracket, but with some reason.

Whether to knee the beam depends (in the general case) on staff spacing, and staff-spacing could be affected by the placement of the ottava bracket.

Version 2.16 places the ottava bracket relative to the staff first, ignoring the cross-staff beam but fitting over anything else, then does staff-spacing, then sets the beam.

By contrast, with Scripts like in issue 3363, earlier version would mark the Script as cross-staff so that it would be ignored for staff-spacing, and then place the script after the beam is set.

The unpure-pure-containers patch keeps track of whether an object 'aligns-to-cross-staff' so maybe that is supposed to set scripts.

I'll revert the patch that caused the visible change.

%% Image
<<
  \new Staff = "upper" {
  \stemUp\mark "2.16"
  \ottava #1 e'''8
  \change Staff = "lower"
  e''8
  \change Staff = "upper"
  [r4] \ottava#0 s2 }
  \new Staff = "lower" { s2
  \ottava #1 c'''8
  \change Staff = "upper"
  e''8
  \change Staff = "lower"
  [r4] \ottava#0 }
>>

[Google Importer]

Originally posted by: k-ohara5...@oco.net

commit [rd6a5ab877843613c9059d93fb2ffcbae8ff5f30d]

The error messages are still there,
"programming error: Grob direction requested while calculation in progress."
but the output is correct.

Labels: Fixed_2_17_20
Status: Fixed

[Google Importer]

Originally posted by: m...@mikesolomon.org

My immediate gut reaction is that the OttavaBracket grobs in the above example, or any grob whose alignment depends on that of multiple vertical axis groups, needs to be marked cross staff.  In general, any grob using:

(Y-offset . ,side-position-interface::y-aligned-side)

can theoretically be cross-staff and needs to have:

(cross-staff . ,ly:side-position-interface::calc-cross-staff)

or a variant.

[Google Importer]

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

Checks cross staff for all side-positioning grobs

http://codereview.appspot.com/10034048

Labels: Patch-new
Owner: mts...@gmail.com
Status: Started

[Google Importer]

Originally posted by: m...@mikesolomon.org

Uploaded a patch - I'm running on battery today so it's tough for me to put resources to testing it but it implements a crude form of the idea above.

[Google Importer]

Originally posted by: k-ohara5...@oco.net

Marking the 8va-bracket as "cross-staff" means to delay its final placement until after staff-spacing, and that it might be clipped in the .eps output.  Would that move the second 8va-bracket in comment 11 above both staves?

In the (slightly artificial) second example in comment 11, I thought we would want to place the 8va-bracket clear of a tentative estimate of the stem, then space staves allowing room for the 8va-bracket, and then set the stems and beams given the chosen spacing between staves ... someday teaching LilyPond to try to avoid the 8va bracket when setting the beam.

[Google Importer]

Originally posted by: dak@gnu.org

What would it take to include "cross-staff" material into the system stencil dimensions once the staffs and staff groups have been assembled into a system?  Presumably the same mechanism should be used for other material that shows a tendency to disappear from EPS.

[Google Importer]

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

This patch cannot be applied to current master.

Labels: -Fixed_2_17_20 -Patch-new Patch-needs_work

[Google Importer]

Originally posted by: k-ohara5...@oco.net

I don't know what it would take to include "cross-staff" material into the system dimensions, after page layout, for better .eps and .png bounding boxes.  It certainly seems possible, because the layout can be done before .eps written.

Currently (master, and expected version 2_17_20) the output is good (image left) and we have only the programming error messages.

Applying the comment-14 patch by hand clears the messages, but allows the 8va-bracket to cross the next staff in a variation where there is one (image center).  Former versions would let the beams collide with the 8va-bracket, but allow for the bracket when spacing staves (image right).  (I like the old way.)

[Google Importer]

Originally posted by: dak@gnu.org

Are systems assembled from staves "after page layout"?  Namely, will there be stretching between staves of a single system in order to accommodate the page size?

At any rate, I agree with you(?) that the middle image (from making 8va cross-staff) is not what we want.

[Google Importer]

Originally posted by: googlegr...@ensemble101.fr

Picking this back up a bit - is this still critical?
I'm getting programming errors but good output.  The programming errors are thorny - cross-staff issues with beams are really tricky and may take several stable versions to iron out completely.

[Google Importer]

Originally posted by: dak@gnu.org

I don't know what you call "stable versions" but if you are talking about 2.18: the stable versions are supposed to be the _result_, not the starting point of ironing out problems.  The purpose of development versions is not to accumulate unresolved problems but rather provide a starting point where features can get a real-world testing _after_ they are considered in a state suitable for wider distribution.

If a programming error is an indication of an unexpected/unresolved condition, it is not particularly reassuring when the graphics turn out inconspicuous.  For one thing, one does not know whether the result corresponds to the intended result with regard to all parameters (like beam slope).  For another, anything that could equally well be wrong will likely be wrong under some different starting conditions.

Which is exactly what happened triggering this issue, so it is overly optimistic to assume that this won't happen.

[Google Importer]

Originally posted by: k-ohara5...@oco.net

The reason to fix this now is that we might still remember he intent of the patch for issue 3199:
"  A Scheme function is no longer needed to determine pure relevant grobs.
   All grobs can have their Y-extents meaningfully pure-evaluated now. The
   worst-case scenario is that they evaluate to false."

The 'pure-relevant' grobs are the graphical objects that are included (relevant) in the estimated (pure) height of the staff---the height used to choose page-breaks.  Now that all items on the staff are included, the question is: what should we use for their estimated (pure) Y-extents ?

In the example at the top, we do not yet know if the beams should knee, but want to estimate the height of the staves.  What do we want to use for estimated (pure) Y-extents of the stem, beam and 8va bracket ?

I suggest,
cross-staff beam: false (do not count it, possibly letting it overlap another staff)
cross-staff stem: the union of Y-extents of un-beamed stems pointing up and down
8va bracket: the Y-extent it would have if placed over the stem extents estimated just above

[Google Importer]

Originally posted by: googlegr...@ensemble101.fr

In general, err on the side of too much.  It'd take me a bit long to dig into the code, but if there's a patch floating around I'll have time to review it all this week.

[Google Importer]

Originally posted by: m...@mikesolomon.org

WIth respect to your suggestions:

cross-staff beam: false (do not count it, possibly letting it overlap another staff)
Great idea.
cross-staff stem: the union of Y-extents of un-beamed stems pointing up and down
This is more or less what we're currently doing, minus the pointing up and down part.  Also a great idea.
8va bracket: the Y-extent it would have if placed over the stem extents estimated just above
Great idea.

Can you propose a patch doing this?