Re: Question about display engine

[Ergus <spacibba@aol.com>]

Hi Eli and martin.

Here I attach a patch minimal proof of concept for the last solution I
proposed.

At the moment it is not showing the color extension to the end of the
line, but I cannot make it extend because it seems like the merge
function I added is never executed (which makes no sense but it does not
print the messages I added with printf.)

Could any of you give a look to the patch to detect what is failing at
least to triger the merge and extend?
Probably the initialization. (which btw the lisp glue code may be buggy
for sure.)

Please, any help is very welcome.

On Mon, Sep 02, 2019 at 07:18:19PM +0300, Eli Zaretskii wrote:
>> Date: Mon, 2 Sep 2019 13:05:04 +0200
>> From: Ergus <spacibba@aol.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
>>
>> >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.
>
>I don't think I understand why you are talking about face pointers.
>The iterator doesn't keep any face pointers, it keeps face IDs (which
>allow to obtain the face pointer, when needed, by using FACE_FROM_ID).
>
>So all you need is to have another face ID member in the iterator, to
>be used for extending past EOL; depending on how the :extend bits are
>set, that face ID may or may not be identical to the "normal" face ID,
>the one we have now and use for buffer text.
>
>> >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.
>
>Again, not a pointer: a face ID.  And yes, we have saved_face_id
>member of the iterator for this purpose.
>
>> 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.
>
>It's just that we clear to EOL with the last background color, and
>avoid doing that if the background color is identical to the frame's
>background.  I don't think anything will have to be changed there, but
>we will see (I hope).
>