[PATCH xserver 2/2] randr: Use RRCrtcGetScanoutSize() in ProcRRSetScreenSize()

[Alex Goins]

On Tue, 9 Jan 2018, Michel Dänzer wrote:

> On 2018-01-09 03:44 AM, Alex Goins wrote:
> > Previously, ProcRRSetScreenSize() manually computed the dimensions of a CRTC's
> > viewport in order to check that it does not extend beyond the bounds of the new
> > screen size. It did this incorrectly, leading to bugs.
> > 
> > A previous patch "randr: Fix rotation check in ProcRRSetScreenSize()" fixed the
> > issue by directly correcting the calculation, but to avoid future bugs of this
> > class, this patch uses RRCrtcGetScanoutSize() to do the calculation, implicitly
> > accounting for all possible transforms.
> > 
> > There is existing precedent for this method in ProcRRSetCrtcConfig(), where
> > RRModeGetScanoutSize() is used directly due to the transform having not yet been
> > applied but the check is otherwise the same.
> > 
> > Signed-off-by: Alex Goins <agoins at nvidia.com>
> > ---
> >  randr/rrscreen.c | 13 ++++---------
> >  1 file changed, 4 insertions(+), 9 deletions(-)
> > 
> > diff --git a/randr/rrscreen.c b/randr/rrscreen.c
> > index f484383..7321eef 100644
> > --- a/randr/rrscreen.c
> > +++ b/randr/rrscreen.c
> > @@ -265,17 +265,12 @@ ProcRRSetScreenSize(ClientPtr client)
> >      }
> >      for (i = 0; i < pScrPriv->numCrtcs; i++) {
> >          RRCrtcPtr crtc = pScrPriv->crtcs[i];
> > -        RRModePtr mode = crtc->mode;
> >  
> > -        if (mode) {
> > -            int source_width = mode->mode.width;
> > -            int source_height = mode->mode.height;
> > -            Rotation rotation = crtc->rotation;
> > +        if (crtc->mode) {
> > +            int source_width;
> > +            int source_height;
> >  
> > -            if (rotation & (RR_Rotate_90 | RR_Rotate_270)) {
> > -                source_width = mode->mode.height;
> > -                source_height = mode->mode.width;
> > -            }
> > +            RRCrtcGetScanoutSize(crtc, &source_width, &source_height);
> >  
> >              if (crtc->x + source_width > stuff->width ||
> >                  crtc->y + source_height > stuff->height)
> > 
> 
> Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>
> 
> Is there any reason to have patch 1 separately?

The idea was just to preserve the direct fix in the git history, in case some
issue crops up with RRCrtcGetScanoutSize(), or there was some objection to its
use.

RRCrtcGetScanoutSize() adds a significant amount of complexity compared to
simply checking the rotation and swapping width and height, since it operates by
applying the transformation matrix crtc->transform to the mode. 

> P.S. Looks like there are similar issues in randr/rrcrtc.c, in
> crtc_to_box and rrCheckPixmapBounding.

True, looks like those should be fixed as well.

Thanks,
Alex

> -- 
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer
>