Re: [PATCH v2] Fix background color of latex previews

[Roshan Shariff]

Hi Timothy,

On Mon, 8 Jan 2024 at 03:24, Timothy  wrote:
> Thanks for this info and the patch! I've had a look at both, and I'm hopeful 
> that https://git.tecosaur.net/tec/org-mode/commit/3b3d48d3bf0 might solve the 
> problem? Do let me know what your think 🙂

Thanks for your prompt bugfix! I can confirm that it works beautifully
on both Emacs 28 and 29.

Looking at the code, I just had a small suggestion:

diff --git a/lisp/org-latex-preview.el b/lisp/org-latex-preview.el
index 19e34..973eed47b 100644
--- a/lisp/org-latex-preview.el
+++ b/lisp/org-latex-preview.el
@@ -541,7 +541,7 @@ Faces in `org-latex-preview--ignored-faces' are ignored."
 (normalising-face
  (if (>= emacs-major-version 29) 'default '(:inherit default
:extend t
 (cond
- ((consp face)
+ ((and (consp face) (not (keywordp (car face))) (listp (cdr face)))
   (nconc (cl-set-difference face
org-latex-preview--ignored-faces) (list normalising-face)))
  ((and face (not (memq face org-latex-preview--ignored-faces)))
   (list face normalising-face))

The (not (keywordp (car face))) condition ensures that the face isn't
a single anonymous face, i.e. a plist. The (listp (cdr face))
condition ensures that it's not just a cons like (foreground-color .
color). In both cases, the face spec is not a list of faces, so it
wouldn't be correct to append another face to the end. I'm not sure
how commonly this can happen in practice, but it covers all the cases
handled by merge_face_ref in xfaces.c.

Regards,
Roshan

> All the best,
> Timothy
>
> --
> Timothy (‘tecosaur’/‘TEC’), Org mode contributor.
> Learn more about Org mode at .
> Support Org development at ,
> or support my work at .

Re: [PATCH v2] Fix background color of latex previews

[Timothy]

Hi Roshin,

> Thanks again for your work on this exciting feature! I tried out your
> new code and it does indeed fix the problem on Emacs 29. But then I
> remembered that I originally encountered the issue on Emacs 28, and
> sure enough the issue persists there.
>
> To summarize, to get it to look right on Emacs 28, you need to append
> the face spec '(:inherit default :extend t) to the list of faces,
> rather than just 'default. Otherwise, when an equation overlay wraps
> to the next line in visual-line-mode, you'll see the background color
> of the org-block face leak through after the end-of-line. Emacs 29
> appears to behave more sensibly: if the overlay wraps to the next
> line, its face is not applied to the end of the current line.

Thanks for this info and the patch! I've had a look at both, and I'm hopeful 
that https://git.tecosaur.net/tec/org-mode/commit/3b3d48d3bf0 might solve the 
problem? Do let me know what your think 🙂

All the best,
Timothy

-- 
Timothy (‘tecosaur’/‘TEC’), Org mode contributor.
Learn more about Org mode at .
Support Org development at ,
or support my work at .

Re: [PATCH v2] Fix background color of latex previews

[Timothy]

Hi Roshan,

> This change addresses two issues:
>
> 1. Latex previews in headings have the background color of the
> fontified Latex code, rather than the rest of the heading.
>
> 2. If Latex code is fontified with a face that has the :extend
> attribute, and the preview overlay wraps to the next line, then the
> empty space after the end of the line uses the background color of the
> Latex code rather than that of the surrounding text.

Sorry for the delay in getting back to you. I ended up being rather busy around
the middle of last year. I’ve just started another sprint on the LaTeX preview
rewrite, and have made a few changes to how face guessing heuristics.

If you might be willing to try the new code and see if the issue noticed still
appears, that would be brilliant!

All the best,
Timothy

-- 
Timothy (‘tecosaur’/‘TEC’), Org mode contributor.
Learn more about Org mode at .
Support Org development at ,
or support my work at .

[PATCH v2] Fix background color of latex previews

[Roshan Shariff]

This change addresses two issues:

1. Latex previews in headings have the background color of the
fontified Latex code, rather than the rest of the heading.

2. If Latex code is fontified with a face that has the :extend
attribute, and the preview overlay wraps to the next line, then the
empty space after the end of the line uses the background color of the
Latex code rather than that of the surrounding text.

For example, setting org-highlight-latex-and-related to '(native)
applies the org-block face to Latex code.  Since the default heading
faces do not set :background, the org-block background is used for
preview overlays in headings.  And since org-block has :extend t, its
background color is also used for the empty space after the end of the
line preceding a wrapped latex preview.

With this change, we create a new anonymous face that inherits from
the default face and sets the :extend attribute, and use it as a
fallback for any attributes unspecified in the faces around the Latex
code.

1. Since the fallback face inherits from default, it is guaranteed to
be completely specified.  The preview overlay thus appears exactly
like the text around it; any unspecified attributes are taken from the
default face rather than the fontified Latex code.

2. When Emacs displays the empty space at the end of a line, it only
considers faces with a non-nil :extend attribute.  Since the fallback
face sets :extend, so will the merged preview overlay face.  Thus the
background color of the empty space will be taken from the surrounding
text face if it sets :extend, or the default face otherwise.
---

Please ignore the previous patch and look at this one instead :)

* I've fixed a bug in the org-latex-preview--face-with-fallback
function.

* I also noticed that the patch fixes a broader issue with background
colors for previews in headings.  I've updated the commit message to
describe both issues.

 lisp/org-latex-preview.el | 51 +--
 lisp/org.el   |  5 ++--
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/lisp/org-latex-preview.el b/lisp/org-latex-preview.el
index 03c198e32..96ffe894e 100644
--- a/lisp/org-latex-preview.el
+++ b/lisp/org-latex-preview.el
@@ -505,8 +505,7 @@ overlay face is set to `org-latex-preview-processing-face'."
 (overlay-put
  ov 'hidden-face
  (or (and errors 'error)
- (org-latex-preview--face-around
-  (overlay-start ov) (overlay-end ov)
+ (org-latex-preview--overlay-face ov
(errors
 (overlay-put
  ov 'before-string
@@ -526,8 +525,7 @@ overlay face is set to `org-latex-preview-processing-face'."
 (defun org-latex-preview--face-around (start end)
   "Return the relevant face symbol around the region START to END.
 A relevant face symbol before START is prefered, with END
-examined if none could be found, and finally the default face
-used as the final fallback.
+examined if none could be found.
 Faces in `org-latex-preview--ignored-faces' are ignored."
   (or (and (> start (point-min))
(not (eq (char-before start) ?\n))
@@ -544,8 +542,47 @@ Faces in `org-latex-preview--ignored-faces' are ignored."
   ((consp face)
(cl-set-difference face org-latex-preview--ignored-faces))
   ((not (memq face org-latex-preview--ignored-faces))
-   face
-  'default))
+   face))
+
+(defun org-latex-preview--face-with-fallback (face fallback)
+  "Return a face spec combining FACE with FALLBACK.
+FACE should be valid as a value of the `face' text or overlay
+property, as specified in Info node `(elisp)Special properties'.
+The return value will also be valid as a `face' text or overlay
+property.  FALLBACK should be a either a face name (symbol or
+string) or an anonymous face (plist); see the Info
+node `(elisp)Faces'."
+  (pcase face
+('nil
+ fallback)
+(`(:filtered ,filter ,face)
+ `(:filtered ,filter ,(org-latex-preview--face-with-fallback face 
fallback)))
+(`(foreground-color . ,color)
+ `((:foreground ,color) ,fallback))
+(`(background-color . ,color)
+ `((:background ,color) ,fallback))
+(`(,(pred (not keywordp)) . ,_)
+ (append face (list fallback)))
+(_
+ (list face fallback
+
+(defun org-latex-preview--overlay-face (ov)
+  "Return `face' property to be used for preview overlay OV.
+Return the face found by `org-latex-preview--face-around', with
+unset properties falling back to an anonymous face that inherits
+from `default' and sets the `:extend' attribute.
+
+The fallback ensures that if the overlay wraps to the next line
+and the text around the overlay has a face without `:extend', the
+empty space after the end of the line uses the default background
+color.  Otherwise, the space would take on the background color
+of the latex code itself if it has a face with `:extend'.  For
+example, when `org-highlight-late