[Mesa-dev] [PATCH v2 2/3] gallium: Add capability for ARB_robust_buffer_access_behavior

[Bas Nieuwenhuizen]

On Tue, Apr 12, 2016 at 6:09 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 12.04.2016 um 16:23 schrieb Bas Nieuwenhuizen:
>> On Tue, Apr 12, 2016 at 3:56 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>>> Am 12.04.2016 um 15:12 schrieb Bas Nieuwenhuizen:
>>>> Signed-off-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
>>>> ---
>>>>  src/gallium/docs/source/screen.rst               | 5 +++++
>>>>  src/gallium/drivers/freedreno/freedreno_screen.c | 1 +
>>>>  src/gallium/drivers/i915/i915_screen.c           | 1 +
>>>>  src/gallium/drivers/ilo/ilo_screen.c             | 1 +
>>>>  src/gallium/drivers/llvmpipe/lp_screen.c         | 1 +
>>>>  src/gallium/drivers/nouveau/nv30/nv30_screen.c   | 1 +
>>>>  src/gallium/drivers/nouveau/nv50/nv50_screen.c   | 1 +
>>>>  src/gallium/drivers/nouveau/nvc0/nvc0_screen.c   | 1 +
>>>>  src/gallium/drivers/r300/r300_screen.c           | 1 +
>>>>  src/gallium/drivers/r600/r600_pipe.c             | 1 +
>>>>  src/gallium/drivers/radeonsi/si_pipe.c           | 1 +
>>>>  src/gallium/drivers/softpipe/sp_screen.c         | 1 +
>>>>  src/gallium/drivers/svga/svga_screen.c           | 1 +
>>>>  src/gallium/drivers/swr/swr_screen.cpp           | 1 +
>>>>  src/gallium/drivers/vc4/vc4_screen.c             | 1 +
>>>>  src/gallium/drivers/virgl/virgl_screen.c         | 1 +
>>>>  src/gallium/include/pipe/p_defines.h             | 1 +
>>>>  src/mesa/state_tracker/st_extensions.c           | 1 +
>>>>  18 files changed, 22 insertions(+)
>>>>
>>>> diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst
>>>> index 824f580..b281029 100644
>>>> --- a/src/gallium/docs/source/screen.rst
>>>> +++ b/src/gallium/docs/source/screen.rst
>>>> @@ -331,6 +331,11 @@ The integer capabilities:
>>>>    primitive on a layer is obtained from ``PIPE_CAP_MAX_TEXTURE_ARRAY_LAYERS``
>>>>    even though it can be larger than the number of layers supported by either
>>>>    rendering or textures.
>>>> +* ``PIPE_CAP_ROBUST_BUFFER_ACCESS``: Implementation uses bounds checking on
>>>> +  resource accesses by shader if the context is created with
>>>> +  PIPE_CONTEXT_ROBUST_BUFFER_ACCESS. See the ARB_robust_buffer_access_behavior
>>>> +  extension for information on the required behavior for out of bounds accesses
>>>> +  and accesses to unbound resources.
>>> I think this is a bit of a misnomer. "Robust buffer access" is already
>>> provided by just ARB_robustness in gl terms (meaning it just isn't
>>> allowed to crash for out-of-bounds access generally).
>>> Although admitedly PIPE_CAP_ROBUST_BUFFER_ACCESS_BEHAVIOR would be quite
>>> a long name...
>>>
>>
>> There is already a PIPE_CONTEXT_ROBUST_BUFFER_ACCESS flag in gallium
>> with the comment
>>
>> /**
>>  * Whether out-of-bounds shader loads must return zero and out-of-bounds
>>  * shader stores must be dropped.
>>  */
>>
>> Which makes me suspect it is really targeted at
>> ARB_robust_buffer_access_behavior, not ARB_robustness. That said,
>> either name would be fine with me.
>
> You are right, but that flag is really getting passed through context
> creation (that is, cannot really be targeted at the _behavior
> extension). So the comment might not be quite right.
> IIRC it really works like this:

ok, I'll rename the cap to PIPE_CAP_ROBUST_BUFFER_ACCESS_BEHAVIOR.

> no ARB_robustness, or context not created with robust flag:
> implementation is allowed to crash (I believe though all drivers
> actually won't crash but return undefined values already anyway)
>
> ARB_robustness, robust flag, no ARB_rbab extension:
> implementation may not crash, undefined values
>
> ARB_robustness, robust flag, ARB_rbab extension:
> sort of defined behavior (still not quite as strict as d3d10 IIRC)
>
> I thought there wouldn't be a cap bit for ARB_robustness, but that's not
> quite true. If I see that right drivers need to support
> PIPE_CAP_DEVICE_RESET_STATUS_QUERY otherwise ARB_robustness won't be
> exposed (and neither should ARB_rbab be in this case as this is a
> requirement).
>

ARB_robustness is always exposed by mesa: there is no extension enable bit.

- Bas