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

[Alex Goins]

Hi Adam,

Can this be merged?

Thanks,
Alex

On Wed, 15 Aug 2018, Alex Goins wrote:

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