GNU bug report logs - #3233
23.0.93; [NS] Regression in x-display-pixel-width

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 3233 in the body.
You can then email your comments to 3233 AT debbugs.gnu.org in the normal way.

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

Message #5 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Ian Eure <ian <at> digg.com>
To: emacs-pretest-bug <at> gnu.org
Subject: 23.0.93; [NS] Regression in x-display-pixel-width
Date: Wed, 6 May 2009 11:49:39 -0700

It looks like the rewrite of (ns_get_screen) has some problems. In  
recent nightlies, the following code breaks:

(x-display-pixel-width (frame-parameter (selected-frame) 'display))

With:

Wrong type argument: terminal-live-p, "electron.local"

It worked as of 05/05, and is broken in the 05/06 build. It works if  
you don’t provide an argument, but if you give it the display  
paramater from a frame, it breaks.

In GNU Emacs 23.0.93.1 (i386-apple-darwin9.6.0, NS apple-appkit-949.43)
 of 2009-05-06 on neutron.local
Windowing system distributor `Apple', version 10.3.949
configured using `configure  '--with-ns''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: nil
  value of $XMODIFIERS: nil
  locale-coding-system: nil
  default-enable-multibyte-characters: t

Major mode: Git-Log-View

Minor modes in effect:
  diff-auto-refine-mode: t
  icomplete-mode: t
  erc-track-mode: t
  erc-track-minor-mode: t
  erc-spelling-mode: t
  erc-ring-mode: t
  erc-pcomplete-mode: t
  erc-netsplit-mode: t
  erc-match-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-autojoin-mode: t
  erc-irccontrols-mode: t
  erc-noncommands-mode: t
  erc-readonly-mode: t
  erc-scrolltobottom-mode: t
  which-function-mode: t
  yas/minor-mode: t
  shell-dirtrack-mode: t
  ime-bindings: t
  show-paren-mode: t
  recentf-mode: t
  iswitchb-mode: t
  cua-mode: t
  auto-insert-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  global-auto-composition-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  abbrev-mode: t

Recent input:
g r e p <return> x - d i s p l a u <backspace> y -
p u x e <backspace> <backspace> <backspace> i x e l
- w i d t h <return> * . c <backspace> m <return> ~
/ P O r <tab> <M-backspace> <backspace> P <backspace>
i <tab> P r <tab> e m <return> <return> C-x ` C-x v
l C-x o C-c ! g i t SPC p u l l <return> C-x b C-g
C-h f d i s p l <tab> p <tab> <return> <return> i <tab>
w <tab> <return> C-x o C-e C-b C-b C-b C-b <return>
C-n C-n C-n C-n C-n C-n M-x t o g g l e - d e b u f
<tab> <backspace> <tab> e <return> C-x o C-x b s c
r a t <backspace> <backspace> <backspace> <backspace>
<backspace> e m a c s - <return> C-e C-x C-e C-a C-f
x - C-e C-x C-e C-x o C-x b . m <return> M-< C-s t
e r m i n a l - l i <backspace> <backspace> <backspace>
_ <backspace> C-n C-p C-x o C-x b s h e l <return>
C-x o C-x C-v <return> C-s C-g C-x C-v C-g C-x C-f
n s f n <return> M-< C-s l i v e - <backspace> C-n
C-p C-s d i s p l a y _ w <backspace> C-s h C-s C-s
C-n C-p M-< C-x 1 C-x b i n f e r <backspace> <backspace>
<backspace> <backspace> <backspace> e m a c s - <return>
C-x b <return> C-x v l n d C-v <switch-frame> <switch-frame>
<switch-frame> <switch-frame> <switch-frame> <switch-frame>
C-x 4 b C-g C-n C-p C-n C-x o M-x r e p o <tab> <return>
r t - e <tab> <return>

Recent messages:
Mark set
Mark saved where search started
Quit [2 times]
Mark set
Mark saved where search started [2 times]
Mark set
Buffer is read-only: #<buffer *vc-change-log*>
Finding changes in /Users/ieure/Projects/emacs/src/nsfns.m...
Quit
Making completion list... [2 times]

Message #10 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Ian Eure <ian <at> digg.com>
Cc: 3233 <at> debbugs.gnu.org, emacs-pretest-bug <at> gnu.org
Subject: Re: bug#3233: 23.0.93; [NS] Regression in x-display-pixel-width
Date: Wed, 06 May 2009 15:57:59 -0400

> It looks like the rewrite of (ns_get_screen) has some problems. In recent
> nightlies, the following code breaks:

> (x-display-pixel-width (frame-parameter (selected-frame) 'display))

> With:
> Wrong type argument: terminal-live-p, "electron.local"
> It worked as of 05/05,

As of 05/05 it treated any string as equivalent to nil, indeed.
Not sure if that qualifies as "worked".  Can you point us to code that
is affected?


        Stefan

Message #20 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Ian Eure <ian <at> digg.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 3233 <at> debbugs.gnu.org, emacs-pretest-bug <at> gnu.org
Subject: Re: bug#3233: 23.0.93; [NS] Regression in x-display-pixel-width
Date: Wed, 6 May 2009 14:22:27 -0700

On May 6, 2009, at 12:57 PM, Stefan Monnier wrote:

>> It looks like the rewrite of (ns_get_screen) has some problems. In  
>> recent
>> nightlies, the following code breaks:
>
>> (x-display-pixel-width (frame-parameter (selected-frame) 'display))
>
>> With:
>> Wrong type argument: terminal-live-p, "electron.local"
>> It worked as of 05/05,
>
> As of 05/05 it treated any string as equivalent to nil, indeed.
> Not sure if that qualifies as "worked".  Can you point us to code that
> is affected?
>
I don't think there's anything in Emacs itself, but it broke my code  
which centers an Emacs frame:

(defun screen-usable-height (&optional display)
  "Return the usable height of the display.

Some window-systems have portions of the screen which Emacs
cannot address. This function should return the height of the
screen, minus anything which is not usable."
  (- (display-pixel-height display)
     (cond ((eq window-system 'ns) 22)
           (t 0))))

(defun screen-usable-width (&optional display)
  "Return the usable width of the display.

This works like `screen-usable-height', but for the width of the  
display."
  (display-pixel-width display))

(defun frame-box-get-center (w h cw ch)
  "Center a box inside another box.

Returns a list of `(TOP LEFT)' representing the centered position
of the box `(w h)' inside the box `(cw ch)'."
  (list (/ (- cw w) 2) (/ (- ch h) 2)))

(defun frame-get-center (frame)
  "Return the center position of FRAME on it's display."
  (let ((disp (frame-parameter frame 'display)))
    (frame-box-get-center (frame-pixel-width frame) (frame-pixel- 
height frame)
                          (screen-usable-width disp)
                          (screen-usable-height disp))))

(defun frame-center (&optional frame)
  "Center a frame on the screen."
  (interactive)
  (let ((frame (or frame (selected-frame))))
    (apply 'set-frame-position `(,frame ,@(frame-get-center frame)))))

I don't know how (display-pixel-height) handles multihead displays,  
but it seems saner to explicitly pass the display the frame occupies.

 - Ian

Message #30 received at 3233 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Ian Eure <ian <at> digg.com>
Cc: 3233 <at> debbugs.gnu.org
Subject: Re: bug#3233: 23.0.93; [NS] Regression in x-display-pixel-width
Date: Wed, 06 May 2009 21:34:13 -0400

>   (let ((disp (frame-parameter frame 'display)))

Why not use `frame' directly?  The `display' frame parameter is
fundamentally specific to X11 (although we kinds of try to make Emacs
pretend that it also exists under other GUIs).

> I don't know how (display-pixel-height) handles multihead displays, but it
> seems saner to explicitly pass the display the frame occupies.

The "display" you pass is just some string.  So "right" way would be to
pass the corresponding terminal (i.e. (frame-terminal frame)), but
most/all the functions that operate on terminals accept frames as well
(since terminal object were only introduced in Emacs-23 as pat of the
multi-tty code, so frames were previously the canonical way to refer to
a display).


        Stefan

Message #35 received at 3233 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Ian Eure <ian <at> digg.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 3233 <at> debbugs.gnu.org
Subject: Re: bug#3233: 23.0.93; [NS] Regression in x-display-pixel-width
Date: Wed, 6 May 2009 21:50:09 -0700

On May 6, 2009, at 6:34 PM, Stefan Monnier wrote:

>>  (let ((disp (frame-parameter frame 'display)))
>
> Why not use `frame' directly?  The `display' frame parameter is
> fundamentally specific to X11 (although we kinds of try to make Emacs
> pretend that it also exists under other GUIs).
>
Because I didn't know it was possible, because the docstrings refer to  
a `display' argument without elaborating on what that means.


>> I don't know how (display-pixel-height) handles multihead displays,  
>> but it
>> seems saner to explicitly pass the display the frame occupies.
>
> The "display" you pass is just some string.  So "right" way would be  
> to
> pass the corresponding terminal (i.e. (frame-terminal frame)), but
> most/all the functions that operate on terminals accept frames as well
> (since terminal object were only introduced in Emacs-23 as pat of the
> multi-tty code, so frames were previously the canonical way to refer  
> to
> a display).
Looks like it may be moot, since the info docs say:

"For graphical terminals, note that on "multi-monitor" setups this  
refers to the pixel width for all physical monitors associated with  
DISPLAY."

So it may not be possible to handle in a sane manner, i.e. centering  
on the display the frame occupies.

 - Ian

Message #42 received at 3233 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: David Reitter <reitter <at> cmu.edu>
To: 3233 <at> debbugs.gnu.org
Subject: Re: bug#3233: 23.0.93; [NS] Regression in x-display-pixel-width
Date: Fri, 22 May 2009 00:21:51 -0400

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. 

Message #47 received at 3233 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: David Reitter <david.reitter <at> gmail.com>
To: 3233 <at> debbugs.gnu.org
Subject: Re: bug#3233: 23.0.93; [NS] Regression in x-display-pixel-width
Date: Fri, 22 May 2009 18:57:11 -0400

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

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));
 }

[smime.p7s (application/pkcs7-signature, attachment)]

Message #54 received at 3233 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Adrian Robert <adrian.b.robert <at> gmail.com>
To: 3233 <at> debbugs.gnu.org
Cc: Ian Eure <ian <at> digg.com>, David Reitter <david.reitter <at> gmail.com>
Subject: Re: 23.0.93; [NS] Regression in x-display-pixel-width
Date: Thu, 23 Jul 2009 12:42:22 -0400

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=3233

Hi,

Ian, David posted a patch at the end of the bug report referenced  
above -- did you see it or get a chance to try it?  If not could you  
check it now?

David, did this patch receive any other testing?  Do you think it's  
safe to commit, either to branch or trunk?


thanks,
Adrian

Message #59 received at 3233 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Jimmy Yuen Ho Wong <wyuenho <at> gmail.com>
To: 3233 <at> debbugs.gnu.org
Subject: Re: 23.0.93; [NS] Regression in x-display-pixel-width
Date: Mon, 23 Nov 2009 11:49:29 -0500

This bug is giving me a similar problem in which `get_ns_screen` returns 
NULL when Emacs is loading and its window and menu bar are out of focus. 
This subsequently broke maxframe.el, which I use to maximize my Emacs 
frame on load.

I've applied David's patch and it works fine now, 
`display-usable-bounds` works too.

Jimmy Wong

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

From: Chong Yidong <cyd <at> stupidchicken.com>
To: Jimmy Yuen Ho Wong <wyuenho <at> gmail.com>
Cc: 3233 <at> debbugs.gnu.org
Subject: Re: 23.0.93; [NS] Regression in x-display-pixel-width
Date: Wed, 06 Jan 2010 09:45:39 -0700

> This bug is giving me a similar problem in which `get_ns_screen`
> returns NULL when Emacs is loading and its window and menu bar are out
> of focus.  This subsequently broke maxframe.el, which I use to
> maximize my Emacs frame on load.

> I've applied David's patch and it works fine now,
> `display-usable-bounds` works too.

Thanks, I've applied the patch to the repository.

Previous Next

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