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

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

[Ian Eure]

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]

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

[Stefan Monnier]

> 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

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

[Ian Eure]

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

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

[Stefan Monnier]

>   (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

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

[Ian Eure]

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

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

[Adrian Robert]

http://emacsbugs.donarmstrong.com/cgi-bin/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

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

[Jimmy Yuen Ho Wong]

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

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

[Chong Yidong]

> 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.