bug#22549: 25.0.50; Tooltips placed outside of screen limits

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[Óscar Fuentes]

emacs -Q
M-x toggle-frame-fullscreen

Place the mouse pointer on some element of the mode line and wait for
the tooltip. The tooltip frame is partially invisible because its top
left coordinates are determined from the mouse pointer position and, as
it is positioned near the bottom of the screen, there is no room left
for the tooltip.

A tooltip may also be partially hidden because it has no enough
horizontal space when the mouse pointer is near the right edge of the
screen.

The MS Windows port places the tooltips taking into account the screen
edges.

In GNU Emacs 25.0.50.29 (x86_64-unknown-linux-gnu, X toolkit)
 of 2016-01-21 built on qcore
Repository revision: 5293d1bdb3e665a565af032a163004c976328cc2
Windowing system distributor 'The X.Org Foundation', version 11.0.11702000
System Description:	Ubuntu 15.10

Configured using:
 'configure --without-toolkit-scroll-bars --with-x-toolkit=lucid'

Configured features:
XAW3D XPM JPEG TIFF GIF PNG SOUND GSETTINGS NOTIFY GNUTLS LIBXML2
FREETYPE XFT ZLIB LUCID X11

Important settings:
  value of $LANG: C
  locale-coding-system: nil

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[martin rudalics]

 > emacs -Q
 > M-x toggle-frame-fullscreen
 >
 > Place the mouse pointer on some element of the mode line and wait for
 > the tooltip. The tooltip frame is partially invisible because its top
 > left coordinates are determined from the mouse pointer position and, as
 > it is positioned near the bottom of the screen, there is no room left
 > for the tooltip.
 >
 > A tooltip may also be partially hidden because it has no enough
 > horizontal space when the mouse pointer is near the right edge of the
 > screen.

I cannot reproduce this.  In any case the calculations for the x- and
y-position of the tooltip take into account the size of the tooltip and
the width and height of the display as can be easily seen from the code
of compute_tip_xy.  If you see any truncation of a tooltip at a certain
position, please step with GDB through the respective part of the code
to find out what happened.

 > The MS Windows port places the tooltips taking into account the screen
 > edges.

So does the Lucid port.  Something else must be at work here.

martin

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[Óscar Fuentes]

martin rudalics <rudalics@gmx.at> writes:

>> A tooltip may also be partially hidden because it has no enough
>> horizontal space when the mouse pointer is near the right edge of the
>> screen.
>
> I cannot reproduce this.  In any case the calculations for the x- and
> y-position of the tooltip take into account the size of the tooltip and
> the width and height of the display as can be easily seen from the code
> of compute_tip_xy.  If you see any truncation of a tooltip at a certain
> position, please step with GDB through the respective part of the code
> to find out what happened.
>
>> The MS Windows port places the tooltips taking into account the screen
>> edges.
>
> So does the Lucid port.  Something else must be at work here.

I think I know what is happening.

I have two monitors, side by side, simulating a continuous display. The
monitor on the left side, where Emacs is displayed, has a vertical
resolution of 1050 pixels while the monitor on the right side has a
vertical resolution of 1200 points. Emacs is using the vertical
resolution of the virtual display (the largest of the two monitors, i.e.
1200 points) instead of the resolution of the monitor that hosts the
Emacs frame.

Something similar happens for the horizontal overflow: the dimensions of
the virtual desktop are used instead of the dimensions of the monitor.

I've seen similar issues on the past for other applications, and they
were fixed, eventually. The key here is that, while the two monitors
belong to a single desktop, it can't be considered a continuous surface.
For instance: the window manager never gives to a window a default
position that makes it overlap over the two monitors, unless the window
is so big that doesn't fit on one monitor. Ditto for maximizing: windows
maximize on one monitor, not on the whole virtual desktop (Emacs does
this right, so it already knows how to query the resolution of the
current monitor.)

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[Eli Zaretskii]

> From: Óscar Fuentes <ofv@wanadoo.es>
> Date: Thu, 04 Feb 2016 17:20:47 +0100
> Cc: 22549@debbugs.gnu.org
> 
> >> The MS Windows port places the tooltips taking into account the screen
> >> edges.
> >
> > So does the Lucid port.  Something else must be at work here.
> 
> I think I know what is happening.
> 
> I have two monitors, side by side, simulating a continuous display. The
> monitor on the left side, where Emacs is displayed, has a vertical
> resolution of 1050 pixels while the monitor on the right side has a
> vertical resolution of 1200 points. Emacs is using the vertical
> resolution of the virtual display (the largest of the two monitors, i.e.
> 1200 points) instead of the resolution of the monitor that hosts the
> Emacs frame.
> 
> Something similar happens for the horizontal overflow: the dimensions of
> the virtual desktop are used instead of the dimensions of the monitor.

I arrived at the same conclusion by looking at the X code: the APIs
called by x_display_pixel_height and x_display_pixel_width return
information about the entire virtual screen, not about the current
monitor.  We need to use different APIs in the case of tooltip
positioning.

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[martin rudalics]

 > I think I know what is happening.
 >
 > I have two monitors, side by side, simulating a continuous display.

I expected something like that.

 > The
 > monitor on the left side, where Emacs is displayed, has a vertical
 > resolution of 1050 pixels while the monitor on the right side has a
 > vertical resolution of 1200 points. Emacs is using the vertical
 > resolution of the virtual display (the largest of the two monitors, i.e.
 > 1200 points) instead of the resolution of the monitor that hosts the
 > Emacs frame.

Did you check via ‘x-display-pixel-width’ and ‘x-display-pixel-height’
and the selected frame on either of your monitors?

And what does ‘x-display-monitor-attributes-list’ return?

 > Something similar happens for the horizontal overflow: the dimensions of
 > the virtual desktop are used instead of the dimensions of the monitor.
 >
 > I've seen similar issues on the past for other applications, and they
 > were fixed, eventually. The key here is that, while the two monitors
 > belong to a single desktop, it can't be considered a continuous surface.
 > For instance: the window manager never gives to a window a default
 > position that makes it overlap over the two monitors, unless the window
 > is so big that doesn't fit on one monitor. Ditto for maximizing: windows
 > maximize on one monitor, not on the whole virtual desktop (Emacs does
 > this right, so it already knows how to query the resolution of the
 > current monitor.)

It's the window manager that maximizes the frame and tells Emacs the
required coordinates so no querying should occur here.

martin

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[Óscar Fuentes]

martin rudalics <rudalics@gmx.at> writes:

> Did you check via ‘x-display-pixel-width’ and ‘x-display-pixel-height’
> and the selected frame on either of your monitors?
>
> And what does ‘x-display-monitor-attributes-list’ return?

On the monitor with 1680x1050 resolution:

(x-display-pixel-width) 3280

(x-display-pixel-height) 1200

(x-display-monitor-attributes-list)

