issue 5307: Skyline Refinements (Rounded Boxes and Rotated Ellipses)

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).

[Be-3, 2018-04-20 22:52:53]

Please review...

Thanks
Torsten

[lemzwerg, 2018-04-21 04:38:38]

LGTM, thanks!

https://codereview.appspot.com/341140043/diff/1/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):

https://codereview.appspot.com/341140043/diff/1/lily/stencil-integral.cc#newc...
lily/stencil-integral.cc:410: int quantization = max (0, (int) (rounded *
(x_scale + y_scale) * radius * M_PI / QUANTIZATION_UNIT / 4));
[Please use a line length of at most 78 characters if possible.]

Is `max' really the right operation here?  Or to ask differently: Does a
negative value of `diameter' makes sense?  All other values on the right side
are non-negative...

Or maybe the first argument of `max' should be 1, as in function
`make_partial_ellipse_boxes'?

[Be-3, 2018-04-21 10:23:52]

Wrap lines, consistent quantization/radius

[Be-3, 2018-04-21 10:26:16]

> lily/stencil-integral.cc:410: int quantization = max (0, (int) (rounded *
> (x_scale + y_scale) * radius * M_PI / QUANTIZATION_UNIT / 4));
> [Please use a line length of at most 78 characters if possible.]
In the original coding, many lines exceed 78 characters (including the one you
mention here).
But you are right, of course, and I've wrapped some lines in my new resp.
changed coding.

> Is `max' really the right operation here?  Or to ask differently: Does a
> negative value of `diameter' makes sense?  All other values on the right side
> are non-negative...
> 
> Or maybe the first argument of `max' should be 1, as in function
> `make_partial_ellipse_boxes'?
Oh, yes, of course. 
That was a relic from earlier attempts and does not make too much sense, indeed.

I've now taken the quantization declaration in front of the "non-simple box"
coding,
omitted the superfluent max operation and *after* quantization has been
computed,
I set the radius value (to 0., if there's no quantization).
That way, an effective skyline radius of 0 will ensure that the straight lines
meet
in the corners and there is no need to draw "rounded" corners with a
quantization of 1.
In contrast to the ellipses, a box will still have an outline even with sharp
corners.

Thanks,
Torsten

[dak, 2018-06-09 20:56:34]

https://codereview.appspot.com/341140043/diff/20001/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):

https://codereview.appspot.com/341140043/diff/20001/lily/stencil-integral.cc#...
lily/stencil-integral.cc:393: for (vsize i = 0; i < (vsize) points.size () - 1;
i++)
Why would the last point not be transformed?

https://codereview.appspot.com/341140043/diff/20001/lily/stencil-integral.cc#...
lily/stencil-integral.cc:423: for (vsize i = 0; i < (vsize) quantization + 1;
i++)
Why not i <= quantization ?