[Mesa-dev] [PATCH 2/2] mesa: Implement glGet*(GL_PRIMITIVE_RESTART_FOR_PATCHES_SUPPORTED).

Mon May 23 14:38:23 UTC 2016

Other than the get_shader_param thing, this series is:

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek

On Sat, May 21, 2016 at 8:06 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On Saturday, May 21, 2016 10:41:30 AM PDT Ilia Mirkin wrote:
>> On Sat, May 21, 2016 at 1:31 AM, Kenneth Graunke <kenneth at whitecape.org>
> wrote:
>> > Technically, this was introduced with GL 4.4.  However, I believe it
>> > was intended to be retroactive.  As far as I know, AMD has never
>> > supported primitive restart with patches, while NVidia and Intel do.
>> > This necessitated the need for a query which would allow applications
>> > to figure out whether this was usable or not.
>> >
>> > I decided to expose it everywhere ARB_tessellation_shader is exposed.
>> > (It's also in both OES and EXT_tessellation_shader.)
>> >
>> > Enable this for i965 and Gallium drivers which expose the capability.
>> >
>> > Bugzilla: https://cvs.khronos.org/bugzilla/show_bug.cgi?id=10364
>> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>> > ---
>> >  src/mapi/glapi/gen/ARB_tessellation_shader.xml | 1 +
>> >  src/mesa/drivers/dri/i965/brw_context.c        | 1 +
>> >  src/mesa/main/context.c                        | 1 +
>> >  src/mesa/main/get_hash_params.py               | 1 +
>> >  src/mesa/main/mtypes.h                         | 1 +
>> >  src/mesa/state_tracker/st_extensions.c         | 3 +++
>> >  6 files changed, 8 insertions(+)
>> >
>> > I'm working on a branch to add OES_tessellation_shader support, and I
>> > noticed this query that we didn't implement.  I tested the query on
>> > i965 with the dEQP-GLES31.functional.tessellation.state_query.
>> > primitive_restart_for_patches_supported test.  It passed.
>> >
>> > diff --git a/src/mapi/glapi/gen/ARB_tessellation_shader.xml b/src/mapi/
> glapi/gen/ARB_tessellation_shader.xml
>> > index 77f2228..5ffc2ba 100644
>> > --- a/src/mapi/glapi/gen/ARB_tessellation_shader.xml
>> > +++ b/src/mapi/glapi/gen/ARB_tessellation_shader.xml
>> > @@ -48,6 +48,7 @@
>> >      <enum value="0x8E88" name="TESS_CONTROL_SHADER"/>
>> >      <enum value="0x8E89" name="MAX_TESS_CONTROL_UNIFORM_BLOCKS"/>
>> >      <enum value="0x8E8A" name="MAX_TESS_EVALUATION_UNIFORM_BLOCKS"/>
>> > +    <enum value="0x8221" name="PRIMITIVE_RESTART_FOR_PATCHES_SUPPORTED"/>
>> >
>> >      <function name="PatchParameteri">
>> >          <param name="pname" type="GLenum"/>
>> > diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/
> dri/i965/brw_context.c
>> > index 1ab02ae..552e5ec 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_context.c
>> > +++ b/src/mesa/drivers/dri/i965/brw_context.c
>> > @@ -565,6 +565,7 @@ brw_initialize_context_constants(struct brw_context
> *brw)
>> >        ctx->Const.MaxClipPlanes = 8;
>> >
>> >     ctx->Const.LowerTessLevel = true;
>> > +   ctx->Const.PrimitiveRestartForPatches = true;
>> >
>> >     ctx->Const.Program[MESA_SHADER_VERTEX].MaxNativeInstructions = 16 *
> 1024;
>> >     ctx->Const.Program[MESA_SHADER_VERTEX].MaxAluInstructions = 0;
>> > diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
>> > index f690799..7c0a6ef 100644
>> > --- a/src/mesa/main/context.c
>> > +++ b/src/mesa/main/context.c
>> > @@ -725,6 +725,7 @@ _mesa_init_constants(struct gl_constants *consts,
> gl_api api)
>> >     consts->Program[MESA_SHADER_TESS_EVAL].MaxTextureImageUnits =
> MAX_TEXTURE_IMAGE_UNITS;
>> >     consts->MaxTessPatchComponents = MAX_TESS_PATCH_COMPONENTS;
>> >     consts->MaxTessControlTotalOutputComponents =
> MAX_TESS_CONTROL_TOTAL_OUTPUT_COMPONENTS;
>> > +   consts->PrimitiveRestartForPatches = false;
>> >  }
>> >
>> >
>> > diff --git a/src/mesa/main/get_hash_params.py b/src/mesa/main/
> get_hash_params.py
>> > index 8f693ac..2124072 100644
>> > --- a/src/mesa/main/get_hash_params.py
>> > +++ b/src/mesa/main/get_hash_params.py
>> > @@ -912,6 +912,7 @@ descriptor=[
>> >    [ "MAX_TESS_EVALUATION_UNIFORM_BLOCKS",
> "CONTEXT_INT(Const.Program[MESA_SHADER_TESS_EVAL].MaxUniformBlocks),
> extra_ARB_tessellation_shader" ],
>> >    [ "MAX_COMBINED_TESS_CONTROL_UNIFORM_COMPONENTS",
> "CONTEXT_INT(Const.Program[MESA_SHADER_TESS_CTRL].MaxCombinedUniformComponents),
> extra_ARB_tessellation_shader" ],
>> >    [ "MAX_COMBINED_TESS_EVALUATION_UNIFORM_COMPONENTS",
> "CONTEXT_INT(Const.Program[MESA_SHADER_TESS_EVAL].MaxCombinedUniformComponents),
> extra_ARB_tessellation_shader" ],
>> > +  [ "PRIMITIVE_RESTART_FOR_PATCHES_SUPPORTED",
> "CONTEXT_BOOL(Const.PrimitiveRestartForPatches),
> extra_ARB_tessellation_shader" ],
>> >  # Dependencies on GL_ARB_tessellation_shader
>> >    [ "MAX_TESS_CONTROL_ATOMIC_COUNTER_BUFFERS",
> "CONTEXT_INT(Const.Program[MESA_SHADER_TESS_CTRL].MaxAtomicBuffers),
> extra_ARB_shader_atomic_counters_and_tessellation" ],
>> >    [ "MAX_TESS_CONTROL_ATOMIC_COUNTERS",
> "CONTEXT_INT(Const.Program[MESA_SHADER_TESS_CTRL].MaxAtomicCounters),
> extra_ARB_shader_atomic_counters_and_tessellation" ],
>> > diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> > index 569e0ac..feab1a1 100644
>> > --- a/src/mesa/main/mtypes.h
>> > +++ b/src/mesa/main/mtypes.h
>> > @@ -3725,6 +3725,7 @@ struct gl_constants
>> >     GLuint MaxTessPatchComponents;
>> >     GLuint MaxTessControlTotalOutputComponents;
>> >     bool LowerTessLevel; /**< Lower gl_TessLevel* from float[n] to vecn?
> */
>> > +   bool PrimitiveRestartForPatches;
>> >  };
>> >
>> >
>> > diff --git a/src/mesa/state_tracker/st_extensions.c b/src/mesa/
> state_tracker/st_extensions.c
>> > index ea60e41..eeb80dd 100644
>> > --- a/src/mesa/state_tracker/st_extensions.c
>> > +++ b/src/mesa/state_tracker/st_extensions.c
>> > @@ -314,6 +314,9 @@ void st_init_limits(struct pipe_screen *screen,
>> >     }
>> >
>> >     c->LowerTessLevel = true;
>> > +   c->PrimitiveRestartForPatches =
>> > +      screen->get_shader_param(screen, sh,
>> > +                               PIPE_CAP_PRIMITIVE_RESTART_FOR_PATCHES);
>>
>> I think you want ->get_param(screen, PIPE_CAP_FOO). shader_param is
>> for PIPE_SHADER_CAP, which are per-shader stage. (What is "sh" here?)
>
> Whoops, yes!  Fixed locally.
>
>> Otherwise this series is
>>
>> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
>
> Thanks!