(((name . "DVI-0")
  (geometry 0 0 1680 1050)
  (workarea 0 0 1680 1050)
  (mm-size 474 296)
  (frames #<frame Emacs: *scratch* **  @ qcore 0x3251868>)
  (source . "XRandr"))
 ((name . "VGA-0")
  (geometry 1680 0 1600 1200)
  (workarea 1680 0 1600 1200)
  (mm-size 350 262)
  (frames)
  (source . "XRandr")))


On the monitor with 1600x1200 resolution:

(x-display-pixel-width) 3280

(x-display-pixel-height) 1200

(x-display-monitor-attributes-list)

(((name . "DVI-0")
  (geometry 0 0 1680 1050) 
  (workarea 0 0 1680 1050)
  (mm-size 474 296)
  (frames)
  (source . "XRandr"))
 ((name . "VGA-0")
  (geometry 1680 0 1600 1200)
  (workarea 1680 0 1600 1200)
  (mm-size 350 262)
  (frames #<frame Emacs: *scratch* **  @ qcore 0x1214c80>)
  (source . "XRandr")))

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[martin rudalics]

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

Please try the attached patch.  Glenn gave a recipe how to emulate
multiple monitors with a single one but I'm a bit reluctant to test it.
My display resolution setup is somewhat fragile.

Thanks, martin

[-- Attachment #2: xfns.diff --]
[-- Type: text/plain, Size: 3273 bytes --]

--- a/src/xfns.c
+++ b/src/xfns.c
@@ -5683,6 +5683,7 @@ compute_tip_xy (struct frame *f, Lisp_Object parms, Lisp_Object dx, Lisp_Object
   int win_x, win_y;
   Window root, child;
   unsigned pmask;
+  int min_x, min_y, max_x, max_y;

   /* User-specified position?  */
   left = Fcdr (Fassq (Qleft, parms));
@@ -5695,45 +5696,75 @@ compute_tip_xy (struct frame *f, Lisp_Object parms, Lisp_Object dx, Lisp_Object
   if ((!INTEGERP (left) && !INTEGERP (right))
       || (!INTEGERP (top) && !INTEGERP (bottom)))
     {
+      Lisp_Object frame, attributes, monitor, frames, geometry;
+
+      /* Default min and max values.  */
+      min_x = 0;
+      min_y = 0;
+      max_x = x_display_pixel_width (FRAME_DISPLAY_INFO (f));
+      max_y = x_display_pixel_height (FRAME_DISPLAY_INFO (f));
+
       block_input ();
       XQueryPointer (FRAME_X_DISPLAY (f), FRAME_DISPLAY_INFO (f)->root_window,
 		     &root, &child, root_x, root_y, &win_x, &win_y, &pmask);
       unblock_input ();
+
+      XSETFRAME (frame, f);
+      attributes = Fx_display_monitor_attributes_list (frame);
+
+      while (CONSP (attributes))
+	{
+	  monitor = XCAR (attributes);
+	  frames = Fassq (Qframes, monitor);
+	  if (CONSP (frames) && EQ (frame, XCAR (XCDR (frames))))
+	    {
+	      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));
+		}
+
+	      break;
+	    }
+
+	  attributes = XCDR (attributes);
+	}
     }

   if (INTEGERP (top))
     *root_y = XINT (top);
   else if (INTEGERP (bottom))
     *root_y = XINT (bottom) - height;
-  else if (*root_y + XINT (dy) <= 0)
-    *root_y = 0; /* Can happen for negative dy */
-  else if (*root_y + XINT (dy) + height
-	   <= x_display_pixel_height (FRAME_DISPLAY_INFO (f)))
+  else if (*root_y + XINT (dy) <= min_y)
+    *root_y = min_y; /* Can happen for negative dy */
+  else if (*root_y + XINT (dy) + height <= max_y)
     /* It fits below the pointer */
     *root_y += XINT (dy);
-  else if (height + XINT (dy) <= *root_y)
+  else if (height + XINT (dy) + min_y <= *root_y)
     /* It fits above the pointer.  */
     *root_y -= height + XINT (dy);
   else
     /* Put it on the top.  */
-    *root_y = 0;
+    *root_y = min_y;

   if (INTEGERP (left))
     *root_x = XINT (left);
   else if (INTEGERP (right))
     *root_x = XINT (right) - width;
-  else if (*root_x + XINT (dx) <= 0)
+  else if (*root_x + XINT (dx) <= min_x)
     *root_x = 0; /* Can happen for negative dx */
-  else if (*root_x + XINT (dx) + width
-	   <= x_display_pixel_width (FRAME_DISPLAY_INFO (f)))
+  else if (*root_x + XINT (dx) + width <= max_x)
     /* It fits to the right of the pointer.  */
     *root_x += XINT (dx);
-  else if (width + XINT (dx) <= *root_x)
+  else if (width + XINT (dx) + min_x <= *root_x)
     /* It fits to the left of the pointer.  */
     *root_x -= width + XINT (dx);
   else
-    /* Put it left-justified on the screen--it ought to fit that way.  */
-    *root_x = 0;
+    /* Put it left justified on the screen -- it ought to fit that way.  */
+    *root_x = min_x;
 }

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[Óscar Fuentes]

martin rudalics <rudalics@gmx.at> writes:

> Please try the attached patch.  Glenn gave a recipe how to emulate
> multiple monitors with a single one but I'm a bit reluctant to test it.
> My display resolution setup is somewhat fragile.

The first problem with the patch is that it crashes when there is not an
Emacs frame on some of the monitors. This is solved changing the `if'
to:

          if (CONSP (frames) && !NILP (XCDR (frames))
              && EQ (frame, XCAR (XCDR (frames))))
	    {


The next problem is that the condition

EQ (frame, XCAR (XCDR (frames)))

is always false, which makes sense because `frame' is the tooltip frame
(unless I'm misunderstanding what XSETFRAME does) and it is not listed
on `frames'.

For going ahead I "corrected" this with

      struct frame *sf = XFRAME (selected_frame);
      XSETFRAME (frame, sf);

which is wrong when there is more than one frame, because the tooltip
may be shown for a frame that is not the selected one (showing tooltips
for other than the selected frame seems inappropriate to me, but that's
how Emacs behaves right now).

Then, as the comparision only takes into account the first frame listed
on `frames', even if `frame' where correct (the frame that triggered the
tooltip) it could fail when there is more than one Emacs frame on the
same monitor.

I observed that the tooltip is correctly placed when the right
conditions (see below) are satisfied.

What is missing is to determine the parent frame of the tooltip frame,
instead of using just the selected one as I did, and to search for that
frame on the list of frames associated to the current monitor, instead
of just testing the first frame on the list.

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[Óscar Fuentes]

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

We can use the coordinates to determine the monitor. The patch below
works.


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

diff --git a/src/xfns.c b/src/xfns.c
index 9624ac5..a0182cd 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -5683,6 +5683,7 @@ compute_tip_xy (struct frame *f, Lisp_Object parms, Lisp_Object dx, Lisp_Object
   int win_x, win_y;
   Window root, child;
   unsigned pmask;
+  int min_x, min_y, max_x, max_y;
 
   /* User-specified position?  */
   left = Fcdr (Fassq (Qleft, parms));
@@ -5695,45 +5696,74 @@ compute_tip_xy (struct frame *f, Lisp_Object parms, Lisp_Object dx, Lisp_Object
   if ((!INTEGERP (left) && !INTEGERP (right))
       || (!INTEGERP (top) && !INTEGERP (bottom)))
     {
+      Lisp_Object frame, attributes, monitor, geometry;
+
+      /* Default min and max values.  */
+      min_x = 0;
+      min_y = 0;
+      max_x = x_display_pixel_width (FRAME_DISPLAY_INFO (f));
+      max_y = x_display_pixel_height (FRAME_DISPLAY_INFO (f));
+
       block_input ();
       XQueryPointer (FRAME_X_DISPLAY (f), FRAME_DISPLAY_INFO (f)->root_window,
 		     &root, &child, root_x, root_y, &win_x, &win_y, &pmask);
       unblock_input ();
+
+      XSETFRAME(frame, f);
+      attributes = Fx_display_monitor_attributes_list (frame);
+
+      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 <= *root_x && *root_x < max_x
+                  && min_y <= *root_y && *root_y < max_y)
+                {
+                  break;
+                }
+            }
+
+          attributes = XCDR (attributes);
+	}
     }
 
   if (INTEGERP (top))
     *root_y = XINT (top);
   else if (INTEGERP (bottom))
     *root_y = XINT (bottom) - height;
-  else if (*root_y + XINT (dy) <= 0)
-    *root_y = 0; /* Can happen for negative dy */
-  else if (*root_y + XINT (dy) + height
-	   <= x_display_pixel_height (FRAME_DISPLAY_INFO (f)))
+  else if (*root_y + XINT (dy) <= min_y)
+    *root_y = min_y; /* Can happen for negative dy */
+  else if (*root_y + XINT (dy) + height <= max_y)
     /* It fits below the pointer */
     *root_y += XINT (dy);
-  else if (height + XINT (dy) <= *root_y)
+  else if (height + XINT (dy) + min_y <= *root_y)
     /* It fits above the pointer.  */
     *root_y -= height + XINT (dy);
   else
     /* Put it on the top.  */
-    *root_y = 0;
+    *root_y = min_y;
 
   if (INTEGERP (left))
     *root_x = XINT (left);
   else if (INTEGERP (right))
     *root_x = XINT (right) - width;
-  else if (*root_x + XINT (dx) <= 0)
+  else if (*root_x + XINT (dx) <= min_x)
     *root_x = 0; /* Can happen for negative dx */
-  else if (*root_x + XINT (dx) + width
-	   <= x_display_pixel_width (FRAME_DISPLAY_INFO (f)))
+  else if (*root_x + XINT (dx) + width <= max_x)
     /* It fits to the right of the pointer.  */
     *root_x += XINT (dx);
-  else if (width + XINT (dx) <= *root_x)
+  else if (width + XINT (dx) + min_x <= *root_x)
     /* It fits to the left of the pointer.  */
     *root_x -= width + XINT (dx);
   else
-    /* Put it left-justified on the screen--it ought to fit that way.  */
-    *root_x = 0;
+    /* Put it left justified on the screen -- it ought to fit that way.  */
+    *root_x = min_x;
 }
 
 

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[martin rudalics]

 > The first problem with the patch is that it crashes when there is not an
 > Emacs frame on some of the monitors. This is solved changing the `if'
 > to:
 >
 >            if (CONSP (frames) && !NILP (XCDR (frames))
 >                && EQ (frame, XCAR (XCDR (frames))))

Silly me.  I was looking for Fcadr, didn't find it and just grabbed
whatever there was without thinking of the consequences.

Translating code back from Elisp to C is a PITA.  And I don't want to
tear apart ‘x-display-monitor-attributes-list’ for the present purpose,
especially not on Emacs-25.  Maybe I can convince Yamamoto Mitsuharu to
provide a C-friendly interface for master.

 > The next problem is that the condition
 >
 > EQ (frame, XCAR (XCDR (frames)))
 >
 > is always false, which makes sense because `frame' is the tooltip frame
 > (unless I'm misunderstanding what XSETFRAME does) and it is not listed
 > on `frames'.

Another silliness.  With GTK, FRAME is the original frame and I stepped
through this with GDB on a GTK build only and it obviously worked.  With
Lucid, FRAME is the tooltip frame and I just looked at the results which
were OK since I have only one monitor.

 > For going ahead I "corrected" this with
 >
 >        struct frame *sf = XFRAME (selected_frame);
 >        XSETFRAME (frame, sf);
 >
 > which is wrong when there is more than one frame, because the tooltip
 > may be shown for a frame that is not the selected one (showing tooltips
 > for other than the selected frame seems inappropriate to me, but that's
 > how Emacs behaves right now).

Maybe someone will find a use case for it.  Let's not bother now but
simply agree that the selected frame might be on another monitor and so
could be the wrong frame.

 > Then, as the comparision only takes into account the first frame listed
 > on `frames', even if `frame' where correct (the frame that triggered the
 > tooltip) it could fail when there is more than one Emacs frame on the
 > same monitor.

I don't understand you here.  There can be any number of frames on the
"right" monitor.  The important thing is to get the monitor of the frame
where the tooltip shall be shown.

 > I observed that the tooltip is correctly placed when the right
 > conditions (see below) are satisfied.
 >
 > What is missing is to determine the parent frame of the tooltip frame,

... that's the one passed via FRAME to ‘x-show-tip’ and we just have to
pass it on to compute_tip_xy ...

 > instead of using just the selected one as I did, and to search for that
 > frame on the list of frames associated to the current monitor, instead
 > of just testing the first frame on the list.

martin

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[martin rudalics]

 > We can use the coordinates to determine the monitor. The patch below
 > works.

Good idea.  But do that only if ‘x-display-monitor-attributes-list’
returns more than one monitor so we do not needlessly loop in (and
possibly compromise) the one monitor case.  Or (tediously) assign

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

only if the subsequent test

+              if (min_x <= *root_x && *root_x < max_x
+                  && min_y <= *root_y && *root_y < max_y)

would succeed.

There's no guarantee that ‘x-display-monitor-attributes-list’ works on
all systems, especially on those where neither Xinerama nor XRandr are
installed.  IIRC older versions of the latter might also have problems
when a monitor is added/removed during a session.  The fallback should
always give the same results as the present code, but who knows ...

And just to make sure: Can we rely on the fact that XQueryPointer always
gives reliable results with multiple monitors and its present arguments?

martin

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[Óscar Fuentes]

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

martin rudalics <rudalics@gmx.at> writes:

>> We can use the coordinates to determine the monitor. The patch below
>> works.
>
> Good idea.  But do that only if ‘x-display-monitor-attributes-list’
> returns more than one monitor so we do not needlessly loop in (and
> possibly compromise) the one monitor case.

I don't think that performance considerations are applicable here. About
compromising the one monitor case: in theory, the monitor's geometry is
always the right thing to use for calculating the tooltip position, not
x_display_pixel_width/height. For instance: it is possible to configure
X for having a desktop area larger than the monitor's resolution, IIRC.

> Or (tediously) assign
>
> +              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));
>
> only if the subsequent test
>
> +              if (min_x <= *root_x && *root_x < max_x
> +                  && min_y <= *root_y && *root_y < max_y)
>
> would succeed.

