Re: region-based face-remapping

[Eli Zaretskii]

> From: JD Smith <jdtsmith@gmail.com>
> Date: Wed, 3 Jan 2024 18:15:04 -0500
> Cc: emacs-devel@gnu.org
> 
> > Not exactly. You need to keep in mind that each buffer or string
> > position could have several sources of face information, see
> > "Displaying Faces" in the ELisp manual for the gory details.So first
> > Emacs needs to collect all those faces from all the relevant sources,
> > and then it merges them. Face remapping happens during the merge: each
> > face that is referenced by a symbol (a.k.a. "named face") is remapped
> > if needed.
> 
> I see.  But if there were a property at the position where Emacs is 
> collecting and merging all the face information, say
> 
>        ‘(face-remap ((some-face . another-face)))
> 
> would the merge process be able to easily use this style of 
> remap-information, in addition to the buffer-local 'face-remapping-alist 
> (with the local property taking precedence over the variable)?

Yes, but the buffer position will need to be known at the levels where
this remapping is handled, which is quite low-level code.

> > The workhorse we use now for obtaining face information is the
> > function face_at_buffer_position, which calls the various face-merging
> > routines.  Those merging routines and their subroutines consider
> > face-remapping-alist as part of face merging process outlined above.
> > I don't see how we can avoid passing the buffer position to them if
> > face-remapping depends on buffer position. Also, those routines will
> > now need to perform property or overlay search for the special
> > face-remapping property, using the position they are passed.
> > 
> > Am I missing something?
> 
> Perhaps we are talking about different things.  I was speaking from the point 
> of the view of the ELISP user of the faces API.  From the ELISP face API, I 
> don’t see a need to include any new position information for face definition 
> and face property assignment.  I think now that you were speaking about the 
> internal API.  Internally during the face merge process, I agree there would 
> need to know the properties at a merging-the-face-info point under 
> consideration, which could influence the merge.  

Of course, I was talking about the implementation and internal APIs!
You asked how about this idea, so I provided feedback about what it
would mean to implement such an idea.

> Maybe the issue is that face_at_buffer_position is called only at certain 
> special positions, not on each position in the buffer?

It is called where the 'face' property changes.  The display engine
finds the next position where that happens, and arranges for calling
face_at_buffer_position when it gets to process that position.  The
results of face merging at that position are kept in a cache, to avoid
performing it more often than needed.

> If so, I can see how your concern about property/overlay search would apply 
> (though what those positions would be I don’t know, since I can assign a 
> different 'face property to arbitrary ranges as small as one character).

Try assigning different 'face' property to each character, and you
will see that redisplay will be slower due to the need to stop and
look up faces more often.

> >> This may relate to the text-property “planes” discussion from a
> >> couple years back.  Based on that discussion, perhaps targeting only ‘face 
> >> is not enough. Is there
> >> another approach to “selectively enabling/substituting properties by 
> >> region”, beyond just
> >> text-property-search-forward within the region (looking within ‘display 
> >> properties too)?
> > 
> > I don't think I understand which discussion you have in mind and how
> > it is relevant to the issue at hand.
> 
> I meant this discussion (and the related earlier one mentioned, by Stefan M):
> 
> https://lists.nongnu.org/archive/html/emacs-devel/2019-11/msg00599.html

That's not different from face-remapping, and it is not
position-dependent, AFAIU.  So I don't think I see how it is relevant
to what you want to do.

> >> 1 On point motion, treesitter’s syntax tree is consulted to find an 
> >> "enclosing semantic context
> >> region".
> >> 2 Within that region, pre-existing faces (or perhaps other properties) get 
> >> updated to a “highlighted”
> >> variant.
> >> 3 Outdated/prior regions have the highlighted variants removed.
> >> 
> >> Since you don’t know in advance how large such regions might be, it seems 
> >> problematic to search
> >> and replace all the various faces or other properties.   
> > 
> > How do you envision to do items 2 and 3 above?  IOW, how do you
> > "update pre-existing faces"?  
> 
> In an ideal scenario, by applying a text or overlay property '(face-remap 
> ((some-face . some-highlight-face))) to that region.

Then we are back to the position information needed in low-level code
that merges faces.  Which is probably the simplest and most robust
solution; the only disadvantage is that we will need to "drag" the
buffer position through many existing APIs to a low level.  We'd
probably also need to extend face-font and the likes (more so if you
want the 'default' face to be mutable like this) with an optional
position argument.

> > The naïve solution, of somehow changing
> > the face attributes on the fly, will be a performance killer, since
> > currently any change in any named face invalidates all the faces we
> > already know about and requires their "realization" anew -- because we
> > don't track the dependencies between faces, and thus don't know which
> > other faces need to be updated as result.  So this would mean that any
> > point movement could potentially cause recalculation of all the faces
> > on the selected frame.  
> 
> I do this very thing already in indent-bars[1], globally in the buffer, using 
> face-remapping-alist.  The performance of that remapping is incredibly good 
> in my experience.  So fast in fact that I’ve had to introduce a timer to 
> artificially slow it down since it leads to rapid face flashing during smooth 
> scrolling.

How many faces do you have defined on that frame?  I'm guessing not
many, or maybe they are all almost identical (differ in only one or
two simple attributes).

> > And face realization is a relatively expensive
> > process, especially if there are a lot of them (we had reports about
> > users who routinely have hundreds or even thousands of faces on a
> > frame). Thus my question about items 2 and 3, which seem to do exactly
> > what we avoid doing: modify named faces.
> 
> Does “substitute a named face for another named or anonymous face” lead to 
> similar concerns?  Or can those then all be precomputed without the constant 
> face realization concern?

As long as the faces themselves don't change, no.  But that means we'd
need to store all those faces for all the regions that use such a
position-dependent remapping.