Re: region-based face-remapping

[JD Smith <jdtsmith@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 6056 bytes --]



> On Jan 3, 2024, at 7:31 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: JD Smith <jdtsmith@gmail.com>
>> 
>> To make the entire face API position-dependent does seem complex. I presume face calculation
>> first checks if there is a valid 'face-remapping-alist’ and does substitutions.
> 
> 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)?

>> If 'face-remap were a
>> valid (overlay/text) property, that would seem to obviate any deeper API changes: just add this
>> property where remapping is desired.
> 
> I don't think I follow.
> 
> 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.  

Maybe the issue is that face_at_buffer_position is called only at certain special positions, not on each position in the buffer?  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).

>> 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

My read of that discussion is it would allow external control of various “planes" of a text property: turn some on, others off, without searching through the entirety of the buffer finding and replacing those properties.  Similar to layers in a photo editor.  Whether that type of external control could be limited to a region I am unsure (though the MMM discussion makes me think that was envisioned).  The “layers” or “planes” concept certainly seems related.

>> To make this concrete, how would people approach the following:
>> 
>> 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.

> 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.

> 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?

[1] https://github.com/jdtsmith/indent-bars


[-- Attachment #2.1: Type: text/html, Size: 35856 bytes --]

[-- Attachment #2.2: favicon.ico --]
[-- Type: image/vnd.microsoft.icon, Size: 1406 bytes --]