> There's no guarantee that ‘x-display-monitor-attributes-list’ works on
> all systems, especially on those where neither Xinerama nor XRandr are
> installed.  IIRC older versions of the latter might also have problems
> when a monitor is added/removed during a session.  The fallback should
> always give the same results as the present code, but who knows ...

See the attached patch. If we can't use the monitor's geometry, we
default to the old code.

> And just to make sure: Can we rely on the fact that XQueryPointer always
> gives reliable results with multiple monitors and its present arguments?

Dunno. I know nothing about X Windows.


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

diff --git a/src/xfns.c b/src/xfns.c
index 9624ac5..20ac627 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -5683,6 +5683,7 @@ compute_tip_xy (struct frame *f, Lisp_Object parms, Lisp_Object dx, Lisp_Object
   int win_x, win_y;
   Window root, child;
   unsigned pmask;
+  int min_x, min_y, max_x, max_y = -1;
 
   /* User-specified position?  */
   left = Fcdr (Fassq (Qleft, parms));
@@ -5695,45 +5696,81 @@ compute_tip_xy (struct frame *f, Lisp_Object parms, Lisp_Object dx, Lisp_Object
   if ((!INTEGERP (left) && !INTEGERP (right))
       || (!INTEGERP (top) && !INTEGERP (bottom)))
     {
+      Lisp_Object frame, attributes, monitor, geometry;
+
       block_input ();
       XQueryPointer (FRAME_X_DISPLAY (f), FRAME_DISPLAY_INFO (f)->root_window,
 		     &root, &child, root_x, root_y, &win_x, &win_y, &pmask);
       unblock_input ();
+
+      XSETFRAME(frame, f);
+      attributes = Fx_display_monitor_attributes_list (frame);
+
+      /* Try to determine the monitor where the mouse pointer is and
+         its geometry.  See bug#22549.  */
+      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 <= *root_x && *root_x < max_x
+                  && min_y <= *root_y && *root_y < max_y)
+                {
+                  break;
+                }
+              max_y = -1;
+            }
+
+          attributes = XCDR (attributes);
+	}
+    }
+
+  /* It was not possible to determine the monitor's geometry, so we
+     assign some sane defaults here: */
+  if ( max_y < 0 )
+    {
+      min_x = 0;
+      min_y = 0;
+      max_x = x_display_pixel_width (FRAME_DISPLAY_INFO (f));
+      max_y = x_display_pixel_height (FRAME_DISPLAY_INFO (f));
     }
 
   if (INTEGERP (top))
     *root_y = XINT (top);
   else if (INTEGERP (bottom))
     *root_y = XINT (bottom) - height;
