Re: Issue 3330: redo much of the stencil stacking/spacing/empty-check (issue 8869044)

[dak]

Please try indicating the quoting levels.  This is a bit confusing
otherwise.  Fixing in the reply

On 2013/05/07 06:03:07, Keith wrote:
> On Mon, 06 May 2013 03:21:18 -0700, David Kastrup <mailto:address@hidden>
wrote:

> > "Keith OHara" <mailto:address@hidden> writes:

> > > Oh. I thought it was good.  If the possibly-backspaced cursor is
> > > forgotten when \line is finished, then the user doesn't have to
> > > remember where the cursor ended up.

> > Why would he backspace when he doesn't mean it?

> I was thinking of something like this, where you want to backspace to
place a
> graphic, but then use the whole extent of the overlaid text for
spacing the line
> :

> \markup { A
>    \line {reverse turn \hspace#-5 \raise #2 \musicglyph
#"scripts.reverseturn" }
>    is found in the original manuscript.}

Uh, this seems rather haphazard.  Do you really think a user would
expect a backspace to allow him to position only the following markup,
jumping back to the right afterwards?  So that with

\markup { x \hspace #-20 hello dolly }

"hello" is to the left of "x", but "dolly" to the right?  That would
seem like a rather strange expectation.  It is not what the current
code does either, by the way.  Both examples look just the same after
the patch.  It is true that the behavior of ly:stencil-combine-at-edge
has changed from the behavior you describe to the behavior used in
\line.

> Another case would be

>   symbol = \markup { \hspace#-1 \char ##x2230 }

Again, both with the current version and the current proposal, the
output for

    symbol = \markup { \hspace#-1 \char ##x2230 }

    \markup \line { a b c \symbol d e f }
    \markup \column { a b \symbol }

is identical (namely overlap).  With

    symbol = \markup { \null \hspace#-1 \char ##x2230 }

    \markup \line { a b c \symbol d e f }
    \markup \column { a b \symbol }

again both before-and-after version give the same rendition, the one
that you describe.

So your objections don't hold water where the behavior of \line is
concerned.  The only situation where they start making sense is when
ly:stencil-combine-at-edge is used in isolation.  In that case, the
equivalent of the \symbol code without \null in it is like \line
_with_ \null in the current code, and \line _without_ \null in the
proposal.

That use case is the one actually worrying me.  Currently around 60
regtests appear different.  This number can only be marginally reduced
using the (finally working but not yet submitted) \vline fix.  Some of
them are due to "we should likely keep those" changes: a horizontal
line stacked vertically is currently having its height of half a line
width ignored in stacking while its depth is being registered.  That
does not even make sense.

Introducing a primitive different from ly:stencil-combine-at-edge
would have the advantage of keeping the regtest differences down.  But
one would have to scour the code base for instances where
ly:stencil-combine-at-edge is being used for a situation that would be
more appropriately handled by ly:stack-stencils (or whatever),
changing those individually.  Which is likely 3/4 of the uses of
Stencil::add_at_edge in our code base.

Unfortunately, not 100%.  There are a few differences not accounted
for yet.  The bad dot spacing was just lunatic code, explicitly adding
offsets to a stencil that it relied on being ignored.  It is
conceivable that some other instances are similar, but that does not
cover 100%.

> > > It looks very good that you made 'empty' mean (+inf . inf) so that
> > > negative extents don't count as empty.  But then defining
'half-empty'
> > > complicates things again, and I have not found a use for it.

> > Uh, reality check?  It fixes the hspace/concat bug you encountered
> > elsewhere in a clean manner, it lets page numbers in table of
contents
> > actually appear flush to the right (like the page number in the page
> > headings does), it makes \hspace add a predictable amount of space,
it
> > makes indentations at the start of auto-filled paragraphs remain
rigid
> > rather than being expanded along with other spaces...

> Any way of flagging the stencil made by \hspace as purely spacing,
> so that it doesn't get 'word-space around it, would have those nice
> effects.

So what information is stored in a stencil?  The stencil expression,
and the two stencil extents, each being a pair of numbers.  How do we
flag "spacing-purely"?  An obvious way would be to set the stencil
expression to '() for that while still storing spacing information in
the extents.

It turns out ly:stencil-empty? returns #t without looking at the
extents when the stencil expression is '().  Changing the respective
test to look at the extents only causes changed regtests: there is
code that relies on the stencil expression '() being sufficient for
flagging ly:stencil-empty? as true.  And there is quite a bit of code
that just throws away expressions marked ly:stencil-empty?.

So saying "spacing is an expression of '()" does not work compatibly.
"spacing is an expression of "" " seems rather worse since it would
apply, for example, to \markup "" which is _not_ pure spacing.

\hspace #x naturally has an empty Y extent, \vspace #y naturally has
an empty X extent.  So the condition that is being checked is quite
natural.

Whether the check should be (ly:stencil-empty? st Y) rather than
(ly:spacing? st X) and what (ly:spacing? null-stencil X) should then
return is another question.

> Half-empty extents are an awkward way to flag the stencil,

Not really.  They are a natural property of spacing.  It may just be
an awkward way to _check_ for them.

> especially when its empty stencil-expression is a more-easily
> understandable flag.

I tried with that first, but it seems like quite a larger can of
worms.  For one thing, if somebody explicitly writes

(ly:make-stencil "" '(0 . 4) '(0 . 0))

which is basically the old \hspace sans spacing correction, its
behavior will stay the same with this code.  Similar for

(ly:make-stencil "" '(0 . 0) '(0 . 4)).

I consider that an advantage.  Anything starting with

(ly:make-stencil '() ...

is currently being ignored in more contexts than one would want to.
For that reason, people are unlikely to have been using this with any
expectation other than it getting not heeded universally.

> Your comments at the start of Stencil::add_at_edge() seem to
> indicate that you find the interface awkward, and I would agree, so
> I was pushing for other solutions.  I'll stop.

As I said: a sensible solution to me appears to create a primitive
separate from Stencil::add_at_edge aka ly:stencil-combine-at-edge for
the purpose of stacking stencils, and use that consistently.

That should provide reasonable compatibility with past uses of both
ly:stencil-combine-at-edge as well as \line, while still providing
most of the advantages of a unified code base.

But most of the "other solutions" you are pushing for seem worse than
what they are supposedly fixing, and your examples very much suggest
that they would not actually be fixing anything that you think they
should.


https://codereview.appspot.com/8869044/