Menu
▾
▴
1 Attachments
#5307 Skyline Refinements (Rounded Boxes and Rotated Ellipses)
http://codereview.appspot.com/341140043
Recently, there have been some discussions about mysterious spacing of \rounded-box markup.
Harm identified skyline problems as the cause and submitted the following bug report:
Possible regression with box-markup/rounded-box-markup
Before Mike Solomon's new skyline handling, there were no such uninteded extra margins.
Rounded Box Skylines
The skyline of boxes with rounded corners do not tightly wrap the boxes but there's an unintentional margin (as wide as the corner radius).
Apart from that here are a few other shortcomings and so I have decided to create a more general box-and-rotated-ellipses skyline issue.
Other issues
When working on this case, I came across a few other things that call for improvement and so I've included them into the test file. Most notably, there is a general problem with rotation. And it would be nice to recreate the rounded corners for large boxes/radii.
At the same time, one should keep run-of-the-mill stuff like ledger lines as simple and fast as possible.
Test file
A file with 'debug-skylines switched on and TextScript padding values set to 0 in order to be able to check how the markup objects are fitted together.
I have tried to demonstrate about everything you cold do with boxes (including scaling, rotating, cominations thereof).
#(ly:set-option 'debug-skylines #t)
{
\set Staff.instrumentName = \markup #(lilypond-version)
\override Staff.TextScript.padding = 0
\override Staff.TextScript.outside-staff-padding = 0
\override Staff.TextScript.outside-staff-horizontal-padding = 0
\override Staff.TextScript.self-alignment-X = #CENTER
\override Staff.TextScript.layer = 0
f'''4
c'' ^\markup \override #'(thickness . 4) \box \teeny 1
_\markup \with-color #grey \filled-box #'(-1 . 4) #'(1 . 4) #1
b' ^\markup \rounded-box "2!" ^\markup \rounded-box "3"
d'' ^\markup \circle "*" ^\markup \rotate #90 \ellipse "Elli"
_\markup \with-color #grey \rotate #10 \filled-box #'(-2 . 5) #'(1 . 4) #10
_ \markup \rotate #36 \override #'(corner-radius . 20) \scale #'(1 . 0.4)
\rotate #-68 \scale #'(1.6 . 1) \rounded-box "dop"
f'' _\markup \override #'(corner-radius . 0.5) \rounded-box "."
a' ^\markup \rotate #30 \rounded-box "rotated a"
_\markup \scale #'(1 . 1.5) \rotate #45 \override #'(thickness . 5)
\rounded-box \rotate #-45 \scale #'(2 . 1) \bold "!"
d'' _\markup \override #'(corner-radius . 20) \rounded-box "4"
a'' ^\markup \rotate #-165 \box "rotated b"
_\markup \rotate #180 \override #'(corner-radius . 20) \rounded-box \teeny
\override #'(baseline-skip . 0) \center-column { "Upside" "Down" }
}
(see attached PNG file)
- Ledger lines are filled rounded boxes, too (!)
- Corner radius is misinterpreted as line thickness and padded around the box.
- Ledger lines have rounded corners, but the blot diameter is so small that the effect didn't disturb too much.
- When rotating boxes, the skylines still consist of one single non-rotated rectangular box.
- Rounded corners are not replicated in the skylines, even if this may be beneficial for bigger radii.
- Ellipse skylines gradually deteriorate when being rotated. In the example, the skyline of ellipse "Elli" has completely collapsed because of the 90° rotation.
- The rhombus "!" has been created by rotating and scaling that unfortunately caused a collapse of the skyline.
- Rotations in general cause large skyline boxes that hamper object placement a lot.
Discussion
Rotations by anything than multiples of 90° cannot work revertably for either bounding boxes or skylines. They could be made to work for convex hulls and/or complete outlines. Convex hulls would allow tighter placement than bounding rectangles but would work worse than skylines for a number of cases. Finding some rotatable hull definition that isn't constrained to convexity without requiring a full outline would be challenging.
Hi David,
I'm not rotating any skylines.
It's just a problem of quantisation, i.e. intermediate points:
Basically, ** the number of points** for drawing the ellipse is calculated from the circumference by scaling the radii, but currently, we just multiply the x-radius by the trans.xx value etc.. This does not work anymore as soon as there is any rotation involved, because this is nothing but the projection on the x-axis.
In a 90° rotation, currently the radii collapse to length 0, so does the circumference of the ellipse and we get 0 quantisation points and, consequently, the skyline collapes.
When using the full transformation matrix (i.e. yx and xy values as well) for calculating the lenth of the transformed radii, everything will work fine.
All the best,
Torsten
Before uploading the patch, here's the new rendering of the above file:
All the best,
Torsten
Veeery nice :-)
(1) Rotating Ellipses
This was rather a side-effect when working on boxes with rounded corners.
Ellipse skylines are built out of a number of rectangular boxes and the number of boxes needed is depending on the circumference: the longer the line, the more intermediate points are needed.
Where's the problem in the current code?
The calculation of the circumference takes care of scaling, because, naturally, a larger ellipse needs more intermediate points than a smaller one.
In the current circumference calculation, the radii (in x and y direction) are scaled by just using the xx resp. yy value of the transformation matrix, which actually results in the projection on the x or y axis.
If there is any rotation involved, the calculation of the scaled radii is wrong because using the projection on one axis only gives back a shortened radius length.
Consequently, the estimated circumference is too short, even becoming 0 when 90° are approached, so that rotating decreases the number of points used for drawing the skyline.
But the circumference of an ellipse does not change when just being rotated.
There is a PNG file attached comparing the original scale factors (2.19.81) and corrected scale factors (from my 2.21.0 development branch). You can see how the number of intermediate points decreases as the angle increases, resulting in a total collapse of the skyline.
Solution
Use the length of the transformed x/y unit vectors as scaling factors. With the corrected scale factors, the skylines are stable and consistent even when rotated.
(2) Simple Boxes
Admittedly, the vast majority of boxes is neither rotated nor has it rounded corners with considerable radius. The most prominent example of round filled boxes probably are ledger lines.
Therefore, I consider it important to keep the simple cases simple.
Consequences
A Ledger line skyline (standard corner diameter 0.2 staff spaces) are still drawn as one single box.
Where does the "extra margin" come from?
The "extra margin" around rounded boxes that messed up spacing is caused by a misunderstanding:
Rounded boxes are defined by their total width (left, right), their total height (bottom, top), and the diameter of the corner circles.
In the current inmplementation, the diameter of the corner circles has been misinterpreted as a line thickness (variable th) and therefore, half of it is added around the box. This is particularly obvious for \rounded-box markups (and less for comparatively tiny ledger lines).
(3) Boxes with more detailed outlines
Straight lines
The straight lines are drawn as one simple boxes each (if horizontal).
If they are sloped, they will be drawn as buildings (cf. sloped beams) in order to get nice and
smooth lines without unneccessary staircase optics. Therefore, I had to pass over a building parameter to the make_round_filled_box_boxes function (all the others already have it, the simplistic box sklyine was the only exception).
Rounded corners
Rounded corners (if applicable) are drawn using the boxy skyline technique.
For rotated boxes, the transition from straight lines to rounded corners may look a bit rugged when displaying the skylines in debug-skylines mode, but it nevertheless is a good approximation of the actual rounded box outline.
issue 5307: Skyline Refinements (Rounded Boxes and Rotated Ellipses)
lily/stencil-integral.cc
function make_partial_ellipse_boxes:
Use correct scaling factors for x_rad and y_rad for an arbitrary
transformation matrix
This affects calculation of quantisation value
function make_round_filled_box_boxes:
(1) remove unintentional additional space between box and skyline
(2) In case of rounded corners (threshold: diameter > 0.5 staff spaces)
or rotation, a detailed skyline will be constructed.
Rotated straight lines are drawn using buildings.
Rounded corners built out of boxes (cf. ellipse).
(3) Draw simple single-box skyline for simple boxes (i.e. in most cases,
e.g. ledger lines).
http://codereview.appspot.com/341140043
Diff:
Wrap lines, consistent quantization/radius
http://codereview.appspot.com/341140043
Passes make, make check and a full make doc.
Reg test diffs can be downloaded from here:
https://cloud.indie.host/s/T8r3rKBMqKLgwYr
Passes make, make check and a full make doc.
Reg test diffs can be downloaded from here:
https://cloud.indie.host/s/T8r3rKBMqKLgwYr
Thank you, James!
The various reg test diffs show how important boxes are to LilyPond... ;)
The slight and barely noticeable unintentional extra space that has been removed from rounded-corner box skylines causes many tiny shifts (as changed skylines change spacing).
Just to give one obvious example (a fret diagram regtest) of why and how the unintentional extra padding, small as it may be, had an influence on the skylines and therefore on object placement: Fret diagrams are made out of lines (i.e. boxes) with rounded corners and the scaled-up detail PNG shows (marked by green arrow) how a skyline is dented by a slightly oversized skyline and the letter "C" is shifted a wee bit up compared to the new rendering (right).
This all results in lots of bluish-green regtest differences that remind me of Joni Mitchell's "A Case of You":
"... in the blue TV screen light I drew a map of Canada..."
All the best,
Torsten
Patch on countdown for April 27th.
Patch counted down - please push.
Hi James,
I've attached a patch file (against current master).
Please be so kind as to push it to staging for me as I do not have push access.
Many thanks,
Torsten
I hope it's ok I've pushed this instead of James :)
commit df0ca89d232ca1811605c0a5acc753b6783d8293
Author: Torsten Hämmerle torsten.haemmerle@web.de
Date: Mon Apr 16 00:35:08 2018 +0200
Yes, perfectly ok, it didn't do any Harm... ;)
Thanks a ton,
Torsten