Subject: Re: bug#3233: 23.0.93; [NS] Regression in x-display-pixel-width

From: David Reitter <[email-masked]>
Date: Fri, 22 May 2009 18:57:11 -0400

On May 22, 2009, at 12:21 AM, David Reitter wrote:

> This gives me the width of the current display, not the total multi- 
> monitor width:
>
> (x-display-pixel-width
> (get-device-terminal (frame-parameter (selected-frame) 'display)))
>
> same with the more elegant version that Stefan pointed out:
>
> (x-display-pixel-width (selected-frame))
>
> I think that this is buggy (on NS), because the terminal is the same.
> ns_get_screen() indeed uses dpyinfo->x_focus_frame and ignores the  
> screen argument for purposes of calculating the width.  I don't  
> really see how the pre-2009-05-06 code would have achieved anything  
> more useful.

OP, can you try the patch below?

I think my ns_get_screen rewrite DTRT, preserving the stated intention  
of Stefan's change to handle non-NS frames somehow (we return NULL).  
Regarding this comment:

   // Not sure if this special case for nil is needed.  It does seem  
to be
   // important in xfns.c for the make-frame call in frame-initialize,
   // so let's keep it here for now.

Not sure what that is about.  I can't find frame-initialize in xfns.c.
Also, all uses of ns_get_screen appear to be in nsfns.m, and they all  
assume that
giving it Qnil results in the selected frame's display being used  
rather than mainScreen.

Also, display-usable-bounds was simply broken.  If it was tested at  
some point, they probably assumed a hidden Dock.

diff --git a/src/nsfns.m b/src/nsfns.m
index 01ffcf1..e2e187f 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -205,29 +205,31 @@ ns_get_window (Lisp_Object maybeFrame)
  static NSScreen *
  ns_get_screen (Lisp_Object screen)
  {
+  struct frame *f;
+
+  if (EQ (Qt, screen)) /* not documented */
+    return [NSScreen mainScreen];
+
    struct terminal *terminal = get_terminal (screen, 1);
    if (terminal->type != output_ns)
-    // Not sure if this special case for nil is needed.  It does seem  
to be
-    // important in xfns.c for the make-frame call in frame-initialize,
-    // so let's keep it here for now.
-    return (NILP (screen) ? [NSScreen mainScreen] : NULL);
+    return NULL;
    else
      {
-      struct ns_display_info *dpyinfo = terminal->display_info.ns;
-      struct frame *f = dpyinfo->x_focus_frame;
-      if (!f)
-	f = dpyinfo->x_highlight_frame;
-      if (!f)
-	return NULL;
+      if (NILP (screen))
+	f = SELECTED_FRAME ();
+      else if (FRAMEP (screen))
+	f = XFRAME (screen);
        else
  	{
-	  id window = nil;
-	  Lisp_Object frame;
-	  eassert (FRAME_NS_P (f));
-	  XSETFRAME (frame, f);
-	  window = ns_get_window (frame);
-	  return window ? [window screen] : NULL;
+	  struct ns_display_info *dpyinfo = terminal->display_info.ns;
+	  f = dpyinfo->x_focus_frame;
+	  if (!f)
+	    f = dpyinfo->x_highlight_frame;
  	}
+      if (!f || !FRAME_NS_P (f))
+	return NULL;
+      else
+	return [[FRAME_NS_VIEW (f) window] screen];
      }
  }

@@ -2358,15 +2367,23 @@ that stands for the selected frame's display.  
*/)
       Lisp_Object display;
  {
    int top;
+  NSScreen *screen;
+
    NSRect vScreen;

    check_ns ();
-  vScreen = [ns_get_screen (display) visibleFrame];
-  top = vScreen.origin.y == 0.0 ?
-    (int) [ns_get_screen (display) frame].size.height -  
vScreen.size.height : 0;

+  screen = ns_get_screen (display);
+  if (!screen)
+    return Qnil;
+
+  vScreen = [screen visibleFrame];
+
+  /* NS coordinate system is upside-down.
+     Transform to screen-specific coordinates. */
    return list4 (make_number ((int) vScreen.origin.x),
-                make_number (top),
+                make_number ((int) [screen frame].size.height
+			     - vScreen.size.height - vScreen.origin.y),
                  make_number ((int) vScreen.size.width),
                  make_number ((int) vScreen.size.height));
  }