-  else if (*root_y + XINT (dy) <= 0)
-    *root_y = 0; /* Can happen for negative dy */
-  else if (*root_y + XINT (dy) + height
-	   <= x_display_pixel_height (FRAME_DISPLAY_INFO (f)))
+  else if (*root_y + XINT (dy) <= min_y)
+    *root_y = min_y; /* Can happen for negative dy */
+  else if (*root_y + XINT (dy) + height <= max_y)
     /* It fits below the pointer */
     *root_y += XINT (dy);
-  else if (height + XINT (dy) <= *root_y)
+  else if (height + XINT (dy) + min_y <= *root_y)
     /* It fits above the pointer.  */
     *root_y -= height + XINT (dy);
   else
     /* Put it on the top.  */
-    *root_y = 0;
+    *root_y = min_y;
 
   if (INTEGERP (left))
     *root_x = XINT (left);
   else if (INTEGERP (right))
     *root_x = XINT (right) - width;
-  else if (*root_x + XINT (dx) <= 0)
+  else if (*root_x + XINT (dx) <= min_x)
     *root_x = 0; /* Can happen for negative dx */
-  else if (*root_x + XINT (dx) + width
-	   <= x_display_pixel_width (FRAME_DISPLAY_INFO (f)))
+  else if (*root_x + XINT (dx) + width <= max_x)
     /* It fits to the right of the pointer.  */
     *root_x += XINT (dx);
