Re: [PATCH xserver 1/2] randr: rrCheckPixmapBounding: Do not substract crtc non 0 x, y from screen size

[Hans de Goede]

Hi,

On 22-11-16 20:41, Dave Airlie wrote:

On 23 November 2016 at 00:28, Hans de Goede  wrote:

The purpose of rrCheckPixmapBounding is to make sure that the
screen_pixmap is large enough for the slave-output which crtc is
being configured.

This should include crtc->x and crtc->y, otherwise the crtc might
still end up scanning out an area outside of the screen-pixmap.

For example: Take a laptop with an external monitor on a slave-output at
1920x1080+0+0 and its internal-screen at 3840x2160+1920+0 and in
gnome-settings-daemon move the external monitor to be on the ri ght of
the internal screen rather then on the left. First g-s-d will do a
RRSetScreenSize to 5760*2160 (which is a nop), then it calls RRSetCrtc
to move the slave output to 1920x1080+3840+0, since this is a slave
output, rrCheckPixmapBounding gets called, since the 2 crtcs now overlap
the code before this commit would shrinks the screen_pixmap to 3180*2160.
Then g-s-d calls RRSetCrtc to move the internal screen to 3180*2160+0+0.

And we end up with the slave-output configured to scan-out an area
which completely falls outside of the screen-pixmap (and end up with
a black display on the external monitor).

This commit fixes this by not substracting the x1 and y1 coordinates
of the union-ed region when determining the new screen_pixmap size.

Cc: Nikhil Mahale 
Cc: Dave Airlie 
Signed-off-by: Hans de Goede 


Thanks Hans, these seems to make sense to me.

Reviewed-by: Dave Airlie 


Thank you, "these" means the reviewed-by is for the series, right ?

Ajax, Keith, the exact push policy for the server is a bit unclear to
me now, I've seen several people push their own patches after a favorable
review. So can I just push these 2 with Dave's reviewed-by, or ... ?

Regards,

Hans







---
 randr/rrcrtc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
index 5d404e8..ac853ea 100644
--- a/randr/rrcrtc.c
+++ b/randr/rrcrtc.c
@@ -686,8 +686,8 @@ rrCheckPixmapBounding(ScreenPtr pScreen,
 }

 newsize = RegionExtents(_region);
-new_width = newsize->x2 - newsize->x1;
-new_height = newsize->y2 - newsize->y1;
+new_width = newsize->x2;
+new_height = newsize->y2;

 if (new_width == screen_pixmap->drawable.width &&
 new_height == screen_pixmap->drawable.height) {
--
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 1/2] randr: rrCheckPixmapBounding: Do not substract crtc non 0 x, y from screen size

[Hans de Goede]

The purpose of rrCheckPixmapBounding is to make sure that the
screen_pixmap is large enough for the slave-output which crtc is
being configured.

This should include crtc->x and crtc->y, otherwise the crtc might
still end up scanning out an area outside of the screen-pixmap.

For example: Take a laptop with an external monitor on a slave-output at
1920x1080+0+0 and its internal-screen at 3840x2160+1920+0 and in
gnome-settings-daemon move the external monitor to be on the ri ght of
the internal screen rather then on the left. First g-s-d will do a
RRSetScreenSize to 5760*2160 (which is a nop), then it calls RRSetCrtc
to move the slave output to 1920x1080+3840+0, since this is a slave
output, rrCheckPixmapBounding gets called, since the 2 crtcs now overlap
the code before this commit would shrinks the screen_pixmap to 3180*2160.
Then g-s-d calls RRSetCrtc to move the internal screen to 3180*2160+0+0.

And we end up with the slave-output configured to scan-out an area
which completely falls outside of the screen-pixmap (and end up with
a black display on the external monitor).

This commit fixes this by not substracting the x1 and y1 coordinates
of the union-ed region when determining the new screen_pixmap size.

Cc: Nikhil Mahale 
Cc: Dave Airlie 
Signed-off-by: Hans de Goede 
---
 randr/rrcrtc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
index 5d404e8..ac853ea 100644
--- a/randr/rrcrtc.c
+++ b/randr/rrcrtc.c
@@ -686,8 +686,8 @@ rrCheckPixmapBounding(ScreenPtr pScreen,
 }
 
 newsize = RegionExtents(_region);
-new_width = newsize->x2 - newsize->x1;
-new_height = newsize->y2 - newsize->y1;
+new_width = newsize->x2;
+new_height = newsize->y2;
 
 if (new_width == screen_pixmap->drawable.width &&
 new_height == screen_pixmap->drawable.height) {
-- 
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel