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 <[email protected]> > > --- > > 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 <[email protected]> > > 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 >
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
