Re: Question about display engine

[martin rudalics]

>> Consider a user who sets the :extend attribute to non-nil for the
>> 'default' face and wants a 'link' background to not extend to the end
>> of line.  Such a user would want to set the :extend attribute of the
>> 'link' face to nil to get the desired effect.
>
> Yes, and my point is that in that case we can simply ignore the 'link'
> face when extending the face of the last character of a line.

How would the display engine then know that it can "simply ignore"
the background of that face for the rest of the line?

> face_at_buffer_position is the right place, but it isn't called for
> every buffer position.  And your description seemed to hint that you
> thought it was called for every buffer position.

My bad.  But understanding face implementatin _without_ the text you
wrote below would take a couple of days at least for a humble reader
like me.

> This has to do with how the display iterator works in general.  It
> starts by determining where the next "stop position" is.  A "stop
> position" (stored in it->stop_charpos) is the position where iteration
> must stop and examine all the possible sources of affecting what and
> how should be displayed next.  This includes faces, on-the-fly
> fontification, invisible text, display properties, overlays, switching
> from iteration over a buffer to iteration of a string and back, etc.
> At that stop position, the iterator examines all these potential
> game-changers, and computes and stores the results in its fields.
> That is where face_at_buffer_position is called, and the result is
> it->face_id, a numerical index into the frame's face cache which
> defines the realized face to be used from now on for displaying
> characters.  The iterator will not call face_at_buffer_position and
> won't consider faces until it gets to the next stop position.
>
> The next stop position is computed in compute_stop_pos.  You will see
> that it doesn't call Fnext_single_property_change, but instead
> accesses the interval tree directly, because that's more efficient:
> you find changes in _any_ text properties that way.  It also considers
> overlay changes and changes in character composition.  The next stop
> position is the closest one of those provided by any of these sources.
>
> When we get to the stop position, we invoke a series of known
> handlers, each one responsible to handle the relevant Lisp data
> structures that might affect the display.  See handle_stop.  In
> particular, the handler of face properties is handle_face_prop, which
> calls face_at_buffer_position or face_at_string_position.  Each
> handler updates the relevant fields of the iterator structure as
> necessary.  Having handled everything that needs to be handled at a
> particular stop position, we call compute_stop_pos to compute the next
> stop position, and then proceed producing glyphs using the updated
> iterator information until we get to that next stop position, never
> examining any of the above factors again until we get to that place.

Thanks for the lucid description.  This is hopefully clear to me now.

>> and in particular where a :background change is found and applied, I
>> probably could tell you more.
>
> Neither :background nor any other face attribute is "found and
> applied" in the main iterator loop.  The iterator simply records in
> each glyph the face ID to be used for displaying that glyph.  That
> face ID is the value returned by the last call to
> face_at_buffer_position.

IIUC the latter will (unless the face was cached already) end up
calling merge_face_ref and somewhere there I find the lines

              else if (EQ (keyword, QCbackground))
                {
                  if (STRINGP (value))
                    to[LFACE_BACKGROUND_INDEX] = value;
                  else
                    err = true;
                }

Hence IIUC in the

          while (CONSP (face_ref) && CONSP (XCDR (face_ref)))

loop we could manage three booleans background, extend and
extend_value and instead of the above write

              else if (EQ (keyword, QCbackground))
                {
                  if (STRINGP (value))
                    {
                      to[LFACE_BACKGROUND_INDEX] = value;

                      if (extend)
                        to[LFACE_EXTEND_BACKGROUND] = extend_value;

                      background = true;
                    }
                  else
                    err = true;
                }
              else if (EQ (keyword, QCextend_background))
                {
                  extend = true;
                  extend_value = !NILP (value);

                  if (background)
                    to[LFACE_EXTEND_BACKGROUND] = extend_value;
                }

> The code which examines these attributes is
> in the terminal-specific backends: xterm.c, w32term.c, nsterm.m, etc.
> Only there we extract the colors, the underline, the strike-through,
> etc. attributes, and use them to produce the corresponding visual
> appearance.  For example, for the background color, that is where we
> instruct the GUI system to "clear to EOL" with that color.
>
> (The last paragraph is slightly over-simplified: you will see in a
> couple of places that we do look at face->background in xdisp.c,
> notably in extend_face_to_end_of_line, but only in order to compare it
> with the frame's default background.)

All these (the platform specific parts and the xdisp code) are already
too late for what I have in mind.

>> When face_at_buffer_position puts some 'background' value into that
>> single attribute vector, it can simply set an 'extend_background' bit
>> in that vector which tells the display engine whether the background
>> specified by that vector shall be extended or not.
>
> So you propose to have an "extend" bit for every face attribute?
> There are 18 of them.  (Maybe some of the attributes won't need that
> bit, but quite a few will.)

I'd propose to add these lazily if there is any need.  AFAIAC
:background is the only attribute where an extend bit sounds useful.
Some people here have explictily stated that they do not consider it
useful to extend the :underline attribute and I agree.  So if you
think that other attributes will need an extend bit, please tell me
which ones you have in mind.

> Moreover, given the description above of where the attributes are
> acted upon, you are saying that xterm.c, w32term.c etc. will have to
> consider whether the glyphs they display are at the end of a line or
> not, and act accordingly, i.e. ignore some of the face attributes
> under certain conditions, right?

By no means.  The extend_background bit would be set in merge_face_ref
as sketched above (because this is the one that can read the
:background and :extend attributes of any face) and examined in
extend_face_to_end_of_line wherever face->background is involved
(because this is the one that knows that we want to display a
newline).

> But the merged face will be used for displaying both the "extension"
> part of the line and "normal" characters.  When displaying "normal"
> characters, the background color should be used unconditionally,

Definitely so.

> whereas while displaying the "extension" part it should be used only
> if its 'extend' bit is set.  Right?

The extend_background bit, more precisely (the :extend face attribute
conceptually applies to all attributes so we can eventually handle
:underline and :box as well if somone wants that).

> What I wrote above assumed that
> we make these decisions at face merge time, and will actually have 2
> different realized faces;

We should have only one realized face.

> what you are suggesting is that we use a
> single realized face and make those decisions when we use the faces to
> write to the glass.  That doesn't strike me as a simpler
> implementation.

But that decision should have become simpler now that all we need is
to examine that extend_background bit to tell whether the background
shall extend to EOL or not.

martin