Re: Question about display engine

[Ergus]

On Mon, Sep 02, 2019 at 10:36:34AM +0200, martin rudalics wrote:
> > I have been thinking about this a little bit more and it seems like we
> > are trying to replicate a behavior that is already there.
> >
> > If we had an extra face pointer within the face struct initialized to
> > null (or default face), and then when we merge and the extend attribute is set 
> > we
> > initialize it to the current merged face and we transmit that pointer
> > from merge to merge until we have a new face with :extend, so we merge
> > then those two... I think when we reach the end of the line we will
> > have there already all the information we need.
> >
> > Lets say we have a sequence of consecutive merges (a,b,c,d,e); but only
> > b and d have :extend.
> >
> > At the end we will have in the merges:
> >
> > (a, a+b, a+b+c, a+b+c+d, a+b+c+d+e)
> >
> > but with this pointers in them:
> >
> > (nul, b,     b,   b + d,     b + d)
> >
> > Which for me it seems to be what we expect to have right?
> >
> > Does it makes sense?
>
> I think so.  Now the realized face for normal text is a+b+c+d+e and
> the display engine would use it right away.  When the display engine
> encounters a newline character it has to "switch" from a+b+c+d+e to
> b+d.  But it has to assure that b+d _is a realized face_ because a
> reference to that face is the output expected from the display engine.
> Any additional pointers like yours would be ignored after that.
>
> Do you agree so far?  If so, then the question remains _when_ to
> realize b+d.  Eagerly, when the face merger has done its work, or
> lazily when the display engine encounters the newline.  Eagerly has
> the advantage that the display engine has all the faces already
> realized.  Its disadvantage is that when a new stop position is found
> before the EOL, the b+d face was realized needlessly.  Do you still
> agree?
I think any of them is still a good trade-off because it is very
use-case specific. When coding and with the actual wide screens; in most
of the cases the lines are always extended, So I don't think there will
be a significant difference.

In any case we could implement any of them more or less with the same
complexity...
> If so then we might consider the following optimization: The extra
> face pointer in each face is no more needed after the display engine
> has processed the face.  So we could build a "shadow" face for b+d,
> keep it in a separate, static face structure and realize a face from
> that structure whenever we want to (eagerly or lazily).  The display
> engine, when it finds a pointer to such an extra face at EOL, uses it
> (maybe realizing it on the fly).
>
> Also, the merger could nullify the extra face if it's the same as the
> normal one, that is no single merge step had an :extend false value
> override a former :extend true value.  So the display engine would
> know beforehand that it does not have to change the current face.
Yes; in my initial proposed approach the local pointer will be null in
that case.

> Last but not least, the display engine has to, after processing the
> spaces at the EOL from b+d, restore the a+b+c+d+e face as its current
> face.  So we have two static pointers to keep around: One for the b+d
> face structure (or its already realized face) while the display engine
> processes normal text and one for the a+b+c+d+e realized face while
> the display engine processes the spaces at EOL.  Can you still agree?

In the display engine we do this very
frequently. As extend_face_to_end_of_line is very localized we just need
to save a pointer to a+b+c+d on the beginning of the function and
restore it at the end.

It is a bit more complex than that because the code for gui and TUI is
in different portions of an if condition, but this part is almost the
same I made in the first patch I proposed.

> Can you imagine any difficulties with implementing such an approach?
I only see small details like that in some cases we need to
reinitialize the extend face and the default-face value maybe is not the
right choice in all the cases (I need to look into it a bit more).

And that in X there is some extra code (somewhere) to extend the
background color using the color in the last inserted glyph (it is
something happening by default without calling even
extend_face_to_end_of_line). That code should be removed after this
change; but I don't know where is it. But for sure Eli will tell.

> And we would have happily put to rest all those crazy extend_... bits
> I proposed earlier.
>
> martin