[PATCH xserver] randr: rrCheckPixmapBounding should only increase screen size

[Alex Goins]

Thanks, Keith and Michel!

Yes, I'll test and review the change, and help where necessary. Thanks for
pointing it out.

-Alex

On Wed, 15 Aug 2018, Michel Dänzer wrote:

> On 2018-08-14 10:05 PM, Alex Goins wrote:
> > The purpose of rrCheckPixmapBounding() is to make sure that the fb is large
> > enough to accommodate the region scanned out by a GPU screen. Currently, however,
> > it will actually shrink the fb if it's larger than it needs to be.
> > 
> > This is a problem when combining PRIME output slaving with arbitrary transforms
> > with xrandr.
> > 
> > Although arbitrary transforms are not supposed to constrain the size of the fb
> > (https://lists.freedesktop.org/archives/xorg-devel/2018-January/055563.html),
> > xrandr will use RRSetScreenSize to resize the desktop to accommodate scaling
> > transforms, e.g. scaling a 1920x1080 display to 3840x2160 will result in a
> > desktop size of 3840x2160.
> > 
> > In the case of PRIME, rrCheckPixmapBounding() will be called after
> > RRSetScreenSize() and it will resize the fb back down to what it would be
> > without the scaling transform, e.g. 1920x1080. This represents divergence in
> > behavior between PRIME and non-PRIME outputs.
> > 
> > I had originally made rrCheckPixmapBounding() account for arbitrary transforms,
> > but realized that the fb being large enough to accommodate arbitrary transforms
> > is not a hard requirement enforced in the server. Instead, this change simply
> > makes it so that rrCheckPixmapBounding() will only resize the fb to be larger
> > than it already is, preventing it from stepping on prior requests to increase
> > the size of the fb.
> > 
> > Signed-off-by: Alex Goins <agoins at nvidia.com>
> > ---
> >  randr/rrcrtc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
> > index 74dc5a2..cdc7bed 100644
> > --- a/randr/rrcrtc.c
> > +++ b/randr/rrcrtc.c
> > @@ -702,8 +702,8 @@ rrCheckPixmapBounding(ScreenPtr pScreen,
> >      if (new_height < screen_pixmap->drawable.height)
> >          new_height = screen_pixmap->drawable.height;
> >  
> > -    if (new_width == screen_pixmap->drawable.width &&
> > -        new_height == screen_pixmap->drawable.height) {
> > +    if (new_width <= screen_pixmap->drawable.width &&
> > +        new_height <= screen_pixmap->drawable.height) {
> >      } else {
> >          pScrPriv->rrScreenSetSize(pScreen, new_width, new_height, 0, 0);
> >      }
> > 
> 
> Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>
> 
> 
> P.S. Can you review https://patchwork.freedesktop.org/patch/242720/ ,
> and maybe work with Jim to sort out any remaining issues? Some of the
> PRIME synchronization related code in the modesetting driver is relying
> on the peer screen using the modesetting driver as well.
> 
> -- 
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer
>