Re: RFC: flicker-free double-buffered Emacs under X11

[Eli Zaretskii <eliz@gnu.org>]

> From: Daniel Colascione <dancol@dancol.org>
> Cc: emacs-devel@gnu.org
> Date: Sun, 23 Oct 2016 13:51:01 -0700
> 
> > I meant the test for the lone font backend that needs this kludge, not
> > for the existence of the method.  And also a comment that tells it's a
> > kludge whose need we understand only empirically.  I'd even prefer a
> > direct call to the function when the font backend is the one which
> > needs it, rather than having a method created for the benefit of a
> > single backend, whose necessity we don't really understand.
> 
> So we want to write?
> 
>   for font_backend in frame.font_backends:
>     if instanceof(font_backend, XftBackend):
>       xft_refresh_hack()
> 
> That still strikes me as less clean, especially considering we'll have
> to provide the xft_refresh_hack to general-purpose code.

At the very least let's have a comment there saying that this is a
kludge that's actually done only for that font driver.  (Comments tend
to fall out of sync with the code, so making code speak for itself is
better, but a comment is infinitely better than nothing at all.)

> > I'm not saying the code won't work.  I'm saying it could make its
> > purpose more evident, which is better for long-term maintainability.
> 
> Longer-term, I think we need to move toward greater, not
> lesser, modularity.

I agree, but IME such a move cannot be piecemeal in this case, it must
be done in one go, or in a small number of large steps.  IOW, it's a
significant job on its own.

> >> We need to do a buffer flip at the end of redisplay for each frame on
> >> which update_end was called during redisplay.
> >
> > Even if update_end was called for a frame whose update was "paused",
> > i.e. whose redisplay was interrupted by incoming input?
> 
> Right now, we do a buffer flip unconditionally except in the case that
> we `goto end_of_redisplay`. I think that's a bug actually --- but I have
> to understand more of what this logic is actually doing.
> 
> Why _do_ we have a path that short-circuits the rest of the redisplay
> code? What would happen if we just removed it? It appears to be some
> kind of optimization, and I'm not sure it's actually necessary
> (especially since, according to the comment, we disable it anyway in the
> case of a blinking cursor).

It's an optimization for the case that nothing needs to be
redisplayed.

But I wasn't asking about that, I was asking about this part:

		  pending |= update_frame (f, false, false);

If update_frame returns non-zero, we don't mark all the windows as
having accurate display, which will cause redisplay to be re-entered
again on the first opportunity, and it will then try to redisplay all
of them again.  I was asking whether flipping in this case is TRT.

Without double buffer, what happens in this case is that we could
momentarily flash a partially redrawn window.

> I suppose you could make an argument for not doing a buffer flip if
> redisplay is interrupted by input, but I'm honestly not sure what might
> go wrong there, and my preference would be to act as much like the
> single-buffered case as possible and default, if we're unsure, to
> showing the results of our painting efforts to the user.

OK, we can always wait for bug reports, I guess, and take it from
there.

> > My problems start with the name of the hook, which doesn't hint at all
> > that only double-buffered X11 back-end cares about that.  If the hook
> > was called something like double_buffer_flip_hook, or was compiled
> > only if HAVE_XDBE is defined, this issue would go away.
> 
> So let's call it double_buffer_flip_hook.

Great, let's.

> > IOW, if the code speaks for itself, it makes maintenance of this
> > hyper-convoluted piece of Emacs easier, less time consuming, and less
> > error prone.
> 
> This code is an adventure.

It is.

> >> I'm also worried about maintainability: that's why I don't want to make
> >> redisplay_internal any more of a big ball of mud than it already is.
> >
> > It's too late for that.  It's already an extremely complex and hard to
> > understand piece of code.  Hiding more information from its logic
> > makes the code harder to maintain, not easier.  We are not talking
> > about highly-modular package, where the logic is kept local to each
> > module, and the interfaces between them are kept to the absolute
> > minimum.  xdisp.c _knows_ a lot about how xterm.c and xfns.c work.
> > You cannot disengage them without a complete rewrite.
> 
> Well, it also works with w32term and nsterm, so the coupling can't be
> complete. :-)

That's why w32term and nsterm are largely modeled on xterm (and the
same with xfns), right to the point of literally repeating many code
fragments.

> By the way: aren't most calls to x_catch_errors already buggy? AFAICT,
> they don't generally call x_sync first.  The purpose of this mechanism
> is to run a section of code in such a way that Emacs doesn't die if
> something goes wrong in Xlib, but since Xlib error reporting is
> asynchronous anyway, use of x_catch_errors without a preceding x_sync
> (as I have in my code) can silently swallow X errors that we _do_ want
> to kill Emacs.

That's for some X expert to answer, I really don't know enough about
this stuff.  Ken, can you comment on this?

>   1. [must fix] during frame creation, on one of my computers but not
>   the other, I momentarily see an all-black frame with a small white
>   square, and only a few hundred ms later does the normal frame content
>   get draws

Possibly related to the fact that when Emacs starts, it first defines
a small 10x10 frame, before resizing it to the actual size.  Look for
"10" in make_frame.

>   2. [must fix] on the same system, and not the other, after resuming
>   the system from sleep, Emacs frames momentarily display all white
>   before the usual frame contents get filled in

Could be Emacs waits for an expose event before redrawing the frame,
and meanwhile you show an empty buffer?

>   3. [should fix] on the same system, and not the other, resizing the a
>   frame interactively still produces some flicking, particulary in the
>   modeline, but much less than without the patch entirely.
>   This flickering appears to have something to do with XRender, since if
>   I force everything to use X11 core rendering instead, I don't see any
>   flickering at all.

If the flickering is on the modeline, a breakpoint in
redisplay_mode_lines should tell you why it happens.

Thanks.