bug#30553: 26.0.91; underline appears beneath line-spacing rather than beneath text

[Aaron Jensen <aaronjensen@gmail.com>]

On Mon, Feb 26, 2018 at 7:42 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> I'm not sure I see the point of adding yet another variable.  Don't
> you see the same problem with local values of that variable, like you
> saw with the 2 existing ones?

Yes, but the point is actually that this is a setting I would be ok
with for my entire frame. Whereas before I was using different
underline settings depending on whether or not it was code or prose.

> In general, you cannot assume in display code that buffer-local
> variables have their expected values, because redisplay needs to
> redraw windows other than the selected one, and when it does so, the
> window's buffer is not made the current one in the full sense of the
> word.  What you saw is the display engine using the value from the
> last buffer that was current before a redisplay cycle.  So you need to
> explicitly access buffer-local values by calling buffer_local_value;
> see the examples of that in xdisp.c.

Ok, that makes sense. Would you like me to make that change for all of
them given my description below? Is there some performance penalty to
this?

> If we want to allow users to make these variables buffer-local, the
> best way is to modify the display code to use their buffer-local
> values.  That would be a cleaner solution, I think.
>
> But anyway, what is the use case where you need different values for
> these variables in different buffers?  These variables were introduced
> to solve problems with semi-buggy fonts, and these problems are not
> limited to a single buffer.  Also, if you set these variables to
> ignore the line-spacing, it will produce a reasonable display in a
> buffer without any line-spacing at all, so I wonder why you needed to
> make these local.  Can you explain?

I briefly described this above, but here are some more details. Today,
globally, I'm using:

            (setq x-underline-at-descent-line t)

This was the default in spacemacs and it's likely because of #30609
(underlines drawn over descenders make text hard to read) and
different colored underlines are used heavily with flycheck.

And, in org-mode:

            (setq-local line-spacing 1)
            (setq-local x-underline-at-descent-line nil)
            (setq-local x-use-underline-position-properties t)

For reasons that led me to create this initial bug report:
x-underline-at-descent-line looks terrible when line-spacing > 0.

With this variable, you are right, I no longer need to use
`setq-local`, I can just set this variable globally and it will look
reasonable for code and prose.

That said, I'm not sure exactly what you meant by: "if you set these
variables to ignore the line-spacing", are you referring to the new
variable I introduced or are you OK with me making a change to
x-underline-at-descent-line to ignore line spacing?