unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* recent change to nsterm.m: four pixels where the Dock is hidden
@ 2016-03-25  2:59 David Reitter
  2016-03-25  7:05 ` Anders Lindgren
  0 siblings, 1 reply; 4+ messages in thread
From: David Reitter @ 2016-03-25  2:59 UTC (permalink / raw)
  To: Anders Lindgren; +Cc: Emacs-Devel devel

Anders,

I appreciate your work on the NS/OSX port.
Reviewing a recent change, I can’t help but wonder:  Do we really need 50 lines of a hack to counteract design decisions made at the system level?  

If [NSScreen visibleFrame] tells us not to occupy certain space on the screen - four pixels where the Dock is hidden - then that’s a standard that all applications should adhere to.  It’s probably done for a reason (such as being able to un-hide the Dock and to grab the lower horizontal edge of the window for resizing).  

ns_screen_margins_ignoring_hidden_dock is, excuse my bluntness, ugly as it hardcodes some numbers that can change any time with a new OS version. It’s a burden for future maintenance.

If this is what #22988 really was about, then it’s not a bug and we shouldn’t mess with it.  Also not in 25.1.

If I’m wrong, please excuse me.  Could you explain if there is some deeper reasoning that I’m missing?

Thanks,
David


> Author: Anders Lindgren <andlind@gmail.com>
> Date:   Tue Mar 22 20:18:33 2016 +0100
> 
>     Make `toggle-frame-maximized' respect the dock on OS X (bug#22988).
> 
>     * src/nsterm.m (ns_screen_margins): New function.
>     (ns_screen_margins_ignoring_hidden_dock): New function.
>     (ns_menu_bar_height): Reimplement in terms of `ns_screen_margins'.
>     ([EmacsWindow zoom:]): Take all screen margins (except those
>     originating from a hidden dock) into account.
> 



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

* Re: recent change to nsterm.m: four pixels where the Dock is hidden
  2016-03-25  2:59 recent change to nsterm.m: four pixels where the Dock is hidden David Reitter
@ 2016-03-25  7:05 ` Anders Lindgren
  2016-03-25 10:56   ` David Reitter
  0 siblings, 1 reply; 4+ messages in thread
From: Anders Lindgren @ 2016-03-25  7:05 UTC (permalink / raw)
  To: David Reitter; +Cc: Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 3496 bytes --]

Hi!

The change to make `toggle-frame-maximized' cover the entire screen (when
the dock was hidden) was done much earlier, in 2015-10-23 (see link to
commit below), after a discussion that started in bug 21415 (see link). At
the time, everybody involved in the discussion thought that it was a good
idea for Emacs to cover the full height of the screen, when the dock was
hidden.

When the pretest was released, a user reported that the code didn't work
properly when the dock was visible. Effectively, the dock would cover parts
of Emacs. The commit you referred to is a fix for that problem.

When investigating how an application could check if the dock was hidden,
it turned out that there is no standard API for this, but the normal way
this is checked is to compare the result of [screen frame] (i.e. the size
of a display) with [screen visibleFrame] (i.e. the size, excluding the
parts occupied by the OS). The OS reserves four pixels at the edge where a
hidden dock resides, my code use six as a break-off value in case this is
changed in future OS versions.

I don't consider this a big change -- the fact that the change is 50 lines
is that I prefer a style where each logical step is isolated, well
documented, and there is a natural place for trace output. In this case I
decided to represent the margins using a new struct, and place the margin
calculation into two functions `ns_screen_margins' and
`ns_screen_margins_ignoring_hidden_dock'. Of course, I could easily have
written the code in a very condensed manner, inlining all calculations, but
that wouldn't have been maintainable.

This is the original change:


http://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-25&id=ba24d35a3e82cdeba4be5bd794f7f48bbfa5498e

Discussion:

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

    -- Anders Lindgren

On Fri, Mar 25, 2016 at 3:59 AM, David Reitter <david.reitter@gmail.com>
wrote:

> Anders,
>
> I appreciate your work on the NS/OSX port.
> Reviewing a recent change, I can’t help but wonder:  Do we really need 50
> lines of a hack to counteract design decisions made at the system level?
>
> If [NSScreen visibleFrame] tells us not to occupy certain space on the
> screen - four pixels where the Dock is hidden - then that’s a standard that
> all applications should adhere to.  It’s probably done for a reason (such
> as being able to un-hide the Dock and to grab the lower horizontal edge of
> the window for resizing).
>
> ns_screen_margins_ignoring_hidden_dock is, excuse my bluntness, ugly as it
> hardcodes some numbers that can change any time with a new OS version. It’s
> a burden for future maintenance.
>
> If this is what #22988 really was about, then it’s not a bug and we
> shouldn’t mess with it.  Also not in 25.1.
>
> If I’m wrong, please excuse me.  Could you explain if there is some deeper
> reasoning that I’m missing?
>
> Thanks,
> David
>
>
> > Author: Anders Lindgren <andlind@gmail.com>
> > Date:   Tue Mar 22 20:18:33 2016 +0100
> >
> >     Make `toggle-frame-maximized' respect the dock on OS X (bug#22988).
> >
> >     * src/nsterm.m (ns_screen_margins): New function.
> >     (ns_screen_margins_ignoring_hidden_dock): New function.
> >     (ns_menu_bar_height): Reimplement in terms of `ns_screen_margins'.
> >     ([EmacsWindow zoom:]): Take all screen margins (except those
> >     originating from a hidden dock) into account.
> >
>

[-- Attachment #2: Type: text/html, Size: 4356 bytes --]

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

* Re: recent change to nsterm.m: four pixels where the Dock is hidden
  2016-03-25  7:05 ` Anders Lindgren
@ 2016-03-25 10:56   ` David Reitter
  2016-03-25 21:13     ` Anders Lindgren
  0 siblings, 1 reply; 4+ messages in thread
From: David Reitter @ 2016-03-25 10:56 UTC (permalink / raw)
  To: Anders Lindgren; +Cc: Emacs-Devel devel

Anders,

Thanks for the explanation. 
The original change is difficult to read as it conflates the NSTRACE code (a lot) with the more meaningful change.  I can see that your most recent change is small.

What I find problematic is that you’re trying to micro-manage the position and size of the frame when the system provides high-level functions such as [NSScreen visibleFrame] that essentially tell you what is available.  The comment with that commit suggests that you’re going over the Dock (when it’s not hidden), and if that is indeed the case, that would be a mistake. [NSScreen visibleFrame] is documented this way:

> This is the rectangle defining the portion of the screen in which it is currently safe to draw your application content.

In the discussion, you wrote:

> Zooming and fullscreen are two different concepts. Zooming is a generic
> system, and it's up to the application to decide what to do.

Indeed, the documentation on sizing windows gives little guidance [1], however, I would argue that one shouldn’t draw into a space the system considers unsafe.

DR


[1] https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/WinPanel/Tasks/SizingPlacingWindows.html


> On Mar 25, 2016, at 3:05 AM, Anders Lindgren <andlind@gmail.com> wrote:
> 
> Hi!
> 
> The change to make `toggle-frame-maximized' cover the entire screen (when the dock was hidden) was done much earlier, in 2015-10-23 (see link to commit below), after a discussion that started in bug 21415 (see link). At the time, everybody involved in the discussion thought that it was a good idea for Emacs to cover the full height of the screen, when the dock was hidden.
> 
> When the pretest was released, a user reported that the code didn't work properly when the dock was visible. Effectively, the dock would cover parts of Emacs. The commit you referred to is a fix for that problem.
> 
> When investigating how an application could check if the dock was hidden, it turned out that there is no standard API for this, but the normal way this is checked is to compare the result of [screen frame] (i.e. the size of a display) with [screen visibleFrame] (i.e. the size, excluding the parts occupied by the OS). The OS reserves four pixels at the edge where a hidden dock resides, my code use six as a break-off value in case this is changed in future OS versions.
> 
> I don't consider this a big change -- the fact that the change is 50 lines is that I prefer a style where each logical step is isolated, well documented, and there is a natural place for trace output. In this case I decided to represent the margins using a new struct, and place the margin calculation into two functions `ns_screen_margins' and `ns_screen_margins_ignoring_hidden_dock'. Of course, I could easily have written the code in a very condensed manner, inlining all calculations, but that wouldn't have been maintainable.
> 
> This is the original change:
> 
>     http://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-25&id=ba24d35a3e82cdeba4be5bd794f7f48bbfa5498e
> 
> Discussion:
> 
>     http://debbugs.gnu.org/cgi/bugreport.cgi?bug=21415
> 
>     -- Anders Lindgren
> 
> On Fri, Mar 25, 2016 at 3:59 AM, David Reitter <david.reitter@gmail.com> wrote:
> Anders,
> 
> I appreciate your work on the NS/OSX port.
> Reviewing a recent change, I can’t help but wonder:  Do we really need 50 lines of a hack to counteract design decisions made at the system level?
> 
> If [NSScreen visibleFrame] tells us not to occupy certain space on the screen - four pixels where the Dock is hidden - then that’s a standard that all applications should adhere to.  It’s probably done for a reason (such as being able to un-hide the Dock and to grab the lower horizontal edge of the window for resizing).
> 
> ns_screen_margins_ignoring_hidden_dock is, excuse my bluntness, ugly as it hardcodes some numbers that can change any time with a new OS version. It’s a burden for future maintenance.
> 
> If this is what #22988 really was about, then it’s not a bug and we shouldn’t mess with it.  Also not in 25.1.
> 
> If I’m wrong, please excuse me.  Could you explain if there is some deeper reasoning that I’m missing?
> 
> Thanks,
> David
> 
> 
> > Author: Anders Lindgren <andlind@gmail.com>
> > Date:   Tue Mar 22 20:18:33 2016 +0100
> >
> >     Make `toggle-frame-maximized' respect the dock on OS X (bug#22988).
> >
> >     * src/nsterm.m (ns_screen_margins): New function.
> >     (ns_screen_margins_ignoring_hidden_dock): New function.
> >     (ns_menu_bar_height): Reimplement in terms of `ns_screen_margins'.
> >     ([EmacsWindow zoom:]): Take all screen margins (except those
> >     originating from a hidden dock) into account.
> >
> 




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

* Re: recent change to nsterm.m: four pixels where the Dock is hidden
  2016-03-25 10:56   ` David Reitter
@ 2016-03-25 21:13     ` Anders Lindgren
  0 siblings, 0 replies; 4+ messages in thread
From: Anders Lindgren @ 2016-03-25 21:13 UTC (permalink / raw)
  To: David Reitter; +Cc: Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 5777 bytes --]

Hi!


> The original change is difficult to read as it conflates the NSTRACE code
> (a lot) with the more meaningful change.
>

Unfortunately, I enhanced and used the trace system while rewriting the
zoom system, so it would have been very hard to do separate commits.



> What I find problematic is that you’re trying to micro-manage the position
> and size of the frame when the system provides high-level functions such as
> [NSScreen visibleFrame] that essentially tell you what is available.  The
> comment with that commit suggests that you’re going over the Dock (when
> it’s not hidden), and if that is indeed the case, that would be a mistake.


Oh, it's quite the opposite. When the dock is visible, Emacs respect
[NSScreen visibleFrame]. (Before the last commit, it didn't -- which caused
the dock to overlap the Emacs frame.)

However, when the dock is hidden, Emacs use the full height and width of
the screen.



> I would argue that one shouldn’t draw into a space the system considers
> unsafe.
>

I agree that you have a point there. I'm open to reverting this change (or
adding a variable controlling it) -- if people feel strongly about this.
Personally, it feels odd not to use the full screen, though. (Of course, a
user would still be able create a frame covering the entire screen using
`set-frame-size' et.c., or use a package like my `multicolumn'.)

Before I rewrote the zoom system, I looked around and checked how other
applications worked. Many applications respect the four pixels reserved for
the hidden dock, but far from all. One application that use a normal
window, but still use the entire screen is Lightroom from Adobe, so I
thought that we would be in good company.


Anyway, I really appreciate that you take time to review the OS X-related
changes!

Sincerely,
    Anders




> DR
>
>
> [1]
> https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/WinPanel/Tasks/SizingPlacingWindows.html
>
>
> > On Mar 25, 2016, at 3:05 AM, Anders Lindgren <andlind@gmail.com> wrote:
> >
> > Hi!
> >
> > The change to make `toggle-frame-maximized' cover the entire screen
> (when the dock was hidden) was done much earlier, in 2015-10-23 (see link
> to commit below), after a discussion that started in bug 21415 (see link).
> At the time, everybody involved in the discussion thought that it was a
> good idea for Emacs to cover the full height of the screen, when the dock
> was hidden.
> >
> > When the pretest was released, a user reported that the code didn't work
> properly when the dock was visible. Effectively, the dock would cover parts
> of Emacs. The commit you referred to is a fix for that problem.
> >
> > When investigating how an application could check if the dock was
> hidden, it turned out that there is no standard API for this, but the
> normal way this is checked is to compare the result of [screen frame] (i.e.
> the size of a display) with [screen visibleFrame] (i.e. the size, excluding
> the parts occupied by the OS). The OS reserves four pixels at the edge
> where a hidden dock resides, my code use six as a break-off value in case
> this is changed in future OS versions.
> >
> > I don't consider this a big change -- the fact that the change is 50
> lines is that I prefer a style where each logical step is isolated, well
> documented, and there is a natural place for trace output. In this case I
> decided to represent the margins using a new struct, and place the margin
> calculation into two functions `ns_screen_margins' and
> `ns_screen_margins_ignoring_hidden_dock'. Of course, I could easily have
> written the code in a very condensed manner, inlining all calculations, but
> that wouldn't have been maintainable.
> >
> > This is the original change:
> >
> >
> http://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-25&id=ba24d35a3e82cdeba4be5bd794f7f48bbfa5498e
> >
> > Discussion:
> >
> >     http://debbugs.gnu.org/cgi/bugreport.cgi?bug=21415
> >
> >     -- Anders Lindgren
> >
> > On Fri, Mar 25, 2016 at 3:59 AM, David Reitter <david.reitter@gmail.com>
> wrote:
> > Anders,
> >
> > I appreciate your work on the NS/OSX port.
> > Reviewing a recent change, I can’t help but wonder:  Do we really need
> 50 lines of a hack to counteract design decisions made at the system level?
> >
> > If [NSScreen visibleFrame] tells us not to occupy certain space on the
> screen - four pixels where the Dock is hidden - then that’s a standard that
> all applications should adhere to.  It’s probably done for a reason (such
> as being able to un-hide the Dock and to grab the lower horizontal edge of
> the window for resizing).
> >
> > ns_screen_margins_ignoring_hidden_dock is, excuse my bluntness, ugly as
> it hardcodes some numbers that can change any time with a new OS version.
> It’s a burden for future maintenance.
> >
> > If this is what #22988 really was about, then it’s not a bug and we
> shouldn’t mess with it.  Also not in 25.1.
> >
> > If I’m wrong, please excuse me.  Could you explain if there is some
> deeper reasoning that I’m missing?
> >
> > Thanks,
> > David
> >
> >
> > > Author: Anders Lindgren <andlind@gmail.com>
> > > Date:   Tue Mar 22 20:18:33 2016 +0100
> > >
> > >     Make `toggle-frame-maximized' respect the dock on OS X (bug#22988).
> > >
> > >     * src/nsterm.m (ns_screen_margins): New function.
> > >     (ns_screen_margins_ignoring_hidden_dock): New function.
> > >     (ns_menu_bar_height): Reimplement in terms of `ns_screen_margins'.
> > >     ([EmacsWindow zoom:]): Take all screen margins (except those
> > >     originating from a hidden dock) into account.
> > >
> >
>
>

[-- Attachment #2: Type: text/html, Size: 7705 bytes --]

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

end of thread, other threads:[~2016-03-25 21:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-25  2:59 recent change to nsterm.m: four pixels where the Dock is hidden David Reitter
2016-03-25  7:05 ` Anders Lindgren
2016-03-25 10:56   ` David Reitter
2016-03-25 21:13     ` Anders Lindgren

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