balloon-text: robust with empty stencils; issue 4447

balloon-text: robust with empty stencils; issue 4447

[dak, 2015-06-20 06:50:50]

https://codereview.appspot.com/243610043/diff/1/lily/balloon.cc
File lily/balloon.cc (right):

https://codereview.appspot.com/243610043/diff/1/lily/balloon.cc#newcode94
lily/balloon.cc:94: b.widen (padding, padding);
In general, adding padding around an actually non-existing stencil seems like a
bad idea.  But in this particular case, the padding is for the boundaries of a
box.

Basically we have two options here: not draw a box at all (it would be my guess
that this is probably what happened previously when assertions were disabled) or
draw a box around the not-with-stencil anchor point.

Since the "balloon"/anchorpoint is connected with the inscription with another
line, I'd lean towards leaving the box off altogether rather than boxing the
reference point.

I haven't actually looked at the output: if my interpretation of the code is
wrong, holler.

[Keith, 2015-06-21 02:08:13]

On Fri, 19 Jun 2015 23:50:50 -0700, <dak@gnu.org> wrote:

> https://codereview.appspot.com/243610043/diff/1/lily/balloon.cc#newcode94
> lily/balloon.cc:94: b.widen (padding, padding);
>
> Basically we have two options here: not draw a box at all (it would be
> my guess that this is probably what happened previously when assertions
> were disabled) or draw a box around the not-with-stencil anchor point.

The former behavior leaked NaNs and corrupted spacing on the whole line.

This is for the situation where a user, through misunderstanding or oversight,
requested an indicator box be drawn around an object that does not actually
appear in print.  A tiny empty box says clearly "I heard you ask for a box, but
I didn't recognize or find what you wanted me to draw it around."

[dak, 2015-06-21 05:40:07]

"Keith OHara" <k-ohara5a5a@oco.net> writes:

> On Fri, 19 Jun 2015 23:50:50 -0700, <dak@gnu.org> wrote:
>
>> https://codereview.appspot.com/243610043/diff/1/lily/balloon.cc#newcode94
>> lily/balloon.cc:94: b.widen (padding, padding);
>>
>> Basically we have two options here: not draw a box at all (it would be
>> my guess that this is probably what happened previously when assertions
>> were disabled) or draw a box around the not-with-stencil anchor point.
>
> The former behavior leaked NaNs and corrupted spacing on the whole
> line.

Well, so I guessed wrong.

> This is for the situation where a user, through misunderstanding or
> oversight, requested an indicator box be drawn around an object that
> does not actually appear in print.

That's overinterpreting things.  If I ask for a pointer to the Stem of a
whole note, or the key signature of C major or similar, that does not
mean that this was in error.

The object _has_ a location (or we would not be in this code path) but
no glyph.

> A tiny empty box says clearly "I heard you ask for a box, but I didn't
> recognize or find what you wanted me to draw it around."

A tiny empty box says clearly "here is a point-stencil rather than an
empty one".  If we did not also have the line pointing to the element in
question, not drawing a box would leave the user with no visual
indication at all: I'd agree with your solution then.  But as it is, we
get a visual indication and thus can afford to differentiate between an
empty stencil and a point stencil.  And that distinction can be useful
as they do have significantly different behavior.

Either solution is a substantial improvement over what we have now, and
the code you propose is simple.  I haven't looked at the code in
question: if my proposal for dealing with a non-existing stencil (or
non-existing dimensions) is significantly more complex, I'd just file
another feature request for it.

-- 
David Kastrup