Re: Question about display engine

[Ergus <spacibba@aol.com>]

On Fri, Sep 06, 2019 at 04:28:20PM +0300, Eli Zaretskii wrote:
>> Date: Fri, 6 Sep 2019 12:30:23 +0200
>> From: Ergus <spacibba@aol.com>
>> Cc: rudalics@gmx.at, emacs-devel@gnu.org
>>
>> the face should be extended after EOL means (somehow) that the
>> attributes specified in the face are merged with the ones in other
>> extensible faces to extend after EOL.
>
>So the face we use after EOL should be the result of merging only
>those faces which have their :extend attribute set to non-nil, is that
>right?
>
Yes. 
>> >face_id is initialized in init_iterator, which is always called once
>> >before the first call to display_line.  Thereafter, any subsequent
>> >call to display_line "inherits" the value of face_id left in the
>> >iterator object at the end of the previous call to display_line.
>> >
>> I understood this later actually.
>> >
>> >
>> >Whether this fits the logic of using extend_face_id, I cannot say yet;
>> >see the above questions that describe my confusion.
>> >
>> It actually does... but when the it->face_id changes (for example the
>> region ends in the middle of a line) the extend_face_id should know.
>
>Should it?  The way I see it, we don't need to care about
>extend_face_id until we actually come to EOL.
>
You are right.

>> I am actually rethinking the whole code... but I need to understand
>> better some details that are unclear for me. Like how to get the
>> "extensible" face_id from a non extensible mixed merged face. Lets say
>>
>> e = (a + b + c + d) where only a and c were extensible. Because if I don't
>> have a cache/face I will need to recalculate that every time and find a
>> way to remember how a face was composed... (remember that e was composed
>> by a; b; c; d and then iterate over those ids, get_face_from_id and do a
>> loop that if EXTENSIBLE-P will merges in extend_face_id. This will be
>> sub efficient.
>
>I don't think you need to remember anything, because Emacs "remembers"
>for you.  All of those faces (a, b, c, d) will still be in effect at
>EOL (i.e. at the position of the newline character); all you need is
>to merge them there while ignoring those of them whose :extend
>attribute is not set.
>
>IOW, I thing extend_face_id should only be computed at EOL, either in
>extend_face_to_end_of_line or in append_space_for_newline.  Because
>you don't need that face ID before you come to EOL.
>
append_space_for_newline is not called in all the cases. and this has to
do with the yesterdays question about what face should have the extra
space (before extending).

>> The simplest case: suppose that we have (h == b) but h is extensible and
>> b is not. they both will have different face_id because the vectors are
>> different.
>>
>> Merging (a + b + c + d) == (a + h + c + d) -> same face id
>> but the extensible faces (a + c) != (a + h + c) -> different face_id
>>
>> So I don't know how to face this if I want to do it at the EOL
>> only. Because of that I was somehow searching for a method that could
>> give me (a + h + c) or (a + c) on the fly every time... but this seems
>> to be wrong implemented; so I need MORE help here.
>
>I think the solution should be to have a variant of the code in
>handle_face_prop such that it computes the face at EOL.  It would do
>that by modifying face_at_buffer_position and face_at_string_position
>to accept an additional argument EOL_P, which means merge only faces
>which have their :extend attribute set.  Then the face ID computed for
>this specially merged face should be used as extend_face_id.
>
>Does this make sense?
>
Probably yes but more questions :)

Lets say that I actually don't understand very well what
handle_face_prop does (when it is called and when not).

When you say a variant you mean another function to call directly from
extend_face_to_end_of_line? Sorry I still don't understand where is (a +
b + c + d) computed or where emacs "remembers" that, or if it is
computed all the time. But maybe the trick is actually in
face_at_buffer_position, face_for_overlay_string or
face_at_string_position?

If so; then what we really need is a variant of face_at_buffer_position
like extend_face_at_buffer_position? (or add to it a parameter to do
what we want) does it makes any sense.

handle_face_prop can't be modified as it should have a specific
prototype. But we can make it a wrapper and create a generalized or use
ITERATOR_AT_END_OF_LINE_P internally?