[Mesa-dev] [PATCH] i965: enable OES_primitive_bounding_box with the no-op implementation

[Matt Turner]

On Tue, Aug 30, 2016 at 6:07 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Tue, Aug 30, 2016 at 9:01 PM, Matt Turner <mattst88 at gmail.com> wrote:
>> On Tue, Aug 30, 2016 at 5:09 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> ---
>>>
>>> This passes a jenkins run except for 2 tests:
>>>
>>> dEQP-GLES31.functional.primitive_bounding_box.call_order.bbox_first_viewport_second
>>> dEQP-GLES31.functional.primitive_bounding_box.call_order.viewport_first_bbox_second
>>>
>>> In both cases, they assume that they can create tessellation shaders, whereas
>>> this is not yet enabled on i965. This should fix itself when that feature is
>>> flipped on.
>>>
>>>  docs/features.txt                            | 2 +-
>>>  docs/relnotes/12.1.0.html                    | 2 +-
>>>  src/mesa/drivers/dri/i965/brw_context.c      | 3 +++
>>>  src/mesa/drivers/dri/i965/intel_extensions.c | 1 +
>>>  4 files changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/docs/features.txt b/docs/features.txt
>>> index 6959b0b..537880d 100644
>>> --- a/docs/features.txt
>>> +++ b/docs/features.txt
>>> @@ -262,7 +262,7 @@ GLES3.2, GLSL ES 3.2:
>>>    GL_OES_draw_elements_base_vertex                      DONE (all drivers)
>>>    GL_OES_geometry_shader                                DONE (i965/gen8+)
>>>    GL_OES_gpu_shader5                                    DONE (all drivers that support GL_ARB_gpu_shader5)
>>> -  GL_OES_primitive_bounding_box                         DONE (nvc0, radeonsi)
>>> +  GL_OES_primitive_bounding_box                         DONE (i965/hsw+, nvc0, radeonsi)
>>>    GL_OES_sample_shading                                 DONE (i965, nvc0, r600, radeonsi)
>>>    GL_OES_sample_variables                               DONE (i965, nvc0, r600, radeonsi)
>>>    GL_OES_shader_image_atomic                            DONE (all drivers that support GL_ARB_shader_image_load_store)
>>> diff --git a/docs/relnotes/12.1.0.html b/docs/relnotes/12.1.0.html
>>> index 21747d3..6f4ed83 100644
>>> --- a/docs/relnotes/12.1.0.html
>>> +++ b/docs/relnotes/12.1.0.html
>>> @@ -59,7 +59,7 @@ Note: some of the new features are only available with certain drivers.
>>>  <li>GL_KHR_texture_compression_astc_sliced_3d on i965</li>
>>>  <li>GL_OES_copy_image on nv50, nvc0, r600, radeonsi, softpipe, llvmpipe</li>
>>>  <li>GL_OES_geometry_shader on i965/gen8+</li>
>>> -<li>GL_OES_primitive_bounding_box on nvc0, radeonsi</li>
>>> +<li>GL_OES_primitive_bounding_box on i965/hsw+, nvc0, radeonsi</li>
>>>  <li>GL_OES_texture_cube_map_array on i965/gen8+</li>
>>>  </ul>
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
>>> index 41c972c..fec6df8 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_context.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_context.c
>>> @@ -772,6 +772,9 @@ brw_initialize_context_constants(struct brw_context *brw)
>>>     ctx->Const.MaxFramebufferHeight = 16384;
>>>     ctx->Const.MaxFramebufferLayers = ctx->Const.MaxArrayTextureLayers;
>>>     ctx->Const.MaxFramebufferSamples = max_samples;
>>> +
>>> +   /* OES_primitive_bounding_box */
>>> +   ctx->Const.NoPrimitiveBoundingBoxOutput = true;
>>>  }
>>>
>>>  static void
>>> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c b/src/mesa/drivers/dri/i965/intel_extensions.c
>>> index 76b585d..0f28546 100644
>>> --- a/src/mesa/drivers/dri/i965/intel_extensions.c
>>> +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
>>> @@ -354,6 +354,7 @@ intelInitExtensions(struct gl_context *ctx)
>>>        ctx->Extensions.ARB_texture_view = true;
>>>        ctx->Extensions.ARB_shader_storage_buffer_object = true;
>>>        ctx->Extensions.EXT_shader_samples_identical = true;
>>> +      ctx->Extensions.OES_primitive_bounding_box = true;
>>
>> This is in an if (brw->gen >= 7) case, but all of the comments above
>> say i965/hsw+. Shouldn't they say i965/gen7+?
>
> I wasn't sure what the convention was. There's absolutely 0
> functionality required from the driver (other than setting that const
> bit to true, for hardware that doesn't do this bounding box business -
> it's mostly directed at tilers). However the ext requires ES 3.1. gen7
> will get ES 3.1 some day, but it doesn't have it today. So I just put
> i965/hsw+ for now. The ext won't show up in the extension string for
> gen7 right now, but will magically appear once ES 3.1 is flipped on.
>
> Let me know how you want this handled.

Right, okay.

I'd just say i965/gen7+ then (a vacuously true statement in any case)
-- I don't think anyone would remember to update the release notes for
this extension when ES 3.1 is enabled.