Bug 108999

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/i965Assignee: Intel 3D Bugs Mailing List <intel-3d-bugs>
Status: RESOLVED FIXED QA Contact: Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity: normal    
Priority: medium CC: estea
Version: git   
Hardware: Other   
OS: All   
Whiteboard:
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 <estea@igalia.com>
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[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-stable@lists.freedesktop.org>
    Tested-by: Paul Chelombitko <qamonstergl@gmail.com>
    Reviewed-by: Nanley Chery <nanley.g.chery@intel.com>
Comment 4 Nanley Chery 2019-02-26 16:44:39 UTC
Thank you for looking into this, Eleni.
Comment 5 JasonWahl 2019-08-27 07:53:38 UTC
In the last patch, bugs were fixed. What is happening now? I have problems with the same code on the site https://大吉カジノ.jp/. Is a new update possible in the near future?

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.