On Mon, Feb 25, 2019 at 03:14:10PM -0800, Dylan Baker wrote: > Quoting Eleni Maria Stea (2019-02-22 13:02:30) > > Calculating the scissor rectangle fields with the y flipped (0 on top) > > can generate negative values that will cause assertion failure later on > > as the scissor fields are all unsigned. We must clamp the bbox values > > again to make sure they don't exceed the fb_height. Also fixed a > > calculation error. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108999 > > https://bugs.freedesktop.org/show_bug.cgi?id=109594 > > > > v2: > > - I initially clamped the values inside the if (Y is flipped) case > > and I made a mistake in the calculation: the clamp of the bbox[2] should > > be a check if (bbox[2] >= fbheight) bbox[2] = fbheight - 1 instead and I > > shouldn't have changed the ScissorRectangleYMax calculation. As the > > fixed code is equivalent with using CLAMP instead of MAX2 at the top of > > the function when bbox[2] and bbox[3] are calculated, and the 2nd is more > > clear, I replaced it. (Nanley Chery) > > > > v3: > > - Reversed the CLAMP change in bbox[3] as the API guarantees that the > > viewport height is positive. (Nanley Chery) > > > > v4: > > - Added nomination for the mesa-stable branch and the link to the second > > bugzilla bug (Nanley Chery) > > > > CC: <mesa-sta...@lists.freedesktop.org> > > Tested-by: Paul Chelombitko <qamonste...@gmail.com> > > Reviewed-by: Nanley Chery <nanley.g.ch...@intel.com> > > --- > > src/mesa/drivers/dri/i965/genX_state_upload.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c > > b/src/mesa/drivers/dri/i965/genX_state_upload.c > > index 027dad1e089..73c983ce742 100644 > > --- a/src/mesa/drivers/dri/i965/genX_state_upload.c > > +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c > > @@ -2446,7 +2446,7 @@ set_scissor_bits(const struct gl_context *ctx, int i, > > > > bbox[0] = MAX2(ctx->ViewportArray[i].X, 0); > > bbox[1] = MIN2(bbox[0] + ctx->ViewportArray[i].Width, fb_width); > > - bbox[2] = MAX2(ctx->ViewportArray[i].Y, 0); > > + bbox[2] = CLAMP(ctx->ViewportArray[i].Y, 0, fb_height); > > bbox[3] = MIN2(bbox[2] + ctx->ViewportArray[i].Height, fb_height); > > _mesa_intersect_scissor_bounding_box(ctx, i, bbox); > > > > -- > > 2.20.1 > > > > Do you have push access? I'd like to get this merged so we can close said > bugs, > and Nanley or I can push this for you if you don't have access. >
I haven't landed this patch because its piglit test isn't catching the error in CI. I'm hoping we could resolve that soon though. -Nanley _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev