|Summary:||Calculating the scissors fields when the y is flipped (0 on top) can generate negative numbers that will cause assertion failure later on.|
|Product:||Mesa||Reporter:||Eleni Maria Stea <estea>|
|Component:||Drivers/DRI/i965||Assignee:||Intel 3D Bugs Mailing List <intel-3d-bugs>|
|Status:||RESOLVED FIXED||QA Contact:||Intel 3D Bugs Mailing List <intel-3d-bugs>|
|i915 platform:||i915 features:|
|Attachments:||piglit test that triggers SCISSOR bbox clamping errors when the y is flipped (0 on top)|
Description Eleni Maria Stea 2018-12-10 10:13:55 UTC
Created attachment 142768 [details] piglit test that triggers SCISSOR bbox clamping errors when the y is flipped (0 on top) In the function set_scissor_bits, when the y is flipped we might generate negative values. We should clamp them because all the GenX(SCISSOR) struct fields are unsigned and we will have an assertion failure when GenX(SCISSOR_RECT_pack) is going to be called. I am attaching a piglit test that can trigger this problem. When the window size is 800x600 calling glViewport with the values: glViewport(2, 602, 262, 296); before glDrawArrays will cause the assertion failure. I am about to submit a fix for this bug.
Comment 1 Eleni Maria Stea 2019-01-14 14:10:29 UTC
https://patchwork.freedesktop.org/patch/267292/ the patch with the fix.
Comment 2 Paul 2019-02-21 15:34:11 UTC
I've run the piglit-test on latest master's mesa (19.1.0) with applied patch - the test successfully passed. Also, the patch fixes the issue with crash of the totem-app - issue from https://bugs.freedesktop.org/show_bug.cgi?id=109594 . I've tested on: Intel Core i5-4300M with Intel® HD Graphics 4600 on Haswell OS: Manjaro on Gnome 3.30.2 Kernel: 4.20.10
Comment 3 Nanley Chery 2019-02-26 16:42:55 UTC
This bug has been fixed by: commit fd37a19ac4c8b2ebff330b2a06a7f311f7d478e3 Author: Eleni Maria Stea <firstname.lastname@example.org> Date: Fri Feb 22 23:02:30 2019 +0200 i965: fixed clamping in set_scissor_bits when the y is flipped 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 should be a check if (bbox >= fbheight) bbox = 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 and bbox are calculated, and the 2nd is more clear, I replaced it. (Nanley Chery) v3: - Reversed the CLAMP change in bbox 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: <email@example.com> Tested-by: Paul Chelombitko <firstname.lastname@example.org> Reviewed-by: Nanley Chery <email@example.com>
Comment 4 Nanley Chery 2019-02-26 16:44:39 UTC
Thank you for looking into this, Eleni.