Possible regression with box-markup/rounded-box-markup

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Possible regression with box-markup/rounded-box-markup

Thomas Morley-2
The gap from a box-markup and a rounded-box-markup to the Staff
differs for unknown reason.

\version "2.18.2" %% up to current master

{
  c''^\markup \box 1
  c''^\markup \rounded-box 1
  c''_\markup \box 1
  c''_\markup \rounded-box 1
}

2.16.2 and before return same gap.

Regresion?

Attached images for 2.16.2 and 2.18.2

Cheers,
  Harm

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond

boxes-test-2-16-2.png (6K) Download Attachment
boxes-test-2-18-2.png (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Possible regression with box-markup/rounded-box-markup

Torsten Hämmerle
In any case, the additional space around the \rounded-box is proportional to
the corner-radius.
If you set corner-radius to 0, there's no gap anymore. As the corner-radius
increases, so does the gap.

I think it's the rounded-box-stencil's dimensions:
when explicitly setting them to xext/yext, everything works as expected (w/o
the extra gaps).

I suspect ly:round-filled-box, as rounded-box-stencil itself hasn't changed.

All the best,
Torsten



--
Sent from: http://lilypond.1069038.n5.nabble.com/Bugs-f58488.html

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond
Reply | Threaded
Open this post in threaded view
|

Re: Possible regression with box-markup/rounded-box-markup

Thomas Morley-2
2018-03-28 21:09 GMT+02:00 Torsten Hämmerle <[hidden email]>:

> In any case, the additional space around the \rounded-box is proportional to
> the corner-radius.
> If you set corner-radius to 0, there's no gap anymore. As the corner-radius
> increases, so does the gap.
>
> I think it's the rounded-box-stencil's dimensions:
> when explicitly setting them to xext/yext, everything works as expected (w/o
> the extra gaps).
>
> I suspect ly:round-filled-box, as rounded-box-stencil itself hasn't changed.
>
> All the best,
> Torsten



Hi Torsten,

thanks for your findings.
I meanwhile identified the first bad commit:

commit 28f3294954eff1f263d3b2e3de1c520f4d2fbdfc
Author: Mike Solomon <[hidden email]>
Date:   Mon Aug 27 23:47:04 2012 +0200

    Improvements in vertical skyline approximations (issue 2148).

    The file stencil-integral.cc provides a suite of functions that
    traverse a stencil and do linear approximations of its components.
    These are then turned into boxes that are passed to the Skyline
    constructor. This approximation is used for several vertical skylines
    including those of VerticalAxisGroup and System. As a result of these
    more accurate approximations, vertical spacing is more snug between
    grobs.

    Additionally, in axis-group-interface.cc, skylines of grobs are no
    longer compared to a monolithic axis-group skyline but rather all
    of the component skylines of the axis-group, allowing grobs to
    be fit under other ones if there is space instead of always shifted over.

    Two new python scripts allow to visualize the position of skylines.

    All other changes provide functions that allow for better debugging
    of Skylines, better approximations of grobs via skylines, and changes
    to the measurement of distance between grobs via the new Skyline API.

    This results in a significant time increase in score compilation for
    objects with complex skylines such as all text grobs.  For orchestral
    scores, the increase is not as steep.

https://sourceforge.net/p/testlilyissues/issues/2148/
https://codereview.appspot.com/5626052/

Unfortenately this is Mike's major skylines-patch.

With a quick glance over it, I found some code for
make_round_filled_box_boxes
Probably  a point to look at, but it's mostly work in C++, so I'm off :(

I had to revive old LilyDev3 to compile old (2.17.) versions, i.e. with
harm@debian ~$ gcc --version
gcc (Debian 4.7.2-5) 4.7.2
(in case someone wants to recheck)


Thanks,
  Harm

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond
Reply | Threaded
Open this post in threaded view
|

Re: Possible regression with box-markup/rounded-box-markup

Torsten Hämmerle
Hi Harm,

I think I've got it...

You are perfectly right, there is a slight inconsistency in
lily/stencil-integral.cc
C++ code.
(I just have to get it completely right, still experimenting a bit...):

The C++ code obviously confuses line thickness and blot diameter and and
"corrects" (i.e. enlarges) the frame by half the line thickness (in reality:
half the corner-radius).
I'll have to check the complete chain of calls of calls of calls, but when
simply removing the extra shift of the cornering points p0 (left, bottom)
and p1 (right, top), that's what comes out using our original example (with
TextScript.outside-staff-padding set to 0):

{
  \override TextScript #'outside-staff-padding = 0
  f''2^\markup \box "1" ^\markup \box "2"
  f''2^\markup \rounded-box "1" ^\markup \rounded-box "2"
}

<http://lilypond.1069038.n5.nabble.com/file/t3887/rounded-box-2.png>

All the best,
Torsten





--
Sent from: http://lilypond.1069038.n5.nabble.com/Bugs-f58488.html

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond
Reply | Threaded
Open this post in threaded view
|

Re: Possible regression with box-markup/rounded-box-markup

Thomas Morley-2
In reply to this post by Torsten Hämmerle
Hi again,

2018-03-28 21:09 GMT+02:00 Torsten Hämmerle <[hidden email]>:
> In any case, the additional space around the \rounded-box is proportional to
> the corner-radius.
> If you set corner-radius to 0, there's no gap anymore. As the corner-radius
> increases, so does the gap.

Setting corner-radius zero gives a warning. But in general I can
confirm, within certain borders.

> I think it's the rounded-box-stencil's dimensions:
> when explicitly setting them to xext/yext, everything works as expected (w/o
> the extra gaps).

Here I can't reproduce it, I probably didn't understood what you've done.

> I suspect ly:round-filled-box, as rounded-box-stencil itself hasn't changed.


Btw, this example gives terrible output:

#(ly:set-option 'debug-skylines #t)

{
  \set Staff.instrumentName = \markup #(lilypond-version)
  c''4^\markup \box 1
  c''^\markup \rounded-box 1
  c''_\markup \box 1
  c''_\markup \rounded-box 1
}

Cheers,
  Harm

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond

boxes-test-2-21-skylines.png (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Possible regression with box-markup/rounded-box-markup

Torsten Hämmerle
Yes, and your terrible example looks like

<http://lilypond.1069038.n5.nabble.com/file/t3887/rounded-box-harm-2.png>

in LilyDev with my slight corrections applied.
Seems OK to me.

Cheerio,
Torsten




--
Sent from: http://lilypond.1069038.n5.nabble.com/Bugs-f58488.html

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond
Reply | Threaded
Open this post in threaded view
|

Re: Possible regression with box-markup/rounded-box-markup

Thomas Morley-2
2018-03-28 22:43 GMT+02:00 Torsten Hämmerle <[hidden email]>:
> Yes, and your terrible example looks like
>
> <http://lilypond.1069038.n5.nabble.com/file/t3887/rounded-box-harm-2.png>
>
> in LilyDev with my slight corrections applied.
> Seems OK to me.



Indeed, the image looks fine.
I really hope there's no impact on other stuff...

Cheers,
  Harm

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond
Reply | Threaded
Open this post in threaded view
|

Re: Possible regression with box-markup/rounded-box-markup

Torsten Hämmerle
Hi Harm,

The good news:
The correction is just changing the skyline cornering points in
make_round_filled_box_boxes, nothing else.

The bad news:
make_round_filled_box_boxes is very much in use (e.g. for ledger-lines) and
so changing/correcting these skylines will have an effect on basically all
the skyline of any line or box with rounded corners, and there are piles of
them.
After all, a skyline should closely encompass the object and padding should
be done exculsively by, well, padding.

But in any case, I'll have a closer look, run some regression tests and,
ideally will even try to take care of the rounded corners (at least for
larger corner-radii) over Easter.

The extreme vertical shift in your example is still there if we increase the
line thickness (or the box-padding value), but this is standard behaviour
caused by outside-staff-horizontal padding if the objects come too close.

All the best,
Torsten



--
Sent from: http://lilypond.1069038.n5.nabble.com/Bugs-f58488.html

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond
Reply | Threaded
Open this post in threaded view
|

Re: Possible regression with box-markup/rounded-box-markup

Torsten Hämmerle
In reply to this post by Thomas Morley-2
I've now started  #5307:Skyline Refinements (Rounded Boxes and Rotated
Ellipses) <https://sourceforge.net/p/testlilyissues/issues/5307/>  

It also takes care of rotation issues (both boxes and ellipses) and even
produces skylines with rounded corners.

Special case: no rotation, small corner radius -> still uses one single and
simple skyline box.
This is particularly important for the most common and frequent rounded
boxes: ledger lines.

Current status:
<http://lilypond.1069038.n5.nabble.com/file/t3887/skylines-boxes-2-21-0.png>

All the best,
Torsten




--
Sent from: http://lilypond.1069038.n5.nabble.com/Bugs-f58488.html

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond
Reply | Threaded
Open this post in threaded view
|

Re: Possible regression with box-markup/rounded-box-markup

Thomas Morley-2
2018-04-16 1:03 GMT+02:00 Torsten Hämmerle <[hidden email]>:

> I've now started  #5307:Skyline Refinements (Rounded Boxes and Rotated
> Ellipses) <https://sourceforge.net/p/testlilyissues/issues/5307/>
>
> It also takes care of rotation issues (both boxes and ellipses) and even
> produces skylines with rounded corners.
>
> Special case: no rotation, small corner radius -> still uses one single and
> simple skyline box.
> This is particularly important for the most common and frequent rounded
> boxes: ledger lines.
>
> Current status:
> <http://lilypond.1069038.n5.nabble.com/file/t3887/skylines-boxes-2-21-0.png>
>
> All the best,
> Torsten

Hi Torsten,

great you're working on it.
I've already seen before/after images at sourceforge.
Very nice!

Thanks,
  Harm

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond