GNU bug report logs - #29353
OSX/MacOS: Adding support for window-divider-first/last-pixel

Previous Next

Package: emacs;

Reported by: Keith David Bershatsky <esq <at> lawlist.com>

Date: Sat, 18 Nov 2017 23:30:02 UTC

Severity: normal

Done: Alan Third <alan <at> idiocy.org>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#29353; Package emacs. (Sat, 18 Nov 2017 23:30:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Keith David Bershatsky <esq <at> lawlist.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sat, 18 Nov 2017 23:30:02 GMT) Full text and rfc822 format available.

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

From: Keith David Bershatsky <esq <at> lawlist.com>
To: Emacs Bug Reports <bug-gnu-emacs <at> gnu.org>
Subject: OSX/MacOS:  Adding support for window-divider-first/last-pixel
Date: Sat, 18 Nov 2017 15:29:19 -0800
Upon grepping the source code for window_divider_first_pixel and window_divider_last_pixel, I see no existing support for OSX/MacOS builds.

Although I have not done any testing on other platforms, it appears that support for these features already exists for Windows and X11 builds.

It would be nifty if OSX/MacOS users could also enjoy this cool feature.

Thanks,

Keith




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29353; Package emacs. (Sun, 19 Nov 2017 11:54:02 GMT) Full text and rfc822 format available.

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

From: Alan Third <alan <at> idiocy.org>
To: Keith David Bershatsky <esq <at> lawlist.com>
Cc: 29353 <at> debbugs.gnu.org
Subject: [PATCH] Add window divider faces to NS (bug#29353)
Date: Sun, 19 Nov 2017 11:53:13 +0000
* src/nsterm.m (ns_draw_window_divider): Use
window-divider-first-pixel and window-divider-last-pixel faces.
---
 src/nsterm.m | 46 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index 5c29f039e5..b63c85fc56 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3174,18 +3174,50 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
    -------------------------------------------------------------------------- */
 {
   struct frame *f = XFRAME (WINDOW_FRAME (w));
-  struct face *face;
-  NSRect r = NSMakeRect (x0, y0, x1-x0, y1-y0);
+  struct face *face = FACE_FROM_ID_OR_NULL (f, WINDOW_DIVIDER_FACE_ID);
+  struct face *face_first
+    = FACE_FROM_ID_OR_NULL (f, WINDOW_DIVIDER_FIRST_PIXEL_FACE_ID);
+  struct face *face_last
+    = FACE_FROM_ID_OR_NULL (f, WINDOW_DIVIDER_LAST_PIXEL_FACE_ID);
+  unsigned long color = face ? face->foreground : FRAME_FOREGROUND_PIXEL (f);
+  unsigned long color_first = (face_first
+			       ? face_first->foreground
+			       : FRAME_FOREGROUND_PIXEL (f));
+  unsigned long color_last = (face_last
+			      ? face_last->foreground
+			      : FRAME_FOREGROUND_PIXEL (f));
+  NSRect divider = NSMakeRect (x0, y0, x1-x0, y1-y0);
 
   NSTRACE ("ns_draw_window_divider");
 
-  face = FACE_FROM_ID_OR_NULL (f, WINDOW_DIVIDER_FACE_ID);
+  ns_focus (f, &divider, 1);
 
-  ns_focus (f, &r, 1);
-  if (face)
-    [ns_lookup_indexed_color(face->foreground, f) set];
+  if (y1 - y0 > x1 - x0 && x1 - x0 > 2)
+    /* Vertical.  */
+    {
+      [ns_lookup_indexed_color(color_first, f) set];
+      NSRectFill(NSMakeRect (x0, y0, 1, y1 - y0));
+      [ns_lookup_indexed_color(color, f) set];
+      NSRectFill(NSMakeRect (x0 + 1, y0, x1 - x0 - 2, y1 - y0));
+      [ns_lookup_indexed_color(color_last, f) set];
+      NSRectFill(NSMakeRect (x1 - 1, y0, 1, y1 - y0));
+    }
+  else if (x1 - x0 > y1 - y0 && y1 - y0 > 3)
+    /* Horizontal.  */
+    {
+      [ns_lookup_indexed_color(color_first, f) set];
+      NSRectFill(NSMakeRect (x0, y0, x1 - x0, 1));
+      [ns_lookup_indexed_color(color, f) set];
+      NSRectFill(NSMakeRect (x0, y0 + 1, x1 - x0, y1 - y0 -2));
+      [ns_lookup_indexed_color(color_last, f) set];
+      NSRectFill(NSMakeRect (x0, y1 - 1, x1 - x0, 1));
+    }
+  else
+    {
+      [ns_lookup_indexed_color(color, f) set];
+      NSRectFill(divider);
+    }
 
-  NSRectFill(r);
   ns_unfocus (f);
 }
 
-- 
Hi Keith,

Can you please test this patch? I’m unsure how to set these faces so I
don’t know if it’s working right.

-- 
Alan Third




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29353; Package emacs. (Sun, 19 Nov 2017 20:41:01 GMT) Full text and rfc822 format available.

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

From: Keith David Bershatsky <esq <at> lawlist.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 29353 <at> debbugs.gnu.org
Subject: Re: [PATCH] Add window divider faces to NS (bug#29353)
Date: Sun, 19 Nov 2017 12:40:09 -0800
Thank you, Alan, for bringing this feature to Emacs on the OSX/MacOS platforms.

The only issue I observed with the patch applied is when a user selects a divider width of 3 for right and bottom.  In that situation, the bottom divider is entirely one color -- window-divider face.  And, the right divider is 2 pixels in the window-divider-first-pixel face and 1 pixel in the window-divider-last-pixel face.  When there are exactly 3 pixels, both dividers should have the rainbow of all three available colors in the applicable order.

The resolution on my screen is not the greatest in the world, but it looks like all other width variations are working properly.  I.e., 1 and 2 pixel widths properly have just the window-divider color (since there is agreeably no room for both the first/last pixel colors), and 4 pixels in width and above have the rainbow effect.

(face-spec-set 'default '((t :background "black" :foreground "red")))

(face-spec-set 'window-divider-first-pixel '((t :foreground "red")))

(face-spec-set 'window-divider '((t :foreground "DarkBlue")))

(face-spec-set 'window-divider-last-pixel '((t :foreground "magenta")))

(with-selected-frame
  (make-frame '((right-divider-width . 3) (bottom-divider-width . 3)))
  (split-window-horizontally))

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

DATE:  [11-19-2017 03:53:13] <19 Nov 2017 11:53:13 +0000>
FROM:  Alan Third <alan <at> idiocy.org>
> 
> * src/nsterm.m (ns_draw_window_divider): Use
> window-divider-first-pixel and window-divider-last-pixel faces.
> ---
> * * *




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29353; Package emacs. (Sun, 19 Nov 2017 22:13:02 GMT) Full text and rfc822 format available.

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

From: Keith David Bershatsky <esq <at> lawlist.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 29353 <at> debbugs.gnu.org
Subject: Re: [PATCH] Add window divider faces to NS (bug#29353)
Date: Sun, 19 Nov 2017 14:11:23 -0800
Alan:

It appears that the right divider is encroaching on the fringe to a tune of 1 pixel, i.e., the vertical divider is drawing on top of the left side of fringe bitmaps.  Is this related to the patch, or a new issue?

(face-spec-set 'default '((t :background "black" :foreground "red")))

(face-spec-set 'window-divider-first-pixel '((t :foreground "red")))

(face-spec-set 'window-divider '((t :foreground "DarkBlue")))

(face-spec-set 'window-divider-last-pixel '((t :foreground "magenta")))

(with-selected-frame
  (make-frame '((right-divider-width . 8) (bottom-divider-width . 8)))
  (split-window-horizontally))

(visual-line-mode)

(setq fringe-indicator-alist '(
  (truncation . (left-arrow right-arrow))
  (continuation . (left-curly-arrow right-curly-arrow))
  (overlay-arrow . right-triangle)
  (up . up-arrow)
  (down . down-arrow)
  (top . top-left-angle)
  (bottom . bottom-right-angle)
  (top-bottom . (left-bracket
                 right-bracket
                 top-right-angle
                 top-left-angle))
  (empty-line . empty-line)
  (unknown . question-mark)))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29353; Package emacs. (Sun, 19 Nov 2017 22:54:01 GMT) Full text and rfc822 format available.

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

From: Alan Third <alan <at> idiocy.org>
To: Keith David Bershatsky <esq <at> lawlist.com>
Cc: 29353 <at> debbugs.gnu.org
Subject: Re: [PATCH] Add window divider faces to NS (bug#29353)
Date: Sun, 19 Nov 2017 22:53:06 +0000
[Message part 1 (text/plain, inline)]
On Sun, Nov 19, 2017 at 02:11:23PM -0800, Keith David Bershatsky wrote:
> Alan:
> 
> It appears that the right divider is encroaching on the fringe to a
> tune of 1 pixel, i.e., the vertical divider is drawing on top of the
> left side of fringe bitmaps. Is this related to the patch, or a new
> issue?

It looks OK here. Screenshot attached. Am I misinterpreting you, or
are you seeing something different?
-- 
Alan Third
[fringes.png (image/png, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29353; Package emacs. (Sun, 19 Nov 2017 23:19:01 GMT) Full text and rfc822 format available.

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

From: Keith David Bershatsky <esq <at> lawlist.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 29353 <at> debbugs.gnu.org
Subject: Re: [PATCH] Add window divider faces to NS (bug#29353)
Date: Sun, 19 Nov 2017 15:06:54 -0800
Thank you, Alan, for taking a look at that potential issue.

I tried again -- this time with my nose touching the screen -- and I think the issue may be with my vision and/or the screen quality.  Perhaps this is the excuse I've been waiting for to get a higher resolution monitor.  :)

I will defer to your better vision on this.

Thanks,

Keith

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

DATE:  [11-19-2017 14:53:06] <19 Nov 2017 22:53:06 +0000>
FROM:  Alan Third <alan <at> idiocy.org>
> 
> On Sun, Nov 19, 2017 at 02:11:23PM -0800, Keith David Bershatsky wrote:
> > Alan:
> > 
> > It appears that the right divider is encroaching on the fringe to a
> > tune of 1 pixel, i.e., the vertical divider is drawing on top of the
> > left side of fringe bitmaps. Is this related to the patch, or a new
> > issue?
> 
> It looks OK here. Screenshot attached. Am I misinterpreting you, or
> are you seeing something different?
> -- 
> Alan Third
> [* fringes.png]
>




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29353; Package emacs. (Sun, 19 Nov 2017 23:24:01 GMT) Full text and rfc822 format available.

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

From: Alan Third <alan <at> idiocy.org>
To: Keith David Bershatsky <esq <at> lawlist.com>
Cc: 29353 <at> debbugs.gnu.org
Subject: Re: [PATCH] Add window divider faces to NS (bug#29353)
Date: Sun, 19 Nov 2017 23:23:01 +0000
On Sun, Nov 19, 2017 at 12:40:09PM -0800, Keith David Bershatsky wrote:
> The only issue I observed with the patch applied is when a user
> selects a divider width of 3 for right and bottom. In that
> situation, the bottom divider is entirely one color --
> window-divider face.

I can replicate this. It’s because I copied the code verbatim from
xterm.c, and it does a comparison against 3 instead of 2, so this
might be a bug in X builds too.

> And, the right divider is 2 pixels in the window-divider-first-pixel
> face and 1 pixel in the window-divider-last-pixel face. When there
> are exactly 3 pixels, both dividers should have the rainbow of all
> three available colors in the applicable order.

I can’t replicate this. I definitely see three colours in the vertical
divider.

modified   src/nsterm.m
@@ -3202,7 +3202,7 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
       [ns_lookup_indexed_color(color_last, f) set];
       NSRectFill(NSMakeRect (x1 - 1, y0, 1, y1 - y0));
     }
-  else if (x1 - x0 > y1 - y0 && y1 - y0 > 3)
+  else if (x1 - x0 > y1 - y0 && y1 - y0 > 2)
     /* Horizontal.  */
     {
       [ns_lookup_indexed_color(color_first, f) set];

-- 
Alan Third




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29353; Package emacs. (Sun, 19 Nov 2017 23:28:02 GMT) Full text and rfc822 format available.

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

From: Alan Third <alan <at> idiocy.org>
To: Keith David Bershatsky <esq <at> lawlist.com>
Cc: 29353 <at> debbugs.gnu.org
Subject: Re: bug#29353: [PATCH] Add window divider faces to NS (bug#29353)
Date: Sun, 19 Nov 2017 23:27:13 +0000
On Sun, Nov 19, 2017 at 03:06:54PM -0800, Keith David Bershatsky wrote:
> Thank you, Alan, for taking a look at that potential issue.
> 
> I tried again -- this time with my nose touching the screen -- and I
> think the issue may be with my vision and/or the screen quality.
> Perhaps this is the excuse I've been waiting for to get a higher
> resolution monitor. :)

If it’s an old monitor is it possibly some colour bleed? Try inverting
the colours.

The only other way I can think of to be sure is to take a screenshot
and zoom in past 100% in a viewer (you can use emacs if you’ve
compiled it against imagemagick!).
-- 
Alan Third




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29353; Package emacs. (Mon, 20 Nov 2017 06:32:01 GMT) Full text and rfc822 format available.

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

From: Keith David Bershatsky <esq <at> lawlist.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 29353 <at> debbugs.gnu.org
Subject: Re: bug#29353: [PATCH] Add window divider faces to NS (bug#29353)
Date: Sun, 19 Nov 2017 22:31:22 -0800
Thank you, Alan, for teaching me to use screenshots and zooming to see what was really happening.  Yes, it ended up being colors bleeding across pixel boundaries.

Everything looks great with a screenshot and an image viewer with magnification.  And, it sure is much easier than moving filing cabinets out of the way so that I can put my nose on the screen to get a better vantage point.  :)

Keith

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

DATE:  [11-19-2017 15:27:13] <19 Nov 2017 23:27:13 +0000>
FROM:  Alan Third <alan <at> idiocy.org>
> 
> On Sun, Nov 19, 2017 at 03:06:54PM -0800, Keith David Bershatsky wrote:
> > Thank you, Alan, for taking a look at that potential issue.
> > 
> > I tried again -- this time with my nose touching the screen -- and I
> > think the issue may be with my vision and/or the screen quality.
> > Perhaps this is the excuse I've been waiting for to get a higher
> > resolution monitor. :)
> 
> If it's an old monitor is it possibly some colour bleed? Try inverting
> the colours.
> 
> The only other way I can think of to be sure is to take a screenshot
> and zoom in past 100% in a viewer (you can use emacs if you've
> compiled it against imagemagick!).
> -- 
> Alan Third




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29353; Package emacs. (Mon, 20 Nov 2017 06:39:02 GMT) Full text and rfc822 format available.

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

From: Keith David Bershatsky <esq <at> lawlist.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 29353 <at> debbugs.gnu.org
Subject: Re: [PATCH] Add window divider faces to NS (bug#29353)
Date: Sun, 19 Nov 2017 22:38:25 -0800
Yes, this latest patch fixed the issue that I was observing when the width of the dividers is exactly 3.

Thank you again for bringing this feature to OSX/MacOS users.

Greatly appreciated,

Keith

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

DATE:  [11-19-2017 15:23:01] <19 Nov 2017 23:23:01 +0000>
FROM:  Alan Third <alan <at> idiocy.org>
> 
> * * *
> 
> modified   src/nsterm.m
> @@ -3202,7 +3202,7 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
>        [ns_lookup_indexed_color(color_last, f) set];
>        NSRectFill(NSMakeRect (x1 - 1, y0, 1, y1 - y0));
>      }
> -  else if (x1 - x0 > y1 - y0 && y1 - y0 > 3)
> +  else if (x1 - x0 > y1 - y0 && y1 - y0 > 2)
>      /* Horizontal.  */
>      {
>        [ns_lookup_indexed_color(color_first, f) set];
> 
> -- 
> Alan Third




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29353; Package emacs. (Mon, 20 Nov 2017 08:28:01 GMT) Full text and rfc822 format available.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Alan Third <alan <at> idiocy.org>, 
 Keith David Bershatsky <esq <at> lawlist.com>
Cc: 29353 <at> debbugs.gnu.org
Subject: Re: bug#29353: [PATCH] Add window divider faces to NS (bug#29353)
Date: Mon, 20 Nov 2017 09:26:54 +0100
> I can replicate this. It’s because I copied the code verbatim from
> xterm.c, and it does a comparison against 3 instead of 2, so this
> might be a bug in X builds too.

And in w32 builds.  Hopefully fixed now in both.

Thanks for noting this and the analysis, martin





Reply sent to Alan Third <alan <at> idiocy.org>:
You have taken responsibility. (Mon, 20 Nov 2017 20:42:01 GMT) Full text and rfc822 format available.

Notification sent to Keith David Bershatsky <esq <at> lawlist.com>:
bug acknowledged by developer. (Mon, 20 Nov 2017 20:42:01 GMT) Full text and rfc822 format available.

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

From: Alan Third <alan <at> idiocy.org>
To: Keith David Bershatsky <esq <at> lawlist.com>
Cc: 29353-done <at> debbugs.gnu.org
Subject: Re: bug#29353: [PATCH] Add window divider faces to NS (bug#29353)
Date: Mon, 20 Nov 2017 20:41:45 +0000
On Sun, Nov 19, 2017 at 10:31:22PM -0800, Keith David Bershatsky wrote:
> Thank you, Alan, for teaching me to use screenshots and zooming to
> see what was really happening. Yes, it ended up being colors
> bleeding across pixel boundaries.
> 
> Everything looks great with a screenshot and an image viewer with
> magnification. And, it sure is much easier than moving filing
> cabinets out of the way so that I can put my nose on the screen to
> get a better vantage point. :)

Thanks for checking. I’ll push my change, and I think we can mark this
as done.
-- 
Alan Third




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 19 Dec 2017 12:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 6 years and 129 days ago.

Previous Next


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