GNU bug report logs - #22323
Font fallback causes inconsistent stacking of faces in overlays with invisible property

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 22323 in the body.
You can then email your comments to 22323 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Clément Pit--Claudel <clement.pitclaudel <at> live.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Font fallback causes inconsistent stacking of faces in overlays with
 invisible property
Date: Thu, 7 Jan 2016 11:52:55 -0500

[Message part 1 (text/plain, inline)]

Hi all,

Font fallback seems to break face stacking for invisible overlays:

This works fine:

(with-current-buffer (get-buffer-create "No prettification: ellispis is highlighed")
  (erase-buffer)
  (fundamental-mode)
  (add-to-invisibility-spec '(outline . t))
  (insert "abc!!def!!ghi")
  (let ((ov (make-overlay 6 9)))
    (overlay-put ov 'invisible 'outline))
  (let ((ov (make-overlay (point-min) (point-max))))
    (overlay-put ov 'face 'region))
  (pop-to-buffer (current-buffer)))

This doesn't work (assuming that ℙ is not in your usual font):

(with-current-buffer (get-buffer-create "No prettification: ellispis is highlighed")
  (erase-buffer)
  (fundamental-mode)
  (add-to-invisibility-spec '(outline . t))
  (insert "abcℙℙdefℙℙghi")
  (let ((ov (make-overlay 6 9)))
    (overlay-put ov 'invisible 'outline))
  (let ((ov (make-overlay (point-min) (point-max))))
    (overlay-put ov 'face 'region))
  (pop-to-buffer (current-buffer)))

I came across this while using prettify-symbols-mode:

(with-current-buffer (get-buffer-create "With prettification to common character: ellispis is highlighed")
  (erase-buffer)
  (fundamental-mode)
  (add-to-invisibility-spec '(outline . t))
  (insert "abc!!def!!ghi")
  (setq prettify-symbols-alist '(("!!" . ?ℙ)))
  (prettify-symbols-mode)
  (let ((ov (make-overlay 6 9)))
    (overlay-put ov 'invisible 'outline))
  (let ((ov (make-overlay (point-min) (point-max))))
    (overlay-put ov 'face 'region))
  (pop-to-buffer (current-buffer)))

Cheers,
Clément.

[signature.asc (application/pgp-signature, attachment)]

Message #10 received at 22323 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Clément Pit--Claudel <clement.pitclaudel <at> live.com>
Cc: 22323 <at> debbugs.gnu.org
Subject: Re: bug#22323: Font fallback causes inconsistent stacking of faces in
 overlays with invisible property
Date: Thu, 07 Jan 2016 20:22:44 +0200

> From: Clément Pit--Claudel <clement.pitclaudel <at> live.com>
> Date: Thu, 7 Jan 2016 11:52:55 -0500
> 
> Font fallback seems to break face stacking for invisible overlays:
> 
> This works fine:
> 
> (with-current-buffer (get-buffer-create "No prettification: ellispis is highlighed")
>   (erase-buffer)
>   (fundamental-mode)
>   (add-to-invisibility-spec '(outline . t))
>   (insert "abc!!def!!ghi")
>   (let ((ov (make-overlay 6 9)))
>     (overlay-put ov 'invisible 'outline))
>   (let ((ov (make-overlay (point-min) (point-max))))
>     (overlay-put ov 'face 'region))
>   (pop-to-buffer (current-buffer)))
> 
> This doesn't work (assuming that ℙ is not in your usual font):
> 
> (with-current-buffer (get-buffer-create "No prettification: ellispis is highlighed")
>   (erase-buffer)
>   (fundamental-mode)
>   (add-to-invisibility-spec '(outline . t))
>   (insert "abcℙℙdefℙℙghi")
>   (let ((ov (make-overlay 6 9)))
>     (overlay-put ov 'invisible 'outline))
>   (let ((ov (make-overlay (point-min) (point-max))))
>     (overlay-put ov 'face 'region))
>   (pop-to-buffer (current-buffer)))

This is indeed the same issue as bug#22320.  Using a different font
requires a different face, so you have here exactly the same situation
as in #22320: the last visible character ℙ has a face that is
different from the first invisible character d.

Message #13 received at 22323 <at> debbugs.gnu.org (full text, mbox):

From: Clément Pit--Claudel <clement.pitclaudel <at> live.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 22323 <at> debbugs.gnu.org
Subject: Re: bug#22323: Font fallback causes inconsistent stacking of faces in
 overlays with invisible property
Date: Thu, 7 Jan 2016 20:19:25 -0500

[Message part 1 (text/plain, inline)]

On 01/07/2016 04:02 PM, Eli Zaretskii wrote:
>> Date: Thu, 07 Jan 2016 22:12:42 +0200
>> From: Eli Zaretskii <eliz <at> gnu.org>
>> Cc: 22320 <at> debbugs.gnu.org
>>
>>> The same problems exist for composition, but keeping the properties of the first character seems to work well there; maybe we could consider harmonizing both behaviors?
>>
>> I'm not sure I understand what exactly are you proposing to do.  We
>> cannot treat invisible text like we treat character compositions, each
>> one invokes a very different machinery with distinct and very
>> different features.
> 
> Here's a suggestion: ignore the face of the invisible text altogether,
> and instead always use the face of the last visible character.  The
> patch to do that is below; it fixes all of your test cases.  But since
> you would like to see the face of the invisible text show through, I'm
> not sure you will like it...
> 
> WDYT?

I like it :) I find it much more consistent that what we currently have. Plus,
has a desirable property that you pointed out and that I agree with, that the
properties of the invisible text do not affect the display of the ellipsis. The
current implementation doesn't have that property (adding a face to a hidden
character will cause the ellipsis to change to the default face), so I think
it's a net gain.

(In fact, I'm even a bit surprised that it handles the selection and font
fallback cases properly. It's great that it does though; but did I apply the
patch incorrectly?)

Clément.

> diff --git a/src/xdisp.c b/src/xdisp.c
> index 87a92fc..7e5f7df 100644
> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -4583,14 +4583,15 @@ setup_for_ellipsis (struct it *it, int len)
>    it->current.dpvec_index = 0;
>    it->dpvec_face_id = -1;
>  
> -  /* Reset the current face ID to default if the last visible
> -     character and the first invisible character have different faces.
> -     IT->saved_face_id was set in handle_stop to the face of the
> -     preceding character, and will be different from IT->face_id only
> -     if the invisible text skipped in handle_invisible_prop has some
> -     non-default face.  IT's face is restored in set_iterator_to_next.  */
> -  if (it->saved_face_id < 0 || it->saved_face_id != it->face_id)
> -    it->saved_face_id = it->face_id = DEFAULT_FACE_ID;
> +  /* Use IT->saved_face_id for the ellipsis, so that it has the same
> +     face as the preceding text.  IT->saved_face_id was set in
> +     handle_stop to the face of the preceding character, and will be
> +     different from IT->face_id only if the invisible text skipped in
> +     handle_invisible_prop has some non-default face.  We thus ignore
> +     the face of the invisible text when we display the ellipsis.
> +     IT's face is restored in set_iterator_to_next.  */
> +  if (it->saved_face_id >= 0)
> +    it->face_id = it->saved_face_id;
>  
>    /* If the ellipsis represents buffer text, it means we advanced in
>       the buffer, so we should no longer ignore overlay strings.  */
> 
> 

[signature.asc (application/pgp-signature, attachment)]

Message #18 received at 22323-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Clément Pit--Claudel <clement.pitclaudel <at> live.com>
Cc: 22323-done <at> debbugs.gnu.org
Subject: Re: bug#22323: Font fallback causes inconsistent stacking of faces in
 overlays with invisible property
Date: Fri, 08 Jan 2016 12:22:52 +0200

> Cc: 22323 <at> debbugs.gnu.org
> From: Clément Pit--Claudel <clement.pitclaudel <at> live.com>
> Date: Thu, 7 Jan 2016 20:19:25 -0500
> 
> > Here's a suggestion: ignore the face of the invisible text altogether,
> > and instead always use the face of the last visible character.  The
> > patch to do that is below; it fixes all of your test cases.  But since
> > you would like to see the face of the invisible text show through, I'm
> > not sure you will like it...
> > 
> > WDYT?
> 
> I like it :) I find it much more consistent that what we currently have. Plus,
> has a desirable property that you pointed out and that I agree with, that the
> properties of the invisible text do not affect the display of the ellipsis. The
> current implementation doesn't have that property (adding a face to a hidden
> character will cause the ellipsis to change to the default face), so I think
> it's a net gain.

Thanks, I pushed it, and I'm marking these 2 bugs as done.

> (In fact, I'm even a bit surprised that it handles the selection and font
> fallback cases properly. It's great that it does though; but did I apply the
> patch incorrectly?)

Yes, you applied correctly.  The default ellipsis uses ASCII
characters, so I think the fallback font doesn't affect them.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.