Menu

#5086 Fix dashed line errors

Verified
Enhancement
2017-04-02
2017-03-07
No

Issue 5086: Fix dashed line errors

There are two errors in lily/line-interface.cc which cause
dashed lines to be drawn inaccurately.

(1) Line-thickness is improperly subtracted from the "off"
value in Line_interface::make_dashed_line. This was done,
presumably, to account for the line cap of each dash. In fact,
PostScript discounts the line cap when constructing the pattern.

(2) Dashed lines are constructed so that they begin and end
with a dash. Thus, a dashed line is built of dash + whitespace
units and a last lone dash. Period is not adjusted in
Line_interface::line for this last dash, which can lead to a
noticeable difference in its length compared to other dashes.

Correcting these flaws ensures that dashed lines end with a
dash rather than whitespace, and that terminating dashes are
not clipped. Also, the number of dashes drawn reflects the
number calculated.

http://codereview.appspot.com/320320043

Discussion

  • David Nalesnik

    David Nalesnik - 2017-03-07

    There are a number of regtests (42) which show differences as a result of this patch. This is to be expected, as the effect is to draw dashed lines more accurately. Note, for example, that the horizontal line in ottavas always touches the end vertical, which was not always the case before.

     
  • David Nalesnik

    David Nalesnik - 2017-03-07

    Here's a before and after for this snippet:

    \header {
    ragged-right = ##t
    }

    {
    \override TupletBracket.style = #'dashed-line
    \tuplet 3/2 { c''2 d'' e'' }
    }

     
  • David Nalesnik

    David Nalesnik - 2017-03-07

    Hopefully, here are the images...

     
  • David Nalesnik

    David Nalesnik - 2017-03-07

    Here's the old:

     
  • Anonymous

    Anonymous - 2017-03-08
    • Description has changed:

    Diff:

    
    
    • Needs: -->
    • Patch: new --> review
     
  • Anonymous

    Anonymous - 2017-03-08

    Passes make, make check and a full make doc.

     
  • Anonymous

    Anonymous - 2017-03-10
    • Patch: review --> countdown
     
  • Anonymous

    Anonymous - 2017-03-10

    Patch on countdown for March 13th.

     
  • David Kastrup

    David Kastrup - 2017-03-13

    Not intended to hold up the countdown but something I've been slightly worrying about, so maybe double-check?

    (1) Line-thickness is improperly subtracted from the "off"
    value in Line_interface::make_dashed_line. This was done,
    presumably, to account for the line cap of each dash. In fact,
    PostScript discounts the line cap when constructing the pattern.

    LilyPond stencil metrics need not map 1:1 to PostScript: I think that there were some adjustments in this area either in the scm/output-ps.scm backend code or even in the definitions in ps/music-drawing-routines.ps.

    So it might be a good idea to add a regtest with quite thick lines and possibly some tic marks/rulers/arrows added that would make it easy to verify the assumptions visually.

     
    • David Nalesnik

      David Nalesnik - 2017-03-13

      @David Kastrup

      The following doesn't depend on behavior I've touched, but it should illustrate that LilyPond behaves consistently with what I've asserted about PS..

      Is this what you had in mind?

       
      • David Nalesnik

        David Nalesnik - 2017-03-13

        Perhaps some whitespace between lines would be better..

         
  • Anonymous

    Anonymous - 2017-03-13
    • Patch: countdown --> push
     
  • Anonymous

    Anonymous - 2017-03-13

    @David Nalesnik
    If you want to push this, go ahead and create a new tracker for a reg test as per David Kastrup's suggestion, or you can tag the reg test on this issue (in which case just make sure it goes to the correct state so I can spot it to test).

    I'll set this to 'Counted down please push' for now.

    James

     
  • David Nalesnik

    David Nalesnik - 2017-03-14
    • labels: --> Fixed_2_19_57
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -1,4 +1,4 @@
    -Fix dashed line errors
    +Issue 5086: Fix dashed line errors
    
     There are two errors in lily/line-interface.cc which cause
     dashed lines to be drawn inaccurately.
    
    • status: Started --> Fixed
    • Patch: push -->
     
  • David Nalesnik

    David Nalesnik - 2017-03-14

    Pushed to staging as commit ee1c498891ef92ebcd7d043b46e07ea2d3a9d3ae.

     
  • David Nalesnik

    David Nalesnik - 2017-03-14

    I've gone ahead and pushed this.

    I will try to open an issue for a regtest soon, when I've given some thought how to formulate an issue. I'm in a little doubt, however, over what needs to be tested. There are many dashed lines in the regtests already, and a comparison of changes shows a general improvement (for example, the horizontal line of ottavas connecting with the vertical stroke at the end, not always the case before, and the number of dashes drawn matching the number calculated). Also, LilyPond's use of Postscript to draw these lines seems straightforward, so my example further above is simply verifying something about PS itself with which LilyPond doesn't tamper.

     
  • Graham Percival

    Graham Percival - 2017-04-02
    • status: Fixed --> Verified
     
  • Graham Percival

    Graham Percival - 2017-04-02

    Verified, looks beautilful, thanks for the example! :)