unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Identifying the face between STRETCH and right fringe.
@ 2018-11-20 16:39 Keith David Bershatsky
  2018-11-20 17:20 ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Keith David Bershatsky @ 2018-11-20 16:39 UTC (permalink / raw)
  To: Emacs Devel

Step 1:  Open a buffer and evaluate:

(fundamental-mode)
(setq word-wrap t)
(setq buffer-display-table (make-display-table))
(aset buffer-display-table
      ?\t
      (vector (make-glyph-code ?\u00BB 'font-lock-warning-face)
              (make-glyph-code ?\t 'highlight)))

Step 2:  On a new line, type C-q TAB and then hold down the semi-colon key ';' to repeat the semi-colon character until it carries over to the next line.

Step 3:  Observe that the result is different on Emacs --with-ns, versus --with-x and also on a Windows machine.  Specifically, the STRETCH on an NS platform is seen spanning all the way to the right fringe.  On an X11 and NT platform, the STRETCH is only visible for the width of the STRETCH (depicted in the dump-glyph-row for each platform below).

EXAMPLE NS (OSX):  https://www.lawlist.com/images/tab_stretch_ns__2018_11_20.png

EXAMPLE WINDOWS:  https://www.lawlist.com/images/tab_stretch_nt__2018_11_20.png

EXAMPLE X11:  https://www.lawlist.com/images/tab_stretch_x11__2018_11_20.png

QUESTION #1:  In terms of identifying the face between the STRETCH and the right fringe, how can I programmatically know the difference between the NS situation, versus the NT and X11 situation?

QUESTION #2:  Is the difference in behavior between the different platforms "a bug", and should the X11 and NT ports be "fixed" so that they behave like the NS port in this situation?

BACKGROUND:  In the context of feature request #17684 (crosshairs); I am drawing vertical/horizontal lines on the screen using the built-in draw cursor mechanisms.  When there is no glyph at the desired location, I simply draw/erase a vertical/horizontal line with the desired color.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(fundamental-mode)
(setq word-wrap t)
(setq buffer-display-table (make-display-table))
(aset buffer-display-table
      ?\t
      (vector (make-glyph-code ?\u00BB 'font-lock-warning-face)
              (make-glyph-code ?\t 'highlight)))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; NS

Row     Start       End Used oE><\CTZFesm     X    Y    W    H    V    A    P
==============================================================================
 11       384       385    2 010010101000     0  176   56   16   16   12   12
           -1        -1	    0
           -1        -1
           -1        -1
 Glyph#  Type       Pos   O   W     Code      C Face LR
      0     C       384   B   7 0x0000bb      .   30 00
      1     S       384   B  49 0x000000          29 00

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; X11

Row     Start       End Used oE><\CTZFesm     X    Y    W    H    V    A    P
==============================================================================
 11       384       385    2 010010100000     0  165   64   15   15   12   12
           -1        -1     0
           -1        -1
           -1        -1
 Glyph#  Type       Pos   O   W     Code      C Face LR
      0     C       384   B   8 0x0000bb      .   33 00
      1     S       384   B  56 0x000000          26 00

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; NT

Row     Start       End Used oE><\CTZFesm     X    Y    W    H    V    A    P
==============================================================================
 12       385       386    2 010010100000     0  192   64   16   16   12   12
           -1        -1     0
           -1        -1
           -1        -1
 Glyph#  Type       Pos   O   W     Code      C Face LR
      0     C       385   B   8 0x0000bb      .   31 00
      1     S       385   B  56 0x000000          30 00

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-20 16:39 Identifying the face between STRETCH and right fringe Keith David Bershatsky
@ 2018-11-20 17:20 ` Eli Zaretskii
  2018-11-21  7:44   ` Robert Pluim
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-20 17:20 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: emacs-devel

> Date: Tue, 20 Nov 2018 08:39:16 -0800
> From: Keith David Bershatsky <esq@lawlist.com>
> 
> Step 3:  Observe that the result is different on Emacs --with-ns, versus --with-x and also on a Windows machine.  Specifically, the STRETCH on an NS platform is seen spanning all the way to the right fringe.  On an X11 and NT platform, the STRETCH is only visible for the width of the STRETCH (depicted in the dump-glyph-row for each platform below).

It's not the stretch glyph that spans all the window width, it's the
face of the stretch that gets extended to the window end.  The stretch
glyph is still 7-character wide:

> ;;; NS
> 
> Row     Start       End Used oE><\CTZFesm     X    Y    W    H    V    A    P
> ==============================================================================
>  11       384       385    2 010010101000     0  176   56   16   16   12   12
>            -1        -1	    0
>            -1        -1
>            -1        -1
>  Glyph#  Type       Pos   O   W     Code      C Face LR
>       0     C       384   B   7 0x0000bb      .   30 00
>       1     S       384   B  49 0x000000          29 00
                               ^^
49 = 7 * 7

> QUESTION #1:  In terms of identifying the face between the STRETCH and the right fringe, how can I programmatically know the difference between the NS situation, versus the NT and X11 situation?
> 
> QUESTION #2:  Is the difference in behavior between the different platforms "a bug", and should the X11 and NT ports be "fixed" so that they behave like the NS port in this situation?

I will look into this when I have time.  I don't yet know whether the
bug is on NS or on the other 2 platforms, though I tend to think the
former.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-20 17:20 ` Eli Zaretskii
@ 2018-11-21  7:44   ` Robert Pluim
  2018-11-23 10:04     ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-21  7:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Keith David Bershatsky, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Tue, 20 Nov 2018 08:39:16 -0800
>> From: Keith David Bershatsky <esq@lawlist.com>

>> QUESTION #1: In terms of identifying the face between the STRETCH
>> and the right fringe, how can I programmatically know the difference
>> between the NS situation, versus the NT and X11 situation?
>> 
>> QUESTION #2: Is the difference in behavior between the different
>> platforms "a bug", and should the X11 and NT ports be "fixed" so
>> that they behave like the NS port in this situation?
>
> I will look into this when I have time.  I don't yet know whether the
> bug is on NS or on the other 2 platforms, though I tend to think the
> former.

I find the NS behaviour less surprising: the face has been applied to
the TAB character, so the emptiness created by emacs displaying the
';'s on the next line should not have that face, since thereʼs nothing
there.

Having said that, consistency would be good here, in either direction.

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-21  7:44   ` Robert Pluim
@ 2018-11-23 10:04     ` Eli Zaretskii
  2018-11-23 13:25       ` Robert Pluim
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-23 10:04 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Keith David Bershatsky <esq@lawlist.com>,  emacs-devel@gnu.org
> Date: Wed, 21 Nov 2018 08:44:58 +0100
> 
> I find the NS behaviour less surprising: the face has been applied to
> the TAB character, so the emptiness created by emacs displaying the
> ';'s on the next line should not have that face, since thereʼs nothing
> there.

But we don't behave like that anywhere else.  The background of a face
is only extended to the window edge for the default face, not for any
other face.  So I'm puzzled by your preference.  Do you see a behavior
like the one you fancy in any other similar situation?



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-23 10:04     ` Eli Zaretskii
@ 2018-11-23 13:25       ` Robert Pluim
  2018-11-23 13:48         ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-23 13:25 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: Keith David Bershatsky <esq@lawlist.com>,  emacs-devel@gnu.org
>> Date: Wed, 21 Nov 2018 08:44:58 +0100
>> 
>> I find the NS behaviour less surprising: the face has been applied to
>> the TAB character, so the emptiness created by emacs displaying the
>> ';'s on the next line should not have that face, since thereʼs nothing
>> there.
>
> But we don't behave like that anywhere else.  The background of a face
> is only extended to the window edge for the default face, not for any
> other face.

Iʼm not sure I understand here: this is a non-default face, and itʼs
not being extended to the window edge. Based on what you say here
thatʼs normal, but X11 and Windows *donʼt* do it?

> So I'm puzzled by your preference.  Do you see a behavior
> like the one you fancy in any other similar situation?

Yes: on macOS, if I turn on hl-line-mode, then the highlighting is
only extended to the edge if point is not in the last line of the
buffer. That may well be a symptom of the same issue (and itʼs kind of
disconcerting now that Iʼve noticed it :-) )

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-23 13:25       ` Robert Pluim
@ 2018-11-23 13:48         ` Eli Zaretskii
  2018-11-23 14:04           ` Robert Pluim
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-23 13:48 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Fri, 23 Nov 2018 14:25:15 +0100
> 
> Iʼm not sure I understand here: this is a non-default face, and itʼs
> not being extended to the window edge. Based on what you say here
> thatʼs normal, but X11 and Windows *donʼt* do it?

No, it's the other way around: X11 and Windows *don't* extend the
face, the NS build *does*.  Maybe I misunderstood what you said, but
my interpretation was that you liked the NS behavior (which I think is
a bug).

> > So I'm puzzled by your preference.  Do you see a behavior
> > like the one you fancy in any other similar situation?
> 
> Yes: on macOS, if I turn on hl-line-mode, then the highlighting is
> only extended to the edge if point is not in the last line of the
> buffer.

Hl-Line mode explicitly highlights the entire line, so it's a small
wonder you see the entire line highlighted in its color.  What happens
at EOB could be a bug (or a feature) in hl-line, and is not related to
the issue at hand in any way, shape or form, because hl-line.el does
this:

      (setq tmp t
	    b (line-beginning-position)
	    e (line-beginning-position 2)))
    (if tmp
	(move-overlay overlay b e)

Which intends to set the background of every character until the
beginning of the next line.

> That may well be a symptom of the same issue

No, they are two entirely different issues, AFAICT.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-23 13:48         ` Eli Zaretskii
@ 2018-11-23 14:04           ` Robert Pluim
  2018-11-23 15:51             ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-23 14:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Fri, 23 Nov 2018 14:25:15 +0100
>> 
>> Iʼm not sure I understand here: this is a non-default face, and itʼs
>> not being extended to the window edge. Based on what you say here
>> thatʼs normal, but X11 and Windows *donʼt* do it?
>
> No, it's the other way around: X11 and Windows *don't* extend the
> face, the NS build *does*.  Maybe I misunderstood what you said, but
> my interpretation was that you liked the NS behavior (which I think is
> a bug).

Iʼve managed to get the two mixed up: on NS I see the face extended,
on X11 I donʼt, and I prefer the X11 behaviour.

>> > So I'm puzzled by your preference.  Do you see a behavior
>> > like the one you fancy in any other similar situation?
>> 
>> Yes: on macOS, if I turn on hl-line-mode, then the highlighting is
>> only extended to the edge if point is not in the last line of the
>> buffer.
>
> Hl-Line mode explicitly highlights the entire line, so it's a small
> wonder you see the entire line highlighted in its color.  What happens
> at EOB could be a bug (or a feature) in hl-line, and is not related to
> the issue at hand in any way, shape or form, because hl-line.el does
> this:
>
>       (setq tmp t
> 	    b (line-beginning-position)
> 	    e (line-beginning-position 2)))
>     (if tmp
> 	(move-overlay overlay b e)
>
> Which intends to set the background of every character until the
> beginning of the next line.

And in fact thatʼs exactly what I see on all my platforms. As I said,
itʼs somewhat disconcerting, but at least itʼs consistent (and a
different issue).

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-23 14:04           ` Robert Pluim
@ 2018-11-23 15:51             ` Eli Zaretskii
  2018-11-23 15:58               ` Robert Pluim
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-23 15:51 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Fri, 23 Nov 2018 15:04:13 +0100
> 
> > No, it's the other way around: X11 and Windows *don't* extend the
> > face, the NS build *does*.  Maybe I misunderstood what you said, but
> > my interpretation was that you liked the NS behavior (which I think is
> > a bug).
> 
> Iʼve managed to get the two mixed up: on NS I see the face extended,
> on X11 I donʼt, and I prefer the X11 behaviour.

Ah, okay.  Then we agree: there's some bug on NS that causes the face
to be extended.  The expected behavior is the one we see on X and on
Windows (and also on TTY frames).



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-23 15:51             ` Eli Zaretskii
@ 2018-11-23 15:58               ` Robert Pluim
  2018-11-23 20:33                 ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-23 15:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Fri, 23 Nov 2018 15:04:13 +0100
>> 
>> > No, it's the other way around: X11 and Windows *don't* extend the
>> > face, the NS build *does*.  Maybe I misunderstood what you said, but
>> > my interpretation was that you liked the NS behavior (which I think is
>> > a bug).
>> 
>> Iʼve managed to get the two mixed up: on NS I see the face extended,
>> on X11 I donʼt, and I prefer the X11 behaviour.
>
> Ah, okay.  Then we agree: there's some bug on NS that causes the face
> to be extended.  The expected behavior is the one we see on X and on
> Windows (and also on TTY frames).

Any idea where the code that handles this is? I took a quick look at
HAVE_NS, but nothing jumped out at me.

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-23 15:58               ` Robert Pluim
@ 2018-11-23 20:33                 ` Eli Zaretskii
  2018-11-27  8:56                   ` Robert Pluim
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-23 20:33 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Date: Fri, 23 Nov 2018 16:58:41 +0100
> Cc: emacs-devel@gnu.org
> 
> > Ah, okay.  Then we agree: there's some bug on NS that causes the face
> > to be extended.  The expected behavior is the one we see on X and on
> > Windows (and also on TTY frames).
> 
> Any idea where the code that handles this is? I took a quick look at
> HAVE_NS, but nothing jumped out at me.

I didn't see something like that, either.  But it could be the other
way around: that NS needs some code to prevent that from happening.

I think the place to look is ns_maybe_dumpglyphs_background: what is
the width of the rectangle that it clears with the background color of
the face?



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-23 20:33                 ` Eli Zaretskii
@ 2018-11-27  8:56                   ` Robert Pluim
  2018-11-27  9:34                     ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-27  8:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Date: Fri, 23 Nov 2018 16:58:41 +0100
>> Cc: emacs-devel@gnu.org
>> 
>> > Ah, okay.  Then we agree: there's some bug on NS that causes the face
>> > to be extended.  The expected behavior is the one we see on X and on
>> > Windows (and also on TTY frames).
>> 
>> Any idea where the code that handles this is? I took a quick look at
>> HAVE_NS, but nothing jumped out at me.
>
> I didn't see something like that, either.  But it could be the other
> way around: that NS needs some code to prevent that from happening.
>
> I think the place to look is ns_maybe_dumpglyphs_background: what is
> the width of the rectangle that it clears with the background color of
> the face?

Itʼs ns_dumpglyphs_stretch, which is doing exactly as itʼs told:
drawing the background to the right edge of the frame. Thatʼs because
in xdisp.c:display_line we have:

		      /* Make sure that a non-default face is extended
			 up to the right margin of the window.  */
                      extend_face_to_end_of_line (it);

which later causes set_glyph_string_background_width to set the
background_width of the glyph to the full width (commenting out that
call makes NS work the same as X11 for this specific case).

What I donʼt understand is that under X11 Emacs takes exactly the same
code path, yet somehow it doesnʼt end up setting the background up to
the right edge.

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-27  8:56                   ` Robert Pluim
@ 2018-11-27  9:34                     ` Eli Zaretskii
  2018-11-27 11:02                       ` Robert Pluim
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-27  9:34 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 27 Nov 2018 09:56:36 +0100
> 
> Itʼs ns_dumpglyphs_stretch, which is doing exactly as itʼs told:
> drawing the background to the right edge of the frame. Thatʼs because
> in xdisp.c:display_line we have:
> 
> 		      /* Make sure that a non-default face is extended
> 			 up to the right margin of the window.  */
>                       extend_face_to_end_of_line (it);
> 
> which later causes set_glyph_string_background_width to set the
> background_width of the glyph to the full width (commenting out that
> call makes NS work the same as X11 for this specific case).
> 
> What I donʼt understand is that under X11 Emacs takes exactly the same
> code path, yet somehow it doesnʼt end up setting the background up to
> the right edge.

On a GUI frame, extend_face_to_end_of_line is supposed to return
almost immediately, here:

  if (FRAME_WINDOW_P (f)
      && MATRIX_ROW_DISPLAYS_TEXT_P (it->glyph_row)
      && face->box == FACE_NO_BOX
      && face->background == FRAME_BACKGROUND_PIXEL (f)
#ifdef HAVE_WINDOW_SYSTEM
      && !face->stipple
#endif
      && !it->glyph_row->reversed_p)
    return;      <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

The exceptions are:

  (a) when we break a line in the middle of a face whose background is
      different from the default face, or
  (b) when the default face has the box or the stipple attribute set, or
  (b) if the screen line is displayed right-to-left

And none of those should be the case here.

So the question becomes: why doesn't NS reset it->face to the default
face, like it's supposed to?  We are already done with displaying the
stretch glyph, so its face should be bygone.

Let me know if you need guidance for where to look for the answers.

Thanks.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-27  9:34                     ` Eli Zaretskii
@ 2018-11-27 11:02                       ` Robert Pluim
  2018-11-27 11:29                         ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-27 11:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>
> On a GUI frame, extend_face_to_end_of_line is supposed to return
> almost immediately, here:
>
>   if (FRAME_WINDOW_P (f)
>       && MATRIX_ROW_DISPLAYS_TEXT_P (it->glyph_row)
>       && face->box == FACE_NO_BOX
>       && face->background == FRAME_BACKGROUND_PIXEL (f)
> #ifdef HAVE_WINDOW_SYSTEM
>       && !face->stipple
> #endif
>       && !it->glyph_row->reversed_p)
>     return;      <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>

When we get here, face->background == 1, and FRAME_BACKGROUND_PIXEL ==
0xfffeffff

Looking through nsfns.m, the problem becomes obvious: the NS port uses
indices into a color table to specify the background colour of faces,
and FRAME_BACKGROUND_PIXEL is an RGBA value.

Iʼm not sure how best to fix this. Thereʼs
f->output_data.ns->background_color, but that would still need to be
converted to RGBA.

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-27 11:02                       ` Robert Pluim
@ 2018-11-27 11:29                         ` Eli Zaretskii
  2018-11-27 13:55                           ` Robert Pluim
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-27 11:29 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 27 Nov 2018 12:02:51 +0100
> 
> >   if (FRAME_WINDOW_P (f)
> >       && MATRIX_ROW_DISPLAYS_TEXT_P (it->glyph_row)
> >       && face->box == FACE_NO_BOX
> >       && face->background == FRAME_BACKGROUND_PIXEL (f)
> > #ifdef HAVE_WINDOW_SYSTEM
> >       && !face->stipple
> > #endif
> >       && !it->glyph_row->reversed_p)
> >     return;      <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> >
> 
> When we get here, face->background == 1, and FRAME_BACKGROUND_PIXEL ==
> 0xfffeffff
> 
> Looking through nsfns.m, the problem becomes obvious: the NS port uses
> indices into a color table to specify the background colour of faces,
> and FRAME_BACKGROUND_PIXEL is an RGBA value.

Why does NS use indices here, and not RGBA values?

> Iʼm not sure how best to fix this.

If no better/cleaner idea emerges, how about having an NS-specific
code here that computed the it->face's background RGBA by indexing
into the color table, before comparing that with
FRAME_BACKGROUND_PIXEL?



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-27 11:29                         ` Eli Zaretskii
@ 2018-11-27 13:55                           ` Robert Pluim
  2018-11-27 18:55                             ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-27 13:55 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Tue, 27 Nov 2018 12:02:51 +0100
>> 
>> >   if (FRAME_WINDOW_P (f)
>> >       && MATRIX_ROW_DISPLAYS_TEXT_P (it->glyph_row)
>> >       && face->box == FACE_NO_BOX
>> >       && face->background == FRAME_BACKGROUND_PIXEL (f)
>> > #ifdef HAVE_WINDOW_SYSTEM
>> >       && !face->stipple
>> > #endif
>> >       && !it->glyph_row->reversed_p)
>> >     return;      <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>> >
>> 
>> When we get here, face->background == 1, and FRAME_BACKGROUND_PIXEL ==
>> 0xfffeffff
>> 
>> Looking through nsfns.m, the problem becomes obvious: the NS port uses
>> indices into a color table to specify the background colour of faces,
>> and FRAME_BACKGROUND_PIXEL is an RGBA value.
>
> Why does NS use indices here, and not RGBA values?

Probably because NS doesnʼt really use the RGBA values directly at
all, but uses the indices all the time.

>> Iʼm not sure how best to fix this.
>
> If no better/cleaner idea emerges, how about having an NS-specific
> code here that computed the it->face's background RGBA by indexing
> into the color table, before comparing that with
> FRAME_BACKGROUND_PIXEL?

Thatʼs doable. I had a quick try of the opposite, storing the colour
index from the face in FRAME_BACKGROUND_PIXEL, but that ran into all
sorts of issues with face/frame initialisation order.

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-27 13:55                           ` Robert Pluim
@ 2018-11-27 18:55                             ` Eli Zaretskii
  2018-11-27 19:14                               ` Robert Pluim
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-27 18:55 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 27 Nov 2018 14:55:23 +0100
> 
> >> When we get here, face->background == 1, and FRAME_BACKGROUND_PIXEL ==
> >> 0xfffeffff
> >> 
> >> Looking through nsfns.m, the problem becomes obvious: the NS port uses
> >> indices into a color table to specify the background colour of faces,
> >> and FRAME_BACKGROUND_PIXEL is an RGBA value.
> >
> > Why does NS use indices here, and not RGBA values?
> 
> Probably because NS doesnʼt really use the RGBA values directly at
> all, but uses the indices all the time.

So where does the RGBA value in FRAME_BACKGROUND_PIXEL come from?

> > If no better/cleaner idea emerges, how about having an NS-specific
> > code here that computed the it->face's background RGBA by indexing
> > into the color table, before comparing that with
> > FRAME_BACKGROUND_PIXEL?
> 
> Thatʼs doable.

Thanks.

Note that there are a couple more of such comparisons, so perhaps a
macro is in order, with different implementations for NS and the rest.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-27 18:55                             ` Eli Zaretskii
@ 2018-11-27 19:14                               ` Robert Pluim
  2018-11-27 19:38                                 ` Robert Pluim
  2018-11-28  7:13                                 ` Eli Zaretskii
  0 siblings, 2 replies; 41+ messages in thread
From: Robert Pluim @ 2018-11-27 19:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Tue, 27 Nov 2018 14:55:23 +0100
>> 
>> >> When we get here, face->background == 1, and FRAME_BACKGROUND_PIXEL ==
>> >> 0xfffeffff
>> >> 
>> >> Looking through nsfns.m, the problem becomes obvious: the NS port uses
>> >> indices into a color table to specify the background colour of faces,
>> >> and FRAME_BACKGROUND_PIXEL is an RGBA value.
>> >
>> > Why does NS use indices here, and not RGBA values?
>> 
>> Probably because NS doesnʼt really use the RGBA values directly at
>> all, but uses the indices all the time.
>
> So where does the RGBA value in FRAME_BACKGROUND_PIXEL come from?
>
>> > If no better/cleaner idea emerges, how about having an NS-specific
>> > code here that computed the it->face's background RGBA by indexing
>> > into the color table, before comparing that with
>> > FRAME_BACKGROUND_PIXEL?
>> 
>> Thatʼs doable.
>
> Thanks.
>
> Note that there are a couple more of such comparisons, so perhaps a
> macro is in order, with different implementations for NS and the rest.

I love those rare moments when I implement something one way, and then
Eli tells me to do it that way :-)

In theory there are a couple of other fields in the face structure
that might need the same type of treatment, but I didnʼt see any code
comparing them to RGBA pixel values.

I suck at naming things, comments welcome.

diff --git i/src/dispextern.h w/src/dispextern.h
index 579665c2ff..776d14080e 100644
--- i/src/dispextern.h
+++ w/src/dispextern.h
@@ -74,10 +74,13 @@ typedef HDC XImagePtr_or_DC;
 
 #ifdef HAVE_NS
 #include "nsgui.h"
+#define FACE_COLOR_TO_PIXEL(face_color, frame) ns_color_index_to_rgba(face_color, frame)
 /* Following typedef needed to accommodate the MSDOS port, believe it or not.  */
 typedef struct ns_display_info Display_Info;
 typedef Pixmap XImagePtr;
 typedef XImagePtr XImagePtr_or_DC;
+#else
+#define FACE_COLOR_TO_PIXEL(face_color, frame) face_color
 #endif
 
 #ifdef HAVE_WINDOW_SYSTEM
diff --git i/src/nsgui.h w/src/nsgui.h
index 4e7d7d35da..0c29eed5e4 100644
--- i/src/nsgui.h
+++ w/src/nsgui.h
@@ -73,6 +73,8 @@ typedef unichar XChar2b;
 #define XCHAR2B_BYTE2(chp) \
   (*(chp) & 0x00ff)
 
+/* For xdisp.c */
+extern unsigned long ns_color_index_to_rgba(int idx, struct frame *f);
 
 /* XXX: xfaces requires these structures, but the question is are we
         forced to use them?  */
diff --git i/src/nsterm.m w/src/nsterm.m
index bcc23ffeaf..cacfc57fd7 100644
--- i/src/nsterm.m
+++ w/src/nsterm.m
@@ -2356,6 +2356,22 @@ so some key presses (TAB) are swallowed by the system.  */
   return 1;
 }
 
+/* Convert an index into the color table into an RGBA value.  Used in
+   xdisp.c:extend_face_to_end_of_line when comparing faces and frame
+   color values.  */
+
+unsigned long
+ns_color_index_to_rgba(int idx, struct frame *f)
+{
+  NSColor *col;
+  col = ns_lookup_indexed_color (idx, f);
+
+  EmacsCGFloat r, g, b, a;
+  [col getRed: &r green: &g blue: &b alpha: &a];
+
+  return ARGB_TO_ULONG((int)(a*255),
+                       (int)(r*255), (int)(g*255), (int)(b*255));
+}
 
 void
 ns_query_color(void *col, XColor *color_def, int setPixel)
diff --git i/src/xdisp.c w/src/xdisp.c
index fa7691cdd0..e9048afb86 100644
--- i/src/xdisp.c
+++ w/src/xdisp.c
@@ -20287,7 +20287,7 @@ extend_face_to_end_of_line (struct it *it)
   if (FRAME_WINDOW_P (f)
       && MATRIX_ROW_DISPLAYS_TEXT_P (it->glyph_row)
       && face->box == FACE_NO_BOX
-      && face->background == FRAME_BACKGROUND_PIXEL (f)
+      && FACE_COLOR_TO_PIXEL(face->background, f) == FRAME_BACKGROUND_PIXEL (f)
 #ifdef HAVE_WINDOW_SYSTEM
       && !face->stipple
 #endif
@@ -20432,7 +20432,7 @@ extend_face_to_end_of_line (struct it *it)
 	  && (it->glyph_row->used[LEFT_MARGIN_AREA]
 	      < WINDOW_LEFT_MARGIN_WIDTH (it->w))
 	  && !it->glyph_row->mode_line_p
-	  && default_face->background != FRAME_BACKGROUND_PIXEL (f))
+	  && FACE_COLOR_TO_PIXEL(face->background, f) != FRAME_BACKGROUND_PIXEL (f))
 	{
 	  struct glyph *g = it->glyph_row->glyphs[LEFT_MARGIN_AREA];
 	  struct glyph *e = g + it->glyph_row->used[LEFT_MARGIN_AREA];
@@ -20473,7 +20473,7 @@ extend_face_to_end_of_line (struct it *it)
 	  && (it->glyph_row->used[RIGHT_MARGIN_AREA]
 	      < WINDOW_RIGHT_MARGIN_WIDTH (it->w))
 	  && !it->glyph_row->mode_line_p
-	  && default_face->background != FRAME_BACKGROUND_PIXEL (f))
+	  && FACE_COLOR_TO_PIXEL(face->background, f) != FRAME_BACKGROUND_PIXEL (f))
 	{
 	  struct glyph *g = it->glyph_row->glyphs[RIGHT_MARGIN_AREA];
 	  struct glyph *e = g + it->glyph_row->used[RIGHT_MARGIN_AREA];



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-27 19:14                               ` Robert Pluim
@ 2018-11-27 19:38                                 ` Robert Pluim
  2018-11-28  6:03                                   ` Eli Zaretskii
  2018-11-28  7:13                                 ` Eli Zaretskii
  1 sibling, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-27 19:38 UTC (permalink / raw)
  To: emacs-devel

Robert Pluim <rpluim@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Robert Pluim <rpluim@gmail.com>
>>> Cc: emacs-devel@gnu.org
>>> Date: Tue, 27 Nov 2018 14:55:23 +0100
>>> 
>>> >> When we get here, face->background == 1, and FRAME_BACKGROUND_PIXEL ==
>>> >> 0xfffeffff
>>> >> 
>>> >> Looking through nsfns.m, the problem becomes obvious: the NS port uses
>>> >> indices into a color table to specify the background colour of faces,
>>> >> and FRAME_BACKGROUND_PIXEL is an RGBA value.
>>> >
>>> > Why does NS use indices here, and not RGBA values?
>>> 
>>> Probably because NS doesnʼt really use the RGBA values directly at
>>> all, but uses the indices all the time.
>>
>> So where does the RGBA value in FRAME_BACKGROUND_PIXEL come from?
>>
>>> > If no better/cleaner idea emerges, how about having an NS-specific
>>> > code here that computed the it->face's background RGBA by indexing
>>> > into the color table, before comparing that with
>>> > FRAME_BACKGROUND_PIXEL?

(showing my ignorance of redisplay) Is there a reason we need to
compare with the background of the frame rather than with the
background of the default face of the frame?

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-27 19:38                                 ` Robert Pluim
@ 2018-11-28  6:03                                   ` Eli Zaretskii
  2018-11-28  9:00                                     ` Robert Pluim
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-28  6:03 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Date: Tue, 27 Nov 2018 20:38:05 +0100
> 
> (showing my ignorance of redisplay)

Everybody is "ignorant" about redisplay, including your truly.  E.g.,
I learned just a couple of weeks ago something quite important about
it that I succeeded to miss in the last 10 years I'm actively hacking
that code: that try_window_id calls display update routines directly.

> Is there a reason we need to compare with the background of the
> frame rather than with the background of the default face of the
> frame?

Basically, because we clear rectangles on display using the frame's
background color.

Historically, once upon a time, the frame's background was not
necessarily identical to that of the default face.  And given that we
already have the color recorded in the frame, it is less expensive to
access that than look up the default face, which might involve
accessing face-remapping-alist etc.  Finally, the frame's face cache,
which is where we look up faces, is reset from time to time, whereas
the frame's background color is always valid.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-27 19:14                               ` Robert Pluim
  2018-11-27 19:38                                 ` Robert Pluim
@ 2018-11-28  7:13                                 ` Eli Zaretskii
  2018-11-28  8:36                                   ` Robert Pluim
  2018-11-28 21:14                                   ` Alan Third
  1 sibling, 2 replies; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-28  7:13 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 27 Nov 2018 20:14:14 +0100
> 
> >> > Why does NS use indices here, and not RGBA values?
> >> 
> >> Probably because NS doesnʼt really use the RGBA values directly at
> >> all, but uses the indices all the time.
> >
> > So where does the RGBA value in FRAME_BACKGROUND_PIXEL come from?

Can you answer this question?

> I love those rare moments when I implement something one way, and then
> Eli tells me to do it that way :-)
> 
> In theory there are a couple of other fields in the face structure
> that might need the same type of treatment, but I didnʼt see any code
> comparing them to RGBA pixel values.
> 
> I suck at naming things, comments welcome.

LGTM, thanks.  Let's wait for comments from NS people.

> -      && face->background == FRAME_BACKGROUND_PIXEL (f)
> +      && FACE_COLOR_TO_PIXEL(face->background, f) == FRAME_BACKGROUND_PIXEL (f)
                               ^
Please leave one space between the macro name and the opening paren.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-28  7:13                                 ` Eli Zaretskii
@ 2018-11-28  8:36                                   ` Robert Pluim
  2018-11-28  9:45                                     ` Eli Zaretskii
  2018-11-28 21:14                                   ` Alan Third
  1 sibling, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-28  8:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Tue, 27 Nov 2018 20:14:14 +0100
>> 
>> >> > Why does NS use indices here, and not RGBA values?
>> >> 
>> >> Probably because NS doesnʼt really use the RGBA values directly at
>> >> all, but uses the indices all the time.
>> >
>> > So where does the RGBA value in FRAME_BACKGROUND_PIXEL come from?
>
> Can you answer this question?

Itʼs set by the NS version of x_set_background_color, so itʼs correct,
just not the same format as the face background colour.

>> I love those rare moments when I implement something one way, and then
>> Eli tells me to do it that way :-)
>> 
>> In theory there are a couple of other fields in the face structure
>> that might need the same type of treatment, but I didnʼt see any code
>> comparing them to RGBA pixel values.
>> 
>> I suck at naming things, comments welcome.
>
> LGTM, thanks.  Let's wait for comments from NS people.
>
>> -      && face->background == FRAME_BACKGROUND_PIXEL (f)
>> +      && FACE_COLOR_TO_PIXEL(face->background, f) == FRAME_BACKGROUND_PIXEL (f)
>                                ^
> Please leave one space between the macro name and the opening paren.

Fixed (plus a missing space at the end of a comment sentence).

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-28  6:03                                   ` Eli Zaretskii
@ 2018-11-28  9:00                                     ` Robert Pluim
  2018-11-28  9:42                                       ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-28  9:00 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Date: Tue, 27 Nov 2018 20:38:05 +0100

>> Is there a reason we need to compare with the background of the
>> frame rather than with the background of the default face of the
>> frame?
>
> Basically, because we clear rectangles on display using the frame's
> background color.
>
> Historically, once upon a time, the frame's background was not
> necessarily identical to that of the default face.  And given that we
> already have the color recorded in the frame, it is less expensive to
> access that than look up the default face, which might involve
> accessing face-remapping-alist etc.  Finally, the frame's face cache,
> which is where we look up faces, is reset from time to time, whereas
> the frame's background color is always valid.

In this particular case extend_face_to_end_of_line has already done:

  default_face =
    FACE_FROM_ID_OR_NULL (f, lookup_basic_face (it->w, f, DEFAULT_FACE_ID));

so there'd be no additional cost, but that _OR_NULL bit precludes
always using the default face.

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-28  9:00                                     ` Robert Pluim
@ 2018-11-28  9:42                                       ` Eli Zaretskii
  2018-11-28  9:49                                         ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-28  9:42 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 28 Nov 2018 10:00:53 +0100
> 
> In this particular case extend_face_to_end_of_line has already done:
> 
>   default_face =
>     FACE_FROM_ID_OR_NULL (f, lookup_basic_face (it->w, f, DEFAULT_FACE_ID));
> 
> so there'd be no additional cost

Yes, but this part could (and probably should) be moved to after the
'return', because default_face is not used before that.

> but that _OR_NULL bit precludes always using the default face.

There's no special significance to the _OR_NULL bit in this case.  If
we moved that part to after the 'return', we should probably use
FACE_FROM_ID instead, as the face pointer is actually dereferenced.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-28  8:36                                   ` Robert Pluim
@ 2018-11-28  9:45                                     ` Eli Zaretskii
  2018-11-28  9:56                                       ` Robert Pluim
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-28  9:45 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 28 Nov 2018 09:36:53 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Robert Pluim <rpluim@gmail.com>
> >> Cc: emacs-devel@gnu.org
> >> Date: Tue, 27 Nov 2018 20:14:14 +0100
> >> 
> >> >> > Why does NS use indices here, and not RGBA values?
> >> >> 
> >> >> Probably because NS doesnʼt really use the RGBA values directly at
> >> >> all, but uses the indices all the time.
> >> >
> >> > So where does the RGBA value in FRAME_BACKGROUND_PIXEL come from?
> >
> > Can you answer this question?
> 
> Itʼs set by the NS version of x_set_background_color, so itʼs correct,
> just not the same format as the face background colour.

So they call ns_lisp_to_color once and store the result?

I still wonder why they don't do this with other face colors.  Is
ns_lisp_to_color expensive or something?

> >> -      && face->background == FRAME_BACKGROUND_PIXEL (f)
> >> +      && FACE_COLOR_TO_PIXEL(face->background, f) == FRAME_BACKGROUND_PIXEL (f)
> >                                ^
> > Please leave one space between the macro name and the opening paren.
> 
> Fixed (plus a missing space at the end of a comment sentence).

Thanks.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-28  9:42                                       ` Eli Zaretskii
@ 2018-11-28  9:49                                         ` Eli Zaretskii
  2018-11-28 13:24                                           ` Robert Pluim
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-28  9:49 UTC (permalink / raw)
  To: rpluim; +Cc: emacs-devel

> Date: Wed, 28 Nov 2018 11:42:40 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > From: Robert Pluim <rpluim@gmail.com>
> > Cc: emacs-devel@gnu.org
> > Date: Wed, 28 Nov 2018 10:00:53 +0100
> > 
> > In this particular case extend_face_to_end_of_line has already done:
> > 
> >   default_face =
> >     FACE_FROM_ID_OR_NULL (f, lookup_basic_face (it->w, f, DEFAULT_FACE_ID));
> > 
> > so there'd be no additional cost
> 
> Yes, but this part could (and probably should) be moved to after the
> 'return', because default_face is not used before that.

And btw, when the default face is remapped, its background color no
longer has to be the same as the frame's background, right?  Which I
guess is another reason why we use FRAME_BACKGROUND_PIXEL there.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-28  9:45                                     ` Eli Zaretskii
@ 2018-11-28  9:56                                       ` Robert Pluim
  2018-11-28 10:11                                         ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-28  9:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> >> > So where does the RGBA value in FRAME_BACKGROUND_PIXEL come from?
>> >
>> > Can you answer this question?
>> 
>> Itʼs set by the NS version of x_set_background_color, so itʼs correct,
>> just not the same format as the face background colour.
>
> So they call ns_lisp_to_color once and store the result?

Well, they allocate an entry in the frame color_table, store the
result of ns_lisp_to_color there, and then use an index into the table
from there on. And convert it to RGBA and store it in
FRAME_BACKGROUND_PIXEL.

> I still wonder why they don't do this with other face colors.  Is
> ns_lisp_to_color expensive or something?

Donʼt do what with other face colours? As far as I can tell the same
thing is done for the foreground (and ns_lisp_to_color does look
expensive, as it parses colour names and RGB specs and probably more).

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-28  9:56                                       ` Robert Pluim
@ 2018-11-28 10:11                                         ` Eli Zaretskii
  2018-11-28 13:21                                           ` Robert Pluim
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-28 10:11 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 28 Nov 2018 10:56:23 +0100
> 
> > I still wonder why they don't do this with other face colors.  Is
> > ns_lisp_to_color expensive or something?
> 
> Donʼt do what with other face colours?

Why they don't do the same with face colors as they do with
FRAME_BACKGROUND_PIXEL, i.e. convert to RGBA and store the RGBA in the
face colors, instead of the indices.

> (and ns_lisp_to_color does look expensive, as it parses colour names
> and RGB specs and probably more).

Maybe that's the reason, then.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-28 10:11                                         ` Eli Zaretskii
@ 2018-11-28 13:21                                           ` Robert Pluim
  2018-11-28 16:20                                             ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-28 13:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Wed, 28 Nov 2018 10:56:23 +0100
>> 
>> > I still wonder why they don't do this with other face colors.  Is
>> > ns_lisp_to_color expensive or something?
>> 
>> Donʼt do what with other face colours?
>
> Why they don't do the same with face colors as they do with
> FRAME_BACKGROUND_PIXEL, i.e. convert to RGBA and store the RGBA in the
> face colors, instead of the indices.

Because then every time they want to do something with a face colour,
they'd have to convert it from RGBA, allocate the corresponding
NSColor, use it, destroy it etc. Instead the code creates the 'native'
color object, and stores it in the face, so no conversions are
necessary.

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-28  9:49                                         ` Eli Zaretskii
@ 2018-11-28 13:24                                           ` Robert Pluim
  2018-11-28 16:19                                             ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-28 13:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Wed, 28 Nov 2018 11:42:40 +0200
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: emacs-devel@gnu.org
>> 
>> > From: Robert Pluim <rpluim@gmail.com>
>> > Cc: emacs-devel@gnu.org
>> > Date: Wed, 28 Nov 2018 10:00:53 +0100
>> > 
>> > In this particular case extend_face_to_end_of_line has already done:
>> > 
>> >   default_face =
>> >     FACE_FROM_ID_OR_NULL (f, lookup_basic_face (it->w, f, DEFAULT_FACE_ID));
>> > 
>> > so there'd be no additional cost
>> 
>> Yes, but this part could (and probably should) be moved to after the
>> 'return', because default_face is not used before that.
>
> And btw, when the default face is remapped, its background color no
> longer has to be the same as the frame's background, right?  Which I
> guess is another reason why we use FRAME_BACKGROUND_PIXEL there.

OK, we can stick with that if no other proposals come up.

Moving the default_face lookup down in the function I can test for a
while. Iʼll even remove the _OR_NULL, since we donʼt check for NULL
anywhere anyway.

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-28 13:24                                           ` Robert Pluim
@ 2018-11-28 16:19                                             ` Eli Zaretskii
  0 siblings, 0 replies; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-28 16:19 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 28 Nov 2018 14:24:59 +0100
> 
> Moving the default_face lookup down in the function I can test for a
> while. Iʼll even remove the _OR_NULL, since we donʼt check for NULL
> anywhere anyway.

Right, thanks.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-28 13:21                                           ` Robert Pluim
@ 2018-11-28 16:20                                             ` Eli Zaretskii
  2018-11-29 12:51                                               ` Robert Pluim
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-28 16:20 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 28 Nov 2018 14:21:38 +0100
> 
> > Why they don't do the same with face colors as they do with
> > FRAME_BACKGROUND_PIXEL, i.e. convert to RGBA and store the RGBA in the
> > face colors, instead of the indices.
> 
> Because then every time they want to do something with a face colour,
> they'd have to convert it from RGBA, allocate the corresponding
> NSColor, use it, destroy it etc.

But that's how every other platform does that, AFAIU...



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-28  7:13                                 ` Eli Zaretskii
  2018-11-28  8:36                                   ` Robert Pluim
@ 2018-11-28 21:14                                   ` Alan Third
  2018-11-29 12:26                                     ` Robert Pluim
  1 sibling, 1 reply; 41+ messages in thread
From: Alan Third @ 2018-11-28 21:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Robert Pluim, emacs-devel

On Wed, Nov 28, 2018 at 09:13:36AM +0200, Eli Zaretskii wrote:
> > From: Robert Pluim <rpluim@gmail.com>
> > Cc: emacs-devel@gnu.org
> > Date: Tue, 27 Nov 2018 20:14:14 +0100
> > In theory there are a couple of other fields in the face structure
> > that might need the same type of treatment, but I didnʼt see any code
> > comparing them to RGBA pixel values.
> > 
> > I suck at naming things, comments welcome.
> 
> LGTM, thanks.  Let's wait for comments from NS people.

Me too.
-- 
Alan Third



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-28 21:14                                   ` Alan Third
@ 2018-11-29 12:26                                     ` Robert Pluim
  2018-11-29 12:54                                       ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-29 12:26 UTC (permalink / raw)
  To: Alan Third; +Cc: Eli Zaretskii, emacs-devel

Alan Third <alan@idiocy.org> writes:

> On Wed, Nov 28, 2018 at 09:13:36AM +0200, Eli Zaretskii wrote:
>> > From: Robert Pluim <rpluim@gmail.com>
>> > Cc: emacs-devel@gnu.org
>> > Date: Tue, 27 Nov 2018 20:14:14 +0100
>> > In theory there are a couple of other fields in the face structure
>> > that might need the same type of treatment, but I didnʼt see any code
>> > comparing them to RGBA pixel values.
>> > 
>> > I suck at naming things, comments welcome.
>> 
>> LGTM, thanks.  Let's wait for comments from NS people.
>
> Me too.

Eli, release or master branch?

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-28 16:20                                             ` Eli Zaretskii
@ 2018-11-29 12:51                                               ` Robert Pluim
  2018-11-29 14:15                                                 ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-29 12:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Wed, 28 Nov 2018 14:21:38 +0100
>> 
>> > Why they don't do the same with face colors as they do with
>> > FRAME_BACKGROUND_PIXEL, i.e. convert to RGBA and store the RGBA in the
>> > face colors, instead of the indices.
>> 
>> Because then every time they want to do something with a face colour,
>> they'd have to convert it from RGBA, allocate the corresponding
>> NSColor, use it, destroy it etc.
>
> But that's how every other platform does that, AFAIU...

They do? X11 and W32 seem to use RGBA values directly, mostly.

I donʼt think this is worth changing: it works, and rewriting it will
just introduce bugs.

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-29 12:26                                     ` Robert Pluim
@ 2018-11-29 12:54                                       ` Eli Zaretskii
  2018-11-29 13:52                                         ` Robert Pluim
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-29 12:54 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Thu, 29 Nov 2018 13:26:03 +0100
> 
> >> LGTM, thanks.  Let's wait for comments from NS people.
> >
> > Me too.
> 
> Eli, release or master branch?

Master, I think.  Unless someone has good reasons why we must have
this in Emacs 26.2.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-29 12:54                                       ` Eli Zaretskii
@ 2018-11-29 13:52                                         ` Robert Pluim
  2018-11-29 14:08                                           ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-29 13:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Keith David Bershatsky, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
>> Date: Thu, 29 Nov 2018 13:26:03 +0100
>> 
>> >> LGTM, thanks.  Let's wait for comments from NS people.
>> >
>> > Me too.
>> 
>> Eli, release or master branch?
>
> Master, I think.  Unless someone has good reasons why we must have
> this in Emacs 26.2.

Since this originally came from a request by Keith, and he's working
on master, I can't see any reason to put it in 26.2 either.

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-29 13:52                                         ` Robert Pluim
@ 2018-11-29 14:08                                           ` Eli Zaretskii
  2018-11-30  7:59                                             ` Robert Pluim
  2018-11-30  8:04                                             ` Robert Pluim
  0 siblings, 2 replies; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-29 14:08 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org, Keith David Bershatsky <esq@lawlist.com>
> Date: Thu, 29 Nov 2018 14:52:01 +0100
> 
> > Master, I think.  Unless someone has good reasons why we must have
> > this in Emacs 26.2.
> 
> Since this originally came from a request by Keith, and he's working
> on master, I can't see any reason to put it in 26.2 either.

Then master it is.

Thanks.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-29 12:51                                               ` Robert Pluim
@ 2018-11-29 14:15                                                 ` Eli Zaretskii
  0 siblings, 0 replies; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-29 14:15 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 29 Nov 2018 13:51:39 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> >> Because then every time they want to do something with a face colour,
> >> they'd have to convert it from RGBA, allocate the corresponding
> >> NSColor, use it, destroy it etc.
> >
> > But that's how every other platform does that, AFAIU...
> 
> They do? X11 and W32 seem to use RGBA values directly, mostly.

I meant x_defined_color and its subroutines.

> I donʼt think this is worth changing: it works, and rewriting it will
> just introduce bugs.

I don't necessarily disagree, I just don't yet see why NS should be
different in this respect.  Feel free to ignore me on that, though.

Thanks.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-29 14:08                                           ` Eli Zaretskii
@ 2018-11-30  7:59                                             ` Robert Pluim
  2018-11-30  8:04                                             ` Robert Pluim
  1 sibling, 0 replies; 41+ messages in thread
From: Robert Pluim @ 2018-11-30  7:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: emacs-devel@gnu.org, Keith David Bershatsky <esq@lawlist.com>
>> Date: Thu, 29 Nov 2018 14:52:01 +0100
>> 
>> > Master, I think.  Unless someone has good reasons why we must have
>> > this in Emacs 26.2.
>> 
>> Since this originally came from a request by Keith, and he's working
>> on master, I can't see any reason to put it in 26.2 either.
>
> Then master it is.

Pushed as

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-29 14:08                                           ` Eli Zaretskii
  2018-11-30  7:59                                             ` Robert Pluim
@ 2018-11-30  8:04                                             ` Robert Pluim
  2018-11-30  8:22                                               ` Eli Zaretskii
  1 sibling, 1 reply; 41+ messages in thread
From: Robert Pluim @ 2018-11-30  8:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: emacs-devel@gnu.org, Keith David Bershatsky <esq@lawlist.com>
>> Date: Thu, 29 Nov 2018 14:52:01 +0100
>> 
>> > Master, I think.  Unless someone has good reasons why we must have
>> > this in Emacs 26.2.
>> 
>> Since this originally came from a request by Keith, and he's working
>> on master, I can't see any reason to put it in 26.2 either.
>
> Then master it is.

Hit send too soon. Pushed as 5f67353da7

Robert



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: Identifying the face between STRETCH and right fringe.
  2018-11-30  8:04                                             ` Robert Pluim
@ 2018-11-30  8:22                                               ` Eli Zaretskii
  0 siblings, 0 replies; 41+ messages in thread
From: Eli Zaretskii @ 2018-11-30  8:22 UTC (permalink / raw)
  To: Robert Pluim; +Cc: emacs-devel

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Fri, 30 Nov 2018 09:04:05 +0100
> 
> > Then master it is.
> 
> Hit send too soon. Pushed as 5f67353da7

Thanks.



^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2018-11-30  8:22 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-20 16:39 Identifying the face between STRETCH and right fringe Keith David Bershatsky
2018-11-20 17:20 ` Eli Zaretskii
2018-11-21  7:44   ` Robert Pluim
2018-11-23 10:04     ` Eli Zaretskii
2018-11-23 13:25       ` Robert Pluim
2018-11-23 13:48         ` Eli Zaretskii
2018-11-23 14:04           ` Robert Pluim
2018-11-23 15:51             ` Eli Zaretskii
2018-11-23 15:58               ` Robert Pluim
2018-11-23 20:33                 ` Eli Zaretskii
2018-11-27  8:56                   ` Robert Pluim
2018-11-27  9:34                     ` Eli Zaretskii
2018-11-27 11:02                       ` Robert Pluim
2018-11-27 11:29                         ` Eli Zaretskii
2018-11-27 13:55                           ` Robert Pluim
2018-11-27 18:55                             ` Eli Zaretskii
2018-11-27 19:14                               ` Robert Pluim
2018-11-27 19:38                                 ` Robert Pluim
2018-11-28  6:03                                   ` Eli Zaretskii
2018-11-28  9:00                                     ` Robert Pluim
2018-11-28  9:42                                       ` Eli Zaretskii
2018-11-28  9:49                                         ` Eli Zaretskii
2018-11-28 13:24                                           ` Robert Pluim
2018-11-28 16:19                                             ` Eli Zaretskii
2018-11-28  7:13                                 ` Eli Zaretskii
2018-11-28  8:36                                   ` Robert Pluim
2018-11-28  9:45                                     ` Eli Zaretskii
2018-11-28  9:56                                       ` Robert Pluim
2018-11-28 10:11                                         ` Eli Zaretskii
2018-11-28 13:21                                           ` Robert Pluim
2018-11-28 16:20                                             ` Eli Zaretskii
2018-11-29 12:51                                               ` Robert Pluim
2018-11-29 14:15                                                 ` Eli Zaretskii
2018-11-28 21:14                                   ` Alan Third
2018-11-29 12:26                                     ` Robert Pluim
2018-11-29 12:54                                       ` Eli Zaretskii
2018-11-29 13:52                                         ` Robert Pluim
2018-11-29 14:08                                           ` Eli Zaretskii
2018-11-30  7:59                                             ` Robert Pluim
2018-11-30  8:04                                             ` Robert Pluim
2018-11-30  8:22                                               ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).