-  else if (width + XINT (dx) <= *root_x)
+  else if (width + XINT (dx) + min_x <= *root_x)
     /* It fits to the left of the pointer.  */
     *root_x -= width + XINT (dx);
   else
-    /* Put it left-justified on the screen--it ought to fit that way.  */
-    *root_x = 0;
+    /* Put it left justified on the screen -- it ought to fit that way.  */
+    *root_x = min_x;
 }
 
 

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[martin rudalics]

 > I don't think that performance considerations are applicable here. About
 > compromising the one monitor case: in theory, the monitor's geometry is
 > always the right thing to use for calculating the tooltip position, not
 > x_display_pixel_width/height. For instance: it is possible to configure
 > X for having a desktop area larger than the monitor's resolution, IIRC.

But ‘x_display_pixel_height’ returns the value of 'HeightOfScreen' which
is what we need for the one monitor case.

 >> There's no guarantee that ‘x-display-monitor-attributes-list’ works on
 >> all systems, especially on those where neither Xinerama nor XRandr are
 >> installed.  IIRC older versions of the latter might also have problems
 >> when a monitor is added/removed during a session.  The fallback should
 >> always give the same results as the present code, but who knows ...
 >
 > See the attached patch. If we can't use the monitor's geometry, we
 > default to the old code.

My concern was less that ‘x-display-monitor-attributes-list’ would not
assign a value to max_y.  My concern was that that value could be wrong.

Anyway.  Better install your initial patch then.  It's cleaner and we'll
see soon enough whether it causes any problems.

 >> And just to make sure: Can we rely on the fact that XQueryPointer always
 >> gives reliable results with multiple monitors and its present arguments?
 >
 > Dunno. I know nothing about X Windows.

Have you tried with a frame that spans both monitors?

martin

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[Óscar Fuentes]

martin rudalics <rudalics@gmx.at> writes:

> But ‘x_display_pixel_height’ returns the value of 'HeightOfScreen' which
> is what we need for the one monitor case.

Ok.

> My concern was less that ‘x-display-monitor-attributes-list’ would not
> assign a value to max_y.  My concern was that that value could be wrong.
>
> Anyway.  Better install your initial patch then.  It's cleaner and we'll
> see soon enough whether it causes any problems.

My initial patch is not better because it doesn't protect against the
possible lack of information on x-display-monitor-attributes-list you
mentioned above and on your previous e-mail. If that structure has not
the correct geometry info about the current monitor, the variables
max/min_x/y are guaranteed to be wrong. My last patch provides some
protection against that circunstance, so I'll feel better committing it.

(BTW, the path is basically yours, not mine. I just corrected some
issues but the 90% was written by you, so I'll feel a bit uneasy taking
credit for it.)

>>> And just to make sure: Can we rely on the fact that XQueryPointer always
>>> gives reliable results with multiple monitors and its present arguments?
>>
>> Dunno. I know nothing about X Windows.
>
> Have you tried with a frame that spans both monitors?

Just tried. It seems to work fine: for a frame that spawns both
monitors, the tooltip is correctly positioned taking into account the
coordinates of the mouse cursor and the geometry of the corresponding
monitor.

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[martin rudalics]

 > My initial patch is not better because it doesn't protect against the
 > possible lack of information on x-display-monitor-attributes-list you
 > mentioned above and on your previous e-mail. If that structure has not
 > the correct geometry info about the current monitor, the variables
 > max/min_x/y are guaranteed to be wrong. My last patch provides some
 > protection against that circunstance, so I'll feel better committing it.

I'd still prefer something like

if (CONSP (attributes))
   while (CONSP (attributes))
     ... your new code ...
else
   ... the old code ...

but I leave it to you.  I feel uneasy talking about changes I can't
really test here.

 > (BTW, the path is basically yours, not mine. I just corrected some
 > issues but the 90% was written by you, so I'll feel a bit uneasy taking
 > credit for it.)

80% of my code was stolen from the person who wrote the Windows code.  I
suppose it was Andy Moreton but I can't find the corresponding ChangeLog
entry now.  So don't bother ;-)

 >> Have you tried with a frame that spans both monitors?
 >
 > Just tried. It seems to work fine: for a frame that spawns both
 > monitors, the tooltip is correctly positioned taking into account the
 > coordinates of the mouse cursor and the geometry of the corresponding
 > monitor.

I suppose you tested with underlying text very near the dividing screen
edge.  When you drag the frame with the mouse from left to right and
back does the tooltip "jump" accordingly from one monitor to the other?

martin

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[martin rudalics]

 > I suppose you tested with underlying text very near the dividing screen
 > edge.  When you drag the frame with the mouse from left to right and
 > back does the tooltip "jump" accordingly from one monitor to the other?

No that's silly because the mouse must be on the frame border for
dragging.  So you have to release the mouse first and move it to the
appropriate text.

martin

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[Óscar Fuentes]

martin rudalics <rudalics@gmx.at> writes:

>> I suppose you tested with underlying text very near the dividing screen
>> edge.  When you drag the frame with the mouse from left to right and
>> back does the tooltip "jump" accordingly from one monitor to the other?
>
> No that's silly because the mouse must be on the frame border for
> dragging.  So you have to release the mouse first and move it to the
> appropriate text.

Furthermore, the tooltip frame isn't supposed to move once it is
displayed, AFAIK.

I'll commit the patch soon, but the bug will remain open until I check
the MS Windows case. The test on Windows I mentioned at the beginning of
this issue was not valid, because it used only monitor. It is frequent
to connect MS Windows laptops to an external monitor and use both
displays, effectively duplicating the setup I have on GNU/Linux: two
monitors with different resolutions.

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[Óscar Fuentes]

Committed on c77ffc8019bceb850a794c13f2e3ad991cc7e412

The MS Windows code already has code that manages the multi-monitor
case. I tested that it works as expected.

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[martin rudalics]

 > Committed on c77ffc8019bceb850a794c13f2e3ad991cc7e412
 >
 > The MS Windows code already has code that manages the multi-monitor
 > case. I tested that it works as expected.

Obviously so.  50% of the code you just committed _is_ straight from the
MS Windows code you just tested ;-)

martin

bug#22549: 25.0.50; Tooltips placed outside of screen limits

[Óscar Fuentes]

martin rudalics <rudalics@gmx.at> writes:

>> The MS Windows code already has code that manages the multi-monitor
>> case. I tested that it works as expected.
>
> Obviously so.  50% of the code you just committed _is_ straight from the
> MS Windows code you just tested ;-)

Oh, I was a bit puzzled by your mention of the "stolen" code, now I know
from where it came :-)

Thanks for your help.