[Mesa-dev] [PATCH 0/2] r600: Fix array texture slice index evaluation

Roland Scheidegger sroland at vmware.com
Mon Jun 25 21:47:20 UTC 2018


Am 25.06.2018 um 22:13 schrieb Gert Wollny:
> Am Montag, den 25.06.2018, 17:36 +0200 schrieb Roland Scheidegger:
>> I didn't actually get the original email for some reason, so can't
>> comment inline as I'm just looking it up at patchwork...
>> But the array offset stuff (the first patch) looks completely bogus
>> to me, array textures do not support offsets for the array index, at
>> least not in any shader language I know of.
> It is legal to derive the texture array slice index from float value
> and the formula given in the standard is "floor(z + 0.5)". 
> 
> The normal rounding mode doesn't handle this, probably because it
> actually does "round-nearest-even to n.6 and drop fraction when point
> sampling", so it doesn't really round to integers, but to some fixed
> point format that then might get truncated. (It is not written what is
> done if the TEX_Z_FILTER_NONE is set, but I assume it must be trunc
> just like with TEX_Z_FILTER_POINT). When looking at the test results I
> figured out that the slice selection must be off, and voila,  half of
> the tests are fixed by adding 0.5 to the z-coordinate, be it by
> injecting the code into the shader that adds this value to the
> coordinate or by putting this value into the offsets. The first
> approach is obviously cheaper, because it doesn't add any instruction.
> I don't think that it is relevant whether the shader language supports
> this, because in the end this gets translated to byte code that needs
> to communicate the intent to the hardware.
Alright albeit you have logic to handle incoming z offsets, whereas that
should always be 0.
To be honest I'm actually kind of surprised the hw would honor texel
offsets for array coordinates (the term "texel offset" wouldn't even
apply) - this is done after denormalization and coord wrapping, neither
of which apply to the array coord.

Would be surprising if the hw did trunc instead of round (or
+0.5/floor), are you sure that's really (always) the case? I'm wondering
if there could be some (bogus) dependency on other texture/sampler state
(similar to the gather issues when selecting the wrong texels for int
textures).
At a quick glance radeonsi doesn't seem to do anything like this, and
generally I think the sampler hw shares most of the bugs, at least for
early gcn chips... (albeit I don't know if the tests would pass there
neither).

But anyway, if the hw really does trunc and not round for array slice
selection then that is the right thing to do indeed (a directed test
should easily reveal the switch-over points).


> 
>> I'm not really sure about the 2nd patch, what exactly is the
>> difference with the ordinary rounding and the new one? Is there just
>> a difference for values exactly between 2 integers (e.g. 1.5 etc.)?
>> In this case I would suspect the driver is allowed to pick either
>> value and the test is bogus.
> The remaining tests fail for an inaccuracy in the rounding mode (These
> tests explicitly check rounding by varying the slice index from 1.49999
> to 1.50001). The default rounding mode that rounds to n.6 fixed point
> can not capturere this accuracy, and therefore these tests fail. Using
> trunc, which for positive values is equal to floor, fixes this, and
> given that the standard is explicitely written to use floor, I don't
> think that the tests are bogous. One could probably achieve the same in
> the shader, but again, programming the sampler like this saves
> instructions in the shader. 
> 
>> If that's not the case, it looks like it actually needs to be one
>> patch?
>> You're adding the 0.5 offset in one but adjust the sampler state
>> which probably needs to match in another?
> Well, it is two patches because that's how I went forward fixing these
> tests (like described above), but it would probably okay to squash
> them. I'll have to revise the first patch anyway.

I suppose I was misinterpreting what this does, I thought you needed to
add 0.5 because of using different Z_FILTER and TRUNC_COORD values, not
to fix another issue.

Although TRUNCATE_COORD would apply to all coords, which seems like a
bad idea (regardless what the hell those z_filter/truncate_coord values
actually do...). (Among other things, it would mean with point
filtering, you could actually get different texels selected depending if
you declared a texture as 2d or as 2d array with 1 layer - not good.)

Regardless the "bogus" test, that is simply based on GL not actually
requiring any specific precision in that area, therefore it's not
reasonable to expect 100% accurate (to float) results. That is, I think
it would be well allowed to round 1.49999 to 2 there. So imho there's no
value in trying to fix this just to allow the test to pass.

Roland



> 
> best, 
> Gert
> 
>> Roland
>>
>> Am 25.06.2018 um 07:54 schrieb Dave Airlie:
>>> /home/airlied/devel/piglit/bin/textureGather fs nonconst r 0 float
>>> 2DArray repeat
>>>
>>> amongst others appears to regress with these two.
>>>
>>> Dave.
>>>
>>>
>>> On 22 June 2018 at 19:37, Gert Wollny <gert.wollny at collabora.com>
>>> wrote:
>>>> these two patches correct the offets and the rounding modes for
>>>> the
>>>> index evaluation when accessing texture arrays. The patches were
>>>> tested
>>>> with the gles3 test suite where they fix a number of tests
>>>> related to 2D
>>>> texture arrays and didn't show any regressions.
>>>>
>>>> Best,
>>>> Gert
>>>>
>>>> Gert Wollny (2):
>>>>   r600: correct texture offset for array index lookup
>>>>   r600: set rounding mode for texture array layer selection
>>>>
>>>>  src/gallium/drivers/r600/evergreen_state.c | 21 ++++++++++
>>>>  src/gallium/drivers/r600/r600_shader.c     | 64
>>>> +++++++++++++++++++++++++++++-
>>>>  2 files changed, 84 insertions(+), 1 deletion(-)
>>>>
>>>> --
>>>> 2.16.4
>>>>
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fli
>>> sts.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-
>>> dev&data=02%7C01%7Csroland%40vmware.com%7C038ae632c8354209810c08d5d
>>> a601219%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C1%7C63665502853319
>>> 4684&sdata=gvQB0DNGvaNwBN5EHlJNrXED4biq3xC4rFqNEYqS3xs%3D&reserved=
>>> 0
>>>
>>
>>
> 



More information about the mesa-dev mailing list