bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Alex]

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


The menus that display when clicking on various items in the mode line
(major mode, minor mode, buffer position) don't display correctly with
certain monitor setups on X, specifically when using multiple monitors
where the bottoms of the monitors are not even. They appear squished and
are effectively unusable.

FWIW, a similar issue appeared with the mode line tooltips that
displayed when hovering over these items in Emacs 24.5, but they have
since been fixed.

The menu bar menus do not suffer this problem.

When running from the command line, the following message usually
appears:

*** BUG ***
In pixman_region32_init_rect: Invalid rectangle passed
Set a breakpoint on '_pixman_log_error' to debug

I tried to debug this in gdb, but setting the breakpoint made X hang
when encountering this issue, so I was unable to procure a backtrace.

I have attached a picture showing the problem. The static displayed is
the area that I am unable to see, but is included by scrot. The squished
prompt was from clicking on `Lisp Interaction' in the mode line.

[-- Attachment #2: Squished popup example --]
[-- Type: image/png, Size: 5652323 bytes --]

[-- Attachment #3: Type: text/plain, Size: 2716 bytes --]



In GNU Emacs 25.0.94.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.18.9)
 of 2016-05-17 built on lylat
Windowing system distributor 'Fedora Project', version 11.0.11803000
Configured using:
 'configure --with-gif=no'

Configured features:
XPM JPEG TIFF PNG SOUND DBUS GSETTINGS NOTIFY FREETYPE XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LC_CTYPE: en_CA.utf8
  value of $LANG: en_CA.utf8
  value of $XMODIFIERS: @im=none
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
delete-backward-char: Text is read-only [2 times]
Making completion list...
Quit
Making completion list... [2 times]
Quit [4 times]
Type C-x 1 to delete the help window.

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils thingatpt
help-fns help-mode easymenu cl-loaddefs pcase cl-lib time-date mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment elisp-mode lisp-mode prog-mode register
page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core frame cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese charscript case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote dbusbind inotify dynamic-setting
system-font-setting font-render-setting move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 87251 10741)
 (symbols 48 19665 0)
 (miscs 40 56 172)
 (strings 32 14472 3942)
 (string-bytes 1 414309)
 (vectors 16 11790)
 (vector-slots 8 429244 4485)
 (floats 8 166 98)
 (intervals 56 294 13)
 (buffers 976 13)
 (heap 1024 22194 1031))

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Alex]

I have managed to obtain a backtrace:

#0  0x00007ffff035e540 in _pixman_log_error () at /lib64/libpixman-1.so.0
#1  0x00007ffff035b606 in pixman_region32_init_rect () at /lib64/libpixman-1.so.0
#2  0x00007ffff5d8770e in cairo_region_create_rectangle () at /lib64/libcairo.so.2
#3  0x00007ffff69129f9 in recompute_visible_regions_internal () at /lib64/libgdk-3.so.0
#4  0x00007ffff691956e in gdk_window_move_resize_internal () at /lib64/libgdk-3.so.0
#5  0x00007ffff6dc0017 in gtk_menu_scroll_to () at /lib64/libgtk-3.so.0
#6  0x00007ffff6dc58ca in gtk_menu_size_allocate () at /lib64/libgtk-3.so.0
#7  0x00007ffff6efb585 in gtk_widget_size_allocate_with_baseline () at /lib64/libgtk-3.so.0
#8  0x00007ffff6f10bb3 in gtk_window_size_allocate () at /lib64/libgtk-3.so.0
#9  0x00007ffff553a7a5 in g_closure_invoke () at /lib64/libgobject-2.0.so.0
#10 0x00007ffff554c38c in signal_emit_unlocked_R () at /lib64/libgobject-2.0.so.0
#11 0x00007ffff5555530 in g_signal_emit_valist () at /lib64/libgobject-2.0.so.0
#12 0x00007ffff55558ff in g_signal_emit () at /lib64/libgobject-2.0.so.0
#13 0x00007ffff6efb7c7 in gtk_widget_size_allocate_with_baseline () at /lib64/libgtk-3.so.0
#14 0x00007ffff6f0b14f in gtk_window_check_resize () at /lib64/libgtk-3.so.0
#15 0x00007ffff553a9d4 in _g_closure_invoke_va () at /lib64/libgobject-2.0.so.0
#16 0x00007ffff55552bd in g_signal_emit_valist () at /lib64/libgobject-2.0.so.0
#17 0x00007ffff55558ff in g_signal_emit () at /lib64/libgobject-2.0.so.0
#18 0x00007ffff6cfa47c in gtk_container_idle_sizer () at /lib64/libgtk-3.so.0
#19 0x00007ffff553a9d4 in _g_closure_invoke_va () at /lib64/libgobject-2.0.so.0
#20 0x00007ffff55552bd in g_signal_emit_valist () at /lib64/libgobject-2.0.so.0
#21 0x00007ffff5555dc5 in g_signal_emit_by_name () at /lib64/libgobject-2.0.so.0
#22 0x00007ffff6909118 in gdk_frame_clock_paint_idle () at /lib64/libgdk-3.so.0
#23 0x00007ffff68f85a8 in gdk_threads_dispatch () at /lib64/libgdk-3.so.0
#24 0x00007ffff523c893 in g_timeout_dispatch () at /lib64/libglib-2.0.so.0
#25 0x00007ffff523be3a in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
#26 0x00007ffff523c1d0 in g_main_context_iterate.isra () at /lib64/libglib-2.0.so.0
#27 0x00007ffff523c27c in g_main_context_iteration () at /lib64/libglib-2.0.so.0
#28 0x00007ffff6db23c5 in gtk_main_iteration () at /lib64/libgtk-3.so.0
#29 0x0000000000466864 in x_menu_show (do_timers=true, widget=<optimized out>) at xmenu.c:394
#30 0x0000000000466864 in x_menu_show (for_click=true, y=1565, x=247, first_wv=<optimized out>, f=<optimized out>) at xmenu.c:1265
#31 0x0000000000466864 in x_menu_show (f=0x11eb710, x=247, y=928, menuflags=3, title=<optimized out>, error_name=<optimized out>) at xmenu.c:1588
#32 0x0000000000464052 in Fx_popup_menu (position=position@entry=20194707, menu=16113619) at menu.c:1433
#33 0x00000000004efb9b in read_char (used_mouse_menu=0x7fffffffe0db, prev_event=20194707, map=16113619) at keyboard.c:8361
#34 0x00000000004efb9b in read_char (commandflag=commandflag@entry=1, map=map@entry=16113619, prev_event=20194707, used_mouse_menu=used_mouse_menu@entry=0x7fffffffe0db, end_time=end_time@entry=0x0) at keyboard.c:2665
#35 0x00000000004f0c5a in read_key_sequence (keybuf=keybuf@entry=0x7fffffffe1b0, prompt=prompt@entry=0, dont_downcase_last=dont_downcase_last@entry=false, can_return_switch_frame=can_return_switch_frame@entry=true, fix_current_buffer=fix_current_buffer@entry=true, prevent_redisplay=prevent_redisplay@entry=false, bufsize=30) at keyboard.c:9055
#36 0x00000000004f2826 in command_loop_1 () at keyboard.c:1357
#37 0x0000000000553cc2 in internal_condition_case (bfun=bfun@entry=0x4f2630 <command_loop_1>, handlers=handlers@entry=19056, hfun=hfun@entry=0x4e91b0 <cmd_error>) at eval.c:1309
#38 0x00000000004e48ac in command_loop_2 (ignore=ignore@entry=0) at keyboard.c:1099
#39 0x0000000000553c63 in internal_catch (tag=tag@entry=45840, func=func@entry=0x4e4890 <command_loop_2>, arg=arg@entry=0) at eval.c:1074
#40 0x00000000004e4869 in command_loop () at keyboard.c:1078
#41 0x00000000004e8d9b in recursive_edit_1 () at keyboard.c:684
#42 0x00000000004e90e8 in Frecursive_edit () at keyboard.c:755
#43 0x000000000041397b in main (argc=1, argv=0x7fffffffe508) at emacs.c:1606

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Alex]

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

Alex <agrambot@gmail.com> writes:

> The menus that display when clicking on various items in the mode line
> (major mode, minor mode, buffer position) don't display correctly with
> certain monitor setups on X, specifically when using multiple monitors
> where the bottoms of the monitors are not even. They appear squished and
> are effectively unusable.
>
> FWIW, a similar issue appeared with the mode line tooltips that
> displayed when hovering over these items in Emacs 24.5, but they have
> since been fixed.
>
> The menu bar menus do not suffer this problem.
>
> When running from the command line, the following message usually
> appears:
>
> *** BUG ***
> In pixman_region32_init_rect: Invalid rectangle passed
> Set a breakpoint on '_pixman_log_error' to debug
>
> I tried to debug this in gdb, but setting the breakpoint made X hang
> when encountering this issue, so I was unable to procure a backtrace.
>
> I have attached a picture showing the problem. The static displayed is
> the area that I am unable to see, but is included by scrot. The squished
> prompt was from clicking on `Lisp Interaction' in the mode line.

This is still an issue in current master.

The tooltip issue I mentioned above last year was fixed in 7b14da444, so
I tried using pretty much the same code to fix this issue; a proof of
concept diff is included below.

I don't know if it's a good approach though. Partially because I have no
idea why the values of req.{height, width} are so large, why they differ
in different Emacs sessions, and how they relate to the other
width/height variables.

In any case, perhaps this functionality should be abstracted into a
procedure that just computes the dimensions of the current monitor?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: popup-diff --]
[-- Type: text/x-diff, Size: 1647 bytes --]

diff --git a/src/xmenu.c b/src/xmenu.c
index 2805249164..9a87fce380 100644
--- a/src/xmenu.c
+++ b/src/xmenu.c
@@ -1161,8 +1161,43 @@ menu_position_func (GtkMenu *menu, gint *x, gint *y, gboolean *push_in, gpointer
   struct next_popup_x_y *data = user_data;
   GtkRequisition req;
   struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (data->f);
-  int disp_width = x_display_pixel_width (dpyinfo);
-  int disp_height = x_display_pixel_height (dpyinfo);
+  int disp_width;
+  int disp_height;
+
+  int min_x, min_y, max_x, max_y = -1;
+  Lisp_Object frame, attributes, monitor, geometry;
+
+  XSETFRAME(frame, data->f);
+  attributes = Fx_display_monitor_attributes_list (frame);
+
+  /* Try to determine the current monitor's resolution */
+  while (CONSP (attributes))
+    {
+      monitor = XCAR (attributes);
+      geometry = Fassq (Qgeometry, monitor);
+      if (CONSP (geometry))
+        {
+          min_x = XINT (Fnth (make_number (1), geometry));
+          min_y = XINT (Fnth (make_number (2), geometry));
+          max_x = min_x + XINT (Fnth (make_number (3), geometry));
+          max_y = min_y + XINT (Fnth (make_number (4), geometry));
+          if (min_x <= data->x && data->x < max_x
+              && min_y <= data->y && data->y < max_y)
+            {
+              disp_width = max_x;
+              disp_height = max_y;
+              break;
+            }
+          max_y = -1;
+        }
+
+      attributes = XCDR (attributes);
+    }
+  if ( max_y < 0 )
+    {
+      disp_width = x_display_pixel_width (dpyinfo);
+      disp_height = x_display_pixel_height (dpyinfo);
+    }
 
   *x = data->x;
   *y = data->y;

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Alex]

Alex <agrambot@gmail.com> writes:

> This is still an issue in current master.
>
> The tooltip issue I mentioned above last year was fixed in 7b14da444, so
> I tried using pretty much the same code to fix this issue; a proof of
> concept diff is included below.
>
> I don't know if it's a good approach though. Partially because I have no
> idea why the values of req.{height, width} are so large, why they differ
> in different Emacs sessions, and how they relate to the other
> width/height variables.
>
> In any case, perhaps this functionality should be abstracted into a
> procedure that just computes the dimensions of the current monitor?
>
> diff --git a/src/xmenu.c b/src/xmenu.c
> index 2805249164..9a87fce380 100644
> --- a/src/xmenu.c
> +++ b/src/xmenu.c
> @@ -1161,8 +1161,43 @@ menu_position_func (GtkMenu *menu, gint *x, gint *y, gboolean *push_in, gpointer
>    struct next_popup_x_y *data = user_data;
>    GtkRequisition req;
>    struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (data->f);
> -  int disp_width = x_display_pixel_width (dpyinfo);
> -  int disp_height = x_display_pixel_height (dpyinfo);
> +  int disp_width;
> +  int disp_height;
> +
> +  int min_x, min_y, max_x, max_y = -1;
> +  Lisp_Object frame, attributes, monitor, geometry;
> +
> +  XSETFRAME(frame, data->f);
> +  attributes = Fx_display_monitor_attributes_list (frame);
> +
> +  /* Try to determine the current monitor's resolution */
> +  while (CONSP (attributes))
> +    {
> +      monitor = XCAR (attributes);
> +      geometry = Fassq (Qgeometry, monitor);
> +      if (CONSP (geometry))
> +        {
> +          min_x = XINT (Fnth (make_number (1), geometry));
> +          min_y = XINT (Fnth (make_number (2), geometry));
> +          max_x = min_x + XINT (Fnth (make_number (3), geometry));
> +          max_y = min_y + XINT (Fnth (make_number (4), geometry));
> +          if (min_x <= data->x && data->x < max_x
> +              && min_y <= data->y && data->y < max_y)
> +            {
> +              disp_width = max_x;
> +              disp_height = max_y;
> +              break;
> +            }
> +          max_y = -1;
> +        }
> +
> +      attributes = XCDR (attributes);
> +    }
> +  if ( max_y < 0 )
> +    {
> +      disp_width = x_display_pixel_width (dpyinfo);
> +      disp_height = x_display_pixel_height (dpyinfo);
> +    }
>  
>    *x = data->x;
>    *y = data->y;

Sorry, I copied the wrong commit hash. I meant to write that it was
fixed in c77ffc8019, which fixed bug#22549.

Martin, since you were involved with bug#22549, what do you think? Does
this seem reasonable, and should this functionality (window dimensions
of the current position's monitor) be extracted into its own procedure?

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[martin rudalics]

 > Martin, since you were involved with bug#22549, what do you think? Does
 > this seem reasonable, and should this functionality (window dimensions
 > of the current position's monitor) be extracted into its own procedure?

I think so.

But in addition I would like two functions, say display_monitor_geometry
and display_monitor_workarea (maybe in frame.c), so we can avoid the
Fx_display_monitor_attributes_list detour when we want the dimensions of
the dominating monitor for a particular frame only.

martin

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Alex]

martin rudalics <rudalics@gmx.at> writes:

>> Martin, since you were involved with bug#22549, what do you think? Does
>> this seem reasonable, and should this functionality (window dimensions
>> of the current position's monitor) be extracted into its own procedure?
>
> I think so.
>
> But in addition I would like two functions, say display_monitor_geometry
> and display_monitor_workarea (maybe in frame.c), so we can avoid the
> Fx_display_monitor_attributes_list detour when we want the dimensions of
> the dominating monitor for a particular frame only.
>
> martin

That makes sense. What do you think of the following proposal? I made a
quick Lisp implementation for easy testing. The
`display-monitor-attribute' function allows access to any arbitrary
attribute of the "current" monitor, which can either be determined by
the selected frame or an explicit (x, y) coordinate.


(defun display-monitor-geometry (&optional frame x y)
  (display-monitor-attribute 'geometry frame x y))

(defun display-monitor-workarea (&optional frame x y)
  (display-monitor-attribute 'workarea frame x y))

(defun display-monitor-attribute (attribute &optional frame x y)
  "Return the value of the attribute of the 'current' monitor.
By default, use the frame info to determine the current monitor,
but if x and y are non-nil then use the given coordinates to
determine it."
  (let ((attributes (display-monitor-attributes-list))
        (frame (or frame (selected-frame))))
    (if (and x y)
        (cl-loop for monitor in attributes do
                 (let* ((geometry (assq 'geometry monitor))
                        (min-x (nth 1 geometry))
                        (min-y (nth 2 geometry))
                        (max-x (+ min-x (nth 3 geometry)))
                        (max-y (+ min-y (nth 4 geometry))))
                   (when (and (<= min-x x max-x)
                              (<= min-y y max-y))
                     (cl-return (alist-get attribute monitor)))))
      (cl-loop for monitor in attributes do
               (when (memq frame (alist-get 'frames monitor))
                 (cl-return (alist-get attribute monitor)))))))

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[martin rudalics]

 >    "Return the value of the attribute of the 'current' monitor.
 > By default, use the frame info to determine the current monitor,
 > but if x and y are non-nil then use the given coordinates to
 > determine it."

That's the idea, yes.

martin

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Alex]

martin rudalics <rudalics@gmx.at> writes:

>>    "Return the value of the attribute of the 'current' monitor.
>> By default, use the frame info to determine the current monitor,
>> but if x and y are non-nil then use the given coordinates to
>> determine it."
>
> That's the idea, yes.

Since these procedures use display-monitor-attributes-list, which is in
frame.el, should they also be put in frame.el instead of frame.c? Or is
it preferred to put them in frame.c with a DEFUN wrapper for lisp-level
callers?

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Alex]

Alex <agrambot@gmail.com> writes:

> martin rudalics <rudalics@gmx.at> writes:
>
>>>    "Return the value of the attribute of the 'current' monitor.
>>> By default, use the frame info to determine the current monitor,
>>> but if x and y are non-nil then use the given coordinates to
>>> determine it."
>>
>> That's the idea, yes.
>
> Since these procedures use display-monitor-attributes-list, which is in
> frame.el, should they also be put in frame.el instead of frame.c? Or is
> it preferred to put them in frame.c with a DEFUN wrapper for lisp-level
> callers?

I just noticed that frame.el also includes frame-monitor-attributes,
which also makes me think that frame.el is a nice place for these
procedures.

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[martin rudalics]

 > I just noticed that frame.el also includes frame-monitor-attributes,
 > which also makes me think that frame.el is a nice place for these
 > procedures.

For the moment putting them in frame.el should be OK.  Eventually these
functions should be in frame.c and represent stripped down versions of
‘display-monitor-attributes-list’ where only the dominating monitor for
the given frame or coordinates is investigated.  Other monitors and
things like lists of frames dominated by a monitor, mm values, name and
source would not be calculated by the respective backends.  These values
are not needed for checking tip frame or menu positions and repeatedly
evaluating and returning the same values seems rather silly.

martin

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Alex]

martin rudalics <rudalics@gmx.at> writes:

>> I just noticed that frame.el also includes frame-monitor-attributes,
>> which also makes me think that frame.el is a nice place for these
>> procedures.
>
> For the moment putting them in frame.el should be OK.  Eventually these
> functions should be in frame.c and represent stripped down versions of
> ‘display-monitor-attributes-list’ where only the dominating monitor for
> the given frame or coordinates is investigated.  Other monitors and
> things like lists of frames dominated by a monitor, mm values, name and
> source would not be calculated by the respective backends.  These values
> are not needed for checking tip frame or menu positions and repeatedly
> evaluating and returning the same values seems rather silly.

I agree that you shouldn't have to calculate everything each time,
though I don't know enough about the C code to achieve that right now.

In the end, we would still leave the function interface so that users
can still call the discussed procedures from lisp code, right? I don't
want to introduce an interface just for it to be removed later on if I
can avoid it.

In any case, I altered display-monitor-attribute to use
frame-monitor-attributes and to not succeed on {x, y} = max-{x, y}:

(defun display-monitor-attribute (attribute &optional frame x y)
  "Return the value of the attribute of the 'current' monitor.
By default, use the frame info to determine the current monitor,
but if x and y are non-nil then use the given coordinates to
determine it."
  (if (and x y)
      (cl-loop for monitor in (display-monitor-attributes-list)
               for geometry = (alist-get 'geometry monitor)
               for min-x = (pop geometry)
               for min-y = (pop geometry)
               for max-x = (+ min-x (pop geometry))
               for max-y = (+ min-y (car geometry))
               when (and (<= min-x x)
                         (< x max-x)
                         (<= min-y y)
                         (< y max-y))
               return (alist-get attribute monitor))
    (alist-get attribute (frame-monitor-attributes frame))))

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[martin rudalics]

 > In the end, we would still leave the function interface so that users
 > can still call the discussed procedures from lisp code, right?

I think so, yes.

 > (defun display-monitor-attribute (attribute &optional frame x y)
 >   "Return the value of the attribute of the 'current' monitor.
 > By default, use the frame info to determine the current monitor,
 > but if x and y are non-nil then use the given coordinates to
 > determine it."
 >   (if (and x y)

Probably, FRAME should be replaced by DISPLAY as in other ‘display-...’
functions.  X and Y must be checked properly to avoid throwing an error
when they are used in the arithmetics.  And the doc-string should
explicitly state that the first argument is ignored when X and Y are
both non-nil.

martin

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Alex]

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

martin rudalics <rudalics@gmx.at> writes:

>> In the end, we would still leave the function interface so that users
>> can still call the discussed procedures from lisp code, right?
>
> I think so, yes.
>
>> (defun display-monitor-attribute (attribute &optional frame x y)
>>   "Return the value of the attribute of the 'current' monitor.
>> By default, use the frame info to determine the current monitor,
>> but if x and y are non-nil then use the given coordinates to
>> determine it."
>>   (if (and x y)
>
> Probably, FRAME should be replaced by DISPLAY as in other ‘display-...’
> functions.  X and Y must be checked properly to avoid throwing an error
> when they are used in the arithmetics.  And the doc-string should
> explicitly state that the first argument is ignored when X and Y are
> both non-nil.

I fixed up the docstrings to match display-monitor-attributes-list.

Don't you want to throw an error when x and y are used, but are not
integers?

Also, should yet another procedure be created that abstracts out adding
the min_{x,y} and width/height components of the geometry/workarea
attributes, or should it be expected for each caller to do that
manually?

The current diff is attached below:


[-- Attachment #2: multi-headv3 --]
[-- Type: text/x-diff, Size: 4575 bytes --]

diff --git a/lisp/frame.el b/lisp/frame.el
index 02871e0551..005f1f3e83 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1908,6 +1908,61 @@ display-monitor-attributes-list
 		       ,(display-mm-height display)))
 	   (frames . ,(frames-on-display-list display)))))))))
 
+(defun display-monitor-attribute (attribute &optional display x y)
+  "Return the value of the ATTRIBUTE of the current monitor.
+DISPLAY can be a display name, a terminal name, or a frame.
+If DISPLAY is omitted or nil, it defaults to the selected frame’s display.
+
+By default, use DISPLAY to determine the current monitor.  If X
+and Y are both non-nil, then ignore the value of DISPLAY and use
+X and Y as pixel coordinates to determine the current monitor.
+
+See `display-monitor-attributes-list' for information on possible
+attributes."
+  (if (and x y)
+      (cl-loop for monitor in (display-monitor-attributes-list)
+               for geometry = (alist-get 'geometry monitor)
+               for min-x = (pop geometry)
+               for min-y = (pop geometry)
+               for max-x = (+ min-x (pop geometry))
+               for max-y = (+ min-y (car geometry))
+               when (and (<= min-x x)
+                         (< x max-x)
+                         (<= min-y y)
+                         (< y max-y))
+               return (alist-get attribute monitor))
+    (alist-get attribute (frame-monitor-attributes display))))
+
+(defun display-monitor-geometry (&optional display x y)
+    "Return the geometry of the current monitor.
+DISPLAY can be a display name, a terminal name, or a frame.
+If DISPLAY is omitted or nil, it defaults to the selected frame’s display.
+
+By default, use DISPLAY to determine the current monitor.  If X
+and Y are both non-nil, then ignore the value of DISPLAY and use
+X and Y as pixel coordinates to determine the current monitor.
+
+See `display-monitor-attributes-list' for information on the
+geometry attribute."
+  (display-monitor-attribute 'geometry display x y))
+
+(defun display-monitor-workarea (&optional display x y)
+  "Return the workarea of the current monitor.
+DISPLAY can be a display name, a terminal name, or a frame.
+If DISPLAY is omitted or nil, it defaults to the selected frame’s display.
+
+By default, use DISPLAY to determine the current monitor.  If X
+and Y are both non-nil, then ignore the value of DISPLAY and use
+X and Y as pixel coordinates to determine the current monitor.
+
+The result is a list of the form (x-min y-min x-max y-max),
+representing the minimum and maximum pixel coordinates of the
+workarea.
+
+See `display-monitor-attributes-list' for information on the
+workarea attribute."
+  (display-monitor-attribute 'workarea display x y))
+
 \f
 ;;;; Frame geometry values
 
diff --git a/src/xmenu.c b/src/xmenu.c
index 2805249164..04d5bde2ba 100644
--- a/src/xmenu.c
+++ b/src/xmenu.c
@@ -1160,9 +1160,33 @@ menu_position_func (GtkMenu *menu, gint *x, gint *y, gboolean *push_in, gpointer
 {
   struct next_popup_x_y *data = user_data;
   GtkRequisition req;
-  struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (data->f);
-  int disp_width = x_display_pixel_width (dpyinfo);
-  int disp_height = x_display_pixel_height (dpyinfo);
+  int disp_width = -1;
+  int disp_height = -1;
+
+  Lisp_Object frame, geometry;
+
+  XSETFRAME (frame, data->f);
+  geometry = call3 (Qdisplay_monitor_geometry,
+                    Qnil,
+                    make_number (data->x),
+                    make_number (data->y));
+
+  if (CONSP (geometry))
+    {
+      int min_x, min_y;
+
+      min_x = XINT (XCAR (geometry));
+      disp_width = min_x + XINT (Fnth (make_number (2), geometry));
+      min_y = XINT (Fnth (make_number (1), geometry));
+      disp_height = min_y + XINT (Fnth (make_number (3), geometry));
+    }
+  if ( disp_width < 0 || disp_height < 0 )
+    {
+      struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (data->f);
+
+      disp_width = x_display_pixel_width (dpyinfo);
+      disp_height = x_display_pixel_height (dpyinfo);
+    }
 
   *x = data->x;
   *y = data->y;
@@ -2361,6 +2385,10 @@ syms_of_xmenu (void)
   DEFSYM (Qdebug_on_next_call, "debug-on-next-call");
   defsubr (&Smenu_or_popup_active_p);
 
+#ifdef USE_GTK
+  DEFSYM (Qdisplay_monitor_geometry, "display-monitor-geometry");
+#endif
+
 #if defined (USE_GTK) || defined (USE_X_TOOLKIT)
   defsubr (&Sx_menu_bar_open_internal);
   Ffset (intern_c_string ("accelerate-menu"),

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Date: Fri, 02 Jun 2017 18:54:03 -0600
> Cc: 23568@debbugs.gnu.org
> 
> I fixed up the docstrings to match display-monitor-attributes-list.

Thanks, a few comments below.

> Don't you want to throw an error when x and y are used, but are not
> integers?

If this is supposed to be used as part of mode-line display, then no.
Signaling errors in the middle of redisplay is generally a bad idea,
because they cause another redisplay cycle, which again signals an
error, and Emacs just freezes.

> +(defun display-monitor-attribute (attribute &optional display x y)
> +  "Return the value of the ATTRIBUTE of the current monitor.

The doc string should say something about what "the current monitor"
means, or have a link to where that is explained.

> +DISPLAY can be a display name, a terminal name, or a frame.

"Terminal name" or "terminal object"?

> +(defun display-monitor-geometry (&optional display x y)
> +    "Return the geometry of the current monitor.
> +DISPLAY can be a display name, a terminal name, or a frame.

Same here, on both counts.

> diff --git a/src/xmenu.c b/src/xmenu.c
> index 2805249164..04d5bde2ba 100644
> --- a/src/xmenu.c
> +++ b/src/xmenu.c

Why is this in xmenu.c?  Is the problem unique to X window system?

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Alex]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Date: Fri, 02 Jun 2017 18:54:03 -0600
>> Cc: 23568@debbugs.gnu.org

>> Don't you want to throw an error when x and y are used, but are not
>> integers?
>
> If this is supposed to be used as part of mode-line display, then no.
> Signaling errors in the middle of redisplay is generally a bad idea,
> because they cause another redisplay cycle, which again signals an
> error, and Emacs just freezes.

I see. In the general case, is it recommended to silently fallback to a
different method, or should there be a log message?

I changed the procedure so that it only uses the coordinates if they're
numbers.

>> +(defun display-monitor-attribute (attribute &optional display x y)
>> +  "Return the value of the ATTRIBUTE of the current monitor.
>
> The doc string should say something about what "the current monitor"
> means, or have a link to where that is explained.

I expanded that section, copying a chunk of it from
`frame-monitor-attributes'.

>> +DISPLAY can be a display name, a terminal name, or a frame.
>
> "Terminal name" or "terminal object"?

`display-monitor-attributes' can take both, which is where I copied that
line from.

I don't think I should have done that, actually. I don't believe this
function makes sense for a terminal/display input, as a
terminal/display, as I understand it, can have multiple physical
monitors.

I changed all instances of `display' to `frame'. I'm still not sure on
the name, though. Perhaps a better prefix would be `physical-monitor-*',
or just `monitor-*'?

What do you think?

>> diff --git a/src/xmenu.c b/src/xmenu.c
>> index 2805249164..04d5bde2ba 100644
>> --- a/src/xmenu.c
>> +++ b/src/xmenu.c
>
> Why is this in xmenu.c?  Is the problem unique to X window system?

The xmenu.c part contains the actual bugfix, while the frame.el part is
just a partial abstraction for the fix so that other procedures can use
it (such as compute_tip_xy in xfns.c).

The procedure I'm editing (menu_position_func) is inside an #ifdef
USE_GTK, and uses x_display_pixel_{height,width} currently. I'm not sure
if it's useful outside of X currently.

I haven't tested the issue on other systems, but I don't think it's an
issue in Windows as half of the code came from Windows code according to
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=22549#61.

Here's an updated diff:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v4 --]
[-- Type: text/x-diff, Size: 5214 bytes --]

diff --git a/lisp/frame.el b/lisp/frame.el
index 02871e0551..d0ac29694c 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1498,6 +1498,75 @@ frame-monitor-attributes
 	   for frames = (cdr (assq 'frames attributes))
 	   if (memq frame frames) return attributes))
 
+(defun frame-monitor-attribute (attribute &optional frame x y)
+  "Return the value of the ATTRIBUTE of the current monitor.
+If FRAME is omitted or nil, use currently selected frame.
+
+By default, the current monitor is said to be the physical
+monitor dominating FRAME.  A frame is dominated by a physical
+monitor when either the largest area of the frame resides in the
+monitor, or the monitor is the closest to the frame if the frame
+does not intersect any physical monitors.
+
+If X and Y are both numbers, then ignore the value of FRAME; the
+current monitor is determined to be the physical monitor that
+contains the pixel coordinate (X, Y).
+
+See `display-monitor-attributes-list' for the list of attribute
+keys and their meanings."
+  (if (and (numberp x)
+           (numberp y))
+      (cl-loop for monitor in (display-monitor-attributes-list)
+               for geometry = (alist-get 'geometry monitor)
+               for min-x = (pop geometry)
+               for min-y = (pop geometry)
+               for max-x = (+ min-x (pop geometry))
+               for max-y = (+ min-y (car geometry))
+               when (and (<= min-x x)
+                         (< x max-x)
+                         (<= min-y y)
+                         (< y max-y))
+               return (alist-get attribute monitor))
+    (alist-get attribute (frame-monitor-attributes frame))))
+
+(defun frame-monitor-geometry (&optional frame x y)
+    "Return the geometry of the current monitor.
+FRAME can be a frame name, a terminal name, or a frame.
+If FRAME is omitted or nil, use the currently selected frame.
+
+By default, the current monitor is said to be the physical
+monitor dominating FRAME.  A frame is dominated by a physical
+monitor when either the largest area of the frame resides in the
+monitor, or the monitor is the closest to the frame if the frame
+does not intersect any physical monitors.
+
+If X and Y are both numbers, then ignore the value of FRAME; the
+current monitor is determined to be the physical monitor that
+contains the pixel coordinate (X, Y).
+
+See `display-monitor-attributes-list' for information on the
+geometry attribute."
+  (frame-monitor-attribute 'geometry frame x y))
+
+(defun frame-monitor-workarea (&optional frame x y)
+  "Return the workarea of the current monitor.
+FRAME can be a frame name, a terminal name, or a frame.
+If FRAME is omitted or nil, use currently selected frame.
+
+By default, the current monitor is said to be the physical
+monitor dominating FRAME.  A frame is dominated by a physical
+monitor when either the largest area of the frame resides in the
+monitor, or the monitor is the closest to the frame if the frame
+does not intersect any physical monitors.
+
+If X and Y are both numbers, then ignore the value of FRAME; the
+current monitor is determined to be the physical monitor that
+contains the pixel coordinate (X, Y).
+
+See `display-monitor-attributes-list' for information on the
+workarea attribute."
+  (frame-monitor-attribute 'workarea frame x y))
+
 (declare-function x-frame-list-z-order "xfns.c" (&optional display))
 (declare-function w32-frame-list-z-order "w32fns.c" (&optional display))
 (declare-function ns-frame-list-z-order "nsfns.m" (&optional display))
diff --git a/src/xmenu.c b/src/xmenu.c
index 2805249164..5aee85360d 100644
--- a/src/xmenu.c
+++ b/src/xmenu.c
@@ -1160,9 +1160,33 @@ menu_position_func (GtkMenu *menu, gint *x, gint *y, gboolean *push_in, gpointer
 {
   struct next_popup_x_y *data = user_data;
   GtkRequisition req;
-  struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (data->f);
-  int disp_width = x_display_pixel_width (dpyinfo);
-  int disp_height = x_display_pixel_height (dpyinfo);
+  int disp_width = -1;
+  int disp_height = -1;
+
+  Lisp_Object frame, geometry;
+
+  XSETFRAME (frame, data->f);
+  geometry = call3 (Qdisplay_monitor_geometry,
+                    Qnil,
+                    make_number (data->x),
+                    make_number (data->y));
+
+  if (CONSP (geometry))
+    {
+      int min_x, min_y;
+
+      min_x = XINT (XCAR (geometry));
+      min_y = XINT (Fnth (make_number (1), geometry));
+      disp_width = min_x + XINT (Fnth (make_number (2), geometry));
+      disp_height = min_y + XINT (Fnth (make_number (3), geometry));
+    }
+  if ( disp_width < 0 || disp_height < 0 )
+    {
+      struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (data->f);
+
+      disp_width = x_display_pixel_width (dpyinfo);
+      disp_height = x_display_pixel_height (dpyinfo);
+    }
 
   *x = data->x;
   *y = data->y;
@@ -2361,6 +2385,10 @@ syms_of_xmenu (void)
   DEFSYM (Qdebug_on_next_call, "debug-on-next-call");
   defsubr (&Smenu_or_popup_active_p);
 
+#ifdef USE_GTK
+  DEFSYM (Qdisplay_monitor_geometry, "display-monitor-geometry");
+#endif
+
 #if defined (USE_GTK) || defined (USE_X_TOOLKIT)
   defsubr (&Sx_menu_bar_open_internal);
   Ffset (intern_c_string ("accelerate-menu"),

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: rudalics@gmx.at,  23568@debbugs.gnu.org
> Date: Sat, 03 Jun 2017 13:19:50 -0600
> 
> > If this is supposed to be used as part of mode-line display, then no.
> > Signaling errors in the middle of redisplay is generally a bad idea,
> > because they cause another redisplay cycle, which again signals an
> > error, and Emacs just freezes.
> 
> I see. In the general case, is it recommended to silently fallback to a
> different method, or should there be a log message?
> 
> I changed the procedure so that it only uses the coordinates if they're
> numbers.

It depends.  For Lisp code that we write, it is indeed better to avoid
the problem altogether.  For user-defined Lisp, about which we can
make no assumptions, there should be a log message in case of errors.

> >> +(defun display-monitor-attribute (attribute &optional display x y)
> >> +  "Return the value of the ATTRIBUTE of the current monitor.
> >
> > The doc string should say something about what "the current monitor"
> > means, or have a link to where that is explained.
> 
> I expanded that section, copying a chunk of it from
> `frame-monitor-attributes'.
> 
> >> +DISPLAY can be a display name, a terminal name, or a frame.
> >
> > "Terminal name" or "terminal object"?
> 
> `display-monitor-attributes' can take both, which is where I copied that
> line from.
> 
> I don't think I should have done that, actually. I don't believe this
> function makes sense for a terminal/display input, as a
> terminal/display, as I understand it, can have multiple physical
> monitors.
> 
> I changed all instances of `display' to `frame'. I'm still not sure on
> the name, though. Perhaps a better prefix would be `physical-monitor-*',
> or just `monitor-*'?
> 
> What do you think?

LGTM, thanks.

> > Why is this in xmenu.c?  Is the problem unique to X window system?
> 
> The xmenu.c part contains the actual bugfix, while the frame.el part is
> just a partial abstraction for the fix so that other procedures can use
> it (such as compute_tip_xy in xfns.c).
> 
> The procedure I'm editing (menu_position_func) is inside an #ifdef
> USE_GTK, and uses x_display_pixel_{height,width} currently. I'm not sure
> if it's useful outside of X currently.
> 
> I haven't tested the issue on other systems, but I don't think it's an
> issue in Windows as half of the code came from Windows code according to
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=22549#61.

Ah, okay.  Then maybe just add a comment there saying that the w32
case is handled elsewhere, perhaps with a pointer to the function
which does that.

> Here's an updated diff:

Thanks, it would be nice if you could also provide a ChangeLog-style
commit log message.  That'd make the job of pushing the changes much
easier.

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Alex]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: rudalics@gmx.at,  23568@debbugs.gnu.org
>> Date: Sat, 03 Jun 2017 13:19:50 -0600

>> I changed all instances of `display' to `frame'. I'm still not sure on
>> the name, though. Perhaps a better prefix would be `physical-monitor-*',
>> or just `monitor-*'?
>> 
>> What do you think?
>
> LGTM, thanks.

Just to be clear: you prefer the frame-* prefix?

>> The xmenu.c part contains the actual bugfix, while the frame.el part is
>> just a partial abstraction for the fix so that other procedures can use
>> it (such as compute_tip_xy in xfns.c).
>> 
>> The procedure I'm editing (menu_position_func) is inside an #ifdef
>> USE_GTK, and uses x_display_pixel_{height,width} currently. I'm not sure
>> if it's useful outside of X currently.
>> 
>> I haven't tested the issue on other systems, but I don't think it's an
>> issue in Windows as half of the code came from Windows code according to
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=22549#61.
>
> Ah, okay.  Then maybe just add a comment there saying that the w32
> case is handled elsewhere, perhaps with a pointer to the function
> which does that.

Actually, it turns out that I used the half of the code that *didn't*
come from compute_tip_xy from w32fns.c. I instead put a reference to
commit c77ffc8019 in date form in the commit message.

I tested it out on a Windows partition and indeed the problem doesn't
appear there.

>> Here's an updated diff:
>
> Thanks, it would be nice if you could also provide a ChangeLog-style
> commit log message.  That'd make the job of pushing the changes much
> easier.

Right, I just figure that it's easier to save that work until the end
until most/all of the design considerations are taken care of.

Also, I switched from using the geometry attribute (like c77ffc8019
used) to the workarea attribute as according to the GDK doc[1]:

  The work area should be considered when positioning menus and similar
  popups, to avoid placing them below panels, docks or other desktop
  components.

I've attached a patch below.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: multi-monitor-patch --]
[-- Type: text/x-diff, Size: 6701 bytes --]

From 82301e82288298436a568c4968d734df8e77c4d2 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Mon, 5 Jun 2017 00:55:56 -0600
Subject: [PATCH] Fix the placement of GTK menus on multi-monitor systems.

menu_position_func did not properly use the current monitor's
resolution. Also see commit '2016-02-06 22:12:53 +0100'.

* lisp/frame.el (frame-monitor-attribute, frame-monitor-geometry)
(frame-monitor-workarea): New functions.
* src/xmenu.c (menu_position_func): Take into account the workarea of
the monitor that contains the mouse (Bug#23568).
---
 lisp/frame.el | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/xmenu.c   | 46 +++++++++++++++++++++++++++++++++------
 2 files changed, 108 insertions(+), 7 deletions(-)

diff --git a/lisp/frame.el b/lisp/frame.el
index 02871e0551..d0ac29694c 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1498,6 +1498,75 @@ frame-monitor-attributes
 	   for frames = (cdr (assq 'frames attributes))
 	   if (memq frame frames) return attributes))
 
+(defun frame-monitor-attribute (attribute &optional frame x y)
+  "Return the value of the ATTRIBUTE of the current monitor.
+If FRAME is omitted or nil, use currently selected frame.
+
+By default, the current monitor is said to be the physical
+monitor dominating FRAME.  A frame is dominated by a physical
+monitor when either the largest area of the frame resides in the
+monitor, or the monitor is the closest to the frame if the frame
+does not intersect any physical monitors.
+
+If X and Y are both numbers, then ignore the value of FRAME; the
+current monitor is determined to be the physical monitor that
+contains the pixel coordinate (X, Y).
+
+See `display-monitor-attributes-list' for the list of attribute
+keys and their meanings."
+  (if (and (numberp x)
+           (numberp y))
+      (cl-loop for monitor in (display-monitor-attributes-list)
+               for geometry = (alist-get 'geometry monitor)
+               for min-x = (pop geometry)
+               for min-y = (pop geometry)
+               for max-x = (+ min-x (pop geometry))
+               for max-y = (+ min-y (car geometry))
+               when (and (<= min-x x)
+                         (< x max-x)
+                         (<= min-y y)
+                         (< y max-y))
+               return (alist-get attribute monitor))
+    (alist-get attribute (frame-monitor-attributes frame))))
+
+(defun frame-monitor-geometry (&optional frame x y)
+    "Return the geometry of the current monitor.
+FRAME can be a frame name, a terminal name, or a frame.
+If FRAME is omitted or nil, use the currently selected frame.
+
+By default, the current monitor is said to be the physical
+monitor dominating FRAME.  A frame is dominated by a physical
+monitor when either the largest area of the frame resides in the
+monitor, or the monitor is the closest to the frame if the frame
+does not intersect any physical monitors.
+
+If X and Y are both numbers, then ignore the value of FRAME; the
+current monitor is determined to be the physical monitor that
+contains the pixel coordinate (X, Y).
+
+See `display-monitor-attributes-list' for information on the
+geometry attribute."
+  (frame-monitor-attribute 'geometry frame x y))
+
+(defun frame-monitor-workarea (&optional frame x y)
+  "Return the workarea of the current monitor.
+FRAME can be a frame name, a terminal name, or a frame.
+If FRAME is omitted or nil, use currently selected frame.
+
+By default, the current monitor is said to be the physical
+monitor dominating FRAME.  A frame is dominated by a physical
+monitor when either the largest area of the frame resides in the
+monitor, or the monitor is the closest to the frame if the frame
+does not intersect any physical monitors.
+
+If X and Y are both numbers, then ignore the value of FRAME; the
+current monitor is determined to be the physical monitor that
+contains the pixel coordinate (X, Y).
+
+See `display-monitor-attributes-list' for information on the
+workarea attribute."
+  (frame-monitor-attribute 'workarea frame x y))
+
 (declare-function x-frame-list-z-order "xfns.c" (&optional display))
 (declare-function w32-frame-list-z-order "w32fns.c" (&optional display))
 (declare-function ns-frame-list-z-order "nsfns.m" (&optional display))
diff --git a/src/xmenu.c b/src/xmenu.c
index 2805249164..6c8a0c506c 100644
--- a/src/xmenu.c
+++ b/src/xmenu.c
@@ -1160,9 +1160,37 @@ menu_position_func (GtkMenu *menu, gint *x, gint *y, gboolean *push_in, gpointer
 {
   struct next_popup_x_y *data = user_data;
   GtkRequisition req;
-  struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (data->f);
-  int disp_width = x_display_pixel_width (dpyinfo);
-  int disp_height = x_display_pixel_height (dpyinfo);
+  int max_x = -1;
+  int max_y = -1;
+
+  Lisp_Object frame, workarea;
+
+  XSETFRAME (frame, data->f);
+
+  /* TODO: Get the monitor workarea directly without calculating other
+     items in x-display-monitor-attributes-list. */
+  workarea = call3 (Qframe_monitor_workarea,
+                    Qnil,
+                    make_number (data->x),
+                    make_number (data->y));
+
+  if (CONSP (workarea))
+    {
+      int min_x, min_y;
+
+      min_x = XINT (XCAR (workarea));
+      min_y = XINT (Fnth (make_number (1), workarea));
+      max_x = min_x + XINT (Fnth (make_number (2), workarea));
+      max_y = min_y + XINT (Fnth (make_number (3), workarea));
+    }
+
+  if (max_x < 0 || max_y < 0)
+    {
+      struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (data->f);
+
+      max_x = x_display_pixel_width (dpyinfo);
+      max_y = x_display_pixel_height (dpyinfo);
+    }
 
   *x = data->x;
   *y = data->y;
@@ -1170,10 +1198,10 @@ menu_position_func (GtkMenu *menu, gint *x, gint *y, gboolean *push_in, gpointer
   /* Check if there is room for the menu.  If not, adjust x/y so that
      the menu is fully visible.  */
   gtk_widget_get_preferred_size (GTK_WIDGET (menu), NULL, &req);
-  if (data->x + req.width > disp_width)
-    *x -= data->x + req.width - disp_width;
-  if (data->y + req.height > disp_height)
-    *y -= data->y + req.height - disp_height;
+  if (data->x + req.width > max_x)
+    *x -= data->x + req.width - max_x;
+  if (data->y + req.height > max_y)
+    *y -= data->y + req.height - max_y;
 }
 
 static void
@@ -2361,6 +2389,10 @@ syms_of_xmenu (void)
   DEFSYM (Qdebug_on_next_call, "debug-on-next-call");
   defsubr (&Smenu_or_popup_active_p);
 
+#ifdef USE_GTK
+  DEFSYM (Qframe_monitor_workarea, "frame-monitor-workarea");
+#endif
+
 #if defined (USE_GTK) || defined (USE_X_TOOLKIT)
   defsubr (&Sx_menu_bar_open_internal);
   Ffset (intern_c_string ("accelerate-menu"),
-- 
2.11.0


[-- Attachment #3: Type: text/plain, Size: 106 bytes --]


Footnotes: 
[1]  https://developer.gnome.org/gdk3/stable/GdkScreen.html#gdk-screen-get-monitor-workarea

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: rudalics@gmx.at,  23568@debbugs.gnu.org
> Date: Mon, 05 Jun 2017 02:03:45 -0600
> 
> Just to be clear: you prefer the frame-* prefix?

I'm okay with that.  Martin?

> > Thanks, it would be nice if you could also provide a ChangeLog-style
> > commit log message.  That'd make the job of pushing the changes much
> > easier.
> 
> Right, I just figure that it's easier to save that work until the end
> until most/all of the design considerations are taken care of.
> 
> Also, I switched from using the geometry attribute (like c77ffc8019
> used) to the workarea attribute as according to the GDK doc[1]:
> 
>   The work area should be considered when positioning menus and similar
>   popups, to avoid placing them below panels, docks or other desktop
>   components.
> 
> I've attached a patch below.

Thanks.  Let's leave it for a few days to give others a chance to
comment, and then push.

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[martin rudalics]

 >> Just to be clear: you prefer the frame-* prefix?
 >
 > I'm okay with that.  Martin?

No objections.

martin

bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: rudalics@gmx.at,  23568@debbugs.gnu.org
> Date: Mon, 05 Jun 2017 02:03:45 -0600
> 
> I've attached a patch below.

Thanks, pushed to master.