Bug 106074 - radv: si_scissor_from_viewport returns incorrect result when using half-pixel viewport offset
Summary: radv: si_scissor_from_viewport returns incorrect result when using half-pixel...
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/Vulkan/radeon (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: mesa-dev
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-04-16 11:35 UTC by Philip Rebohle
Modified: 2018-04-17 20:13 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:

Attachments
Sample program to demonstrate the issue (1.42 KB, text/x-csrc)
2018-04-16 11:35 UTC, Philip Rebohle 		Details
Proposed patch (850 bytes, patch)
2018-04-16 14:52 UTC, Philip Rebohle 		Details | Splinter Review
View All

Description Philip Rebohle 2018-04-16 11:35:40 UTC 
Created attachment 138865 [details]
Sample program to demonstrate the issue

For the following viewport, si_scissor_from_viewport returns an incorrect scissor rectangle:

  VkViewport vp = { 0.5f, 1.5f, 1.0f, -1.0f, 0.0f, 1.0f };

Expected result:  offset = (0,0), size = (1,1)
Actual result:    offset = (1,1), size = (0,0)

This breaks Final Fantasy XIV on DXVK, see:
https://github.com/doitsujin/dxvk/issues/208

The attachment contains a small C program to demonstrate the issue with the relevant functions directly taken from si_cmd_buffer.c.
Comment 1 Philip Rebohle 2018-04-16 14:52:08 UTC 
Created attachment 138867 [details] [review]
Proposed patch

The attached patch fixes the issue in FF XIV. I'm not sure if that is entirely correct, but the use of the integer version of 'abs' in the following code looks incorrect given that 'scale[i]' can be non-integer even if the viewport size is integer.

    rect.offset.x = translate[0] - abs(scale[0]);
    rect.offset.y = translate[1] - abs(scale[1]);
    rect.extent.width = ceilf(translate[0] + abs(scale[0])) - rect.offset.x;
    rect.extent.height = ceilf(translate[1] + abs(scale[1])) - rect.offset.y;

Also it seems that my assumption that the size should be (1,1) was incorrect, it should indeed be (2,2). This is in line with what RadeonSI returns.
Comment 2 Samuel Pitoiset 2018-04-17 20:13:21 UTC 
I have pushed your patch, thanks!

https://cgit.freedesktop.org/mesa/mesa/commit/?id=893e19efb74edd6133a607e09338bf5d449632f1

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.