Re: [Mesa-dev] [PATCH] mesa: Implement ARB_shader_viewport_layer_array for i965

[Kenneth Graunke]

On Friday, September 9, 2016 4:14:55 PM PDT Dylan Baker wrote:
> This extension is a combination of AMD_vertex_shader_viewport_index and
> AMD_vertex_shader_layer, making it rather trivial to implement.
> 
> For gallium I *think* this needs a new cap because of the addition of
> support in tessellation evaluation shaders, and since I don't have any
> hardware to test it on, I've left that for someone else to wire up.
> 
> Since this requires GL 4.1, this is only available on gen8+.

You've actually enabled this on Gen6+, by virtue of:

> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c 
> b/src/mesa/drivers/dri/i965/intel_extensions.c
> index 0f28546..6573bc2 100644
> --- a/src/mesa/drivers/dri/i965/intel_extensions.c
> +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
> @@ -330,6 +330,7 @@ intelInitExtensions(struct gl_context *ctx)
> */
>if (ctx->API == API_OPENGL_CORE) {
>   ctx->Extensions.ARB_shader_subroutine = true;
> + ctx->Extensions.ARB_shader_viewport_layer_array = true;
>   ctx->Extensions.ARB_viewport_array = true;
>   ctx->Extensions.AMD_vertex_shader_viewport_index = true;
>}

   ^^^ this is in a Gen6+ and core only block.

I agree with Ilia that this is the right thing to do - it makes sense
to expose it where AMD_vertex_shader_viewport_index is already exposed.

I'd just drop that sentence from your commit message.

> diff --git a/src/mesa/main/extensions_table.h 
> b/src/mesa/main/extensions_table.h
> index 75cdcb8..38636b4 100644
> --- a/src/mesa/main/extensions_table.h
> +++ b/src/mesa/main/extensions_table.h
> @@ -115,6 +115,7 @@ EXT(ARB_shader_storage_buffer_object, 
> ARB_shader_storage_buffer_object
>  EXT(ARB_shader_subroutine   , ARB_shader_subroutine  
> ,  x , GLC,  x ,  x , 2010)
>  EXT(ARB_shader_texture_image_samples, 
> ARB_shader_texture_image_samples   , GLL, GLC,  x ,  x , 2014)
>  EXT(ARB_shader_texture_lod  , ARB_shader_texture_lod 
> , GLL, GLC,  x ,  x , 2009)
> +EXT(ARB_shader_viewport_layer_array , 
> ARB_shader_viewport_layer_array, GLL, GLC,  x ,  x , 2015)
>  EXT(ARB_shading_language_100, dummy_true 
> , GLL,  x ,  x ,  x , 2003)
>  EXT(ARB_shading_language_420pack, ARB_shading_language_420pack   
> , GLL, GLC,  x ,  x , 2011)
>  EXT(ARB_shading_language_packing, ARB_shading_language_packing   
> , GLL, GLC,  x ,  x , 2011)

As Ilia mentioned, please drop "GLL", changing it to " x ".

Otherwise, this is:
Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa: Implement ARB_shader_viewport_layer_array for i965

[Dylan Baker]

I like the easy way out in this case. Thanks for the pointer.

On Fri, Sep 9, 2016, at 18:03, Ilia Mirkin wrote:
> On Fri, Sep 9, 2016 at 8:59 PM, Dylan Baker  wrote:
> >> Some piglit tests to check the TES behaviour would be nice.
> >
> > Yeah, I figured. I'll have to go read up on how tessellation works.
> 
> While I would hate to discourage you from such a joyous learning
> experience, if you want the quickest way to success, mash up the AMD_*
> tests with the ARB_tessellation_shader nop.shader_test or
> sanity.shader_test.
> 
>   -ilia


-- 
  Dylan Baker
  dy...@pnwbakers.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa: Implement ARB_shader_viewport_layer_array for i965

[Ilia Mirkin]

On Fri, Sep 9, 2016 at 8:59 PM, Dylan Baker  wrote:
>> Some piglit tests to check the TES behaviour would be nice.
>
> Yeah, I figured. I'll have to go read up on how tessellation works.

While I would hate to discourage you from such a joyous learning
experience, if you want the quickest way to success, mash up the AMD_*
tests with the ARB_tessellation_shader nop.shader_test or
sanity.shader_test.

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa: Implement ARB_shader_viewport_layer_array for i965

[Dylan Baker]

Quoting Ilia Mirkin (2016-09-09 16:40:15)
> On Fri, Sep 9, 2016 at 7:14 PM, Dylan Baker  wrote:
> > +EXT(ARB_shader_viewport_layer_array , 
> > ARB_shader_viewport_layer_array, GLL, GLC,  x ,  x , 2015)
> 
> This relies on GL 3.2 at least, so I think this should just be GLC, not GLL.

The descriptions of what GLL, GLC, etc do are a bit... vague.

I'll change that before I push anything.

> 
> Also, you mention it requires GL 3.1, but really it just requires
> GL_ARB_viewport_array (which became core in GL 4.1). I think it'd be
> safe to enable this on SNB+. Your call.

The spec explicitly calls out that it requires GL 4.1, so I guess the
question is whether we want to to honor that requirement or not.

> 
> Core bits are
> 
> Reviewed-by: Ilia Mirkin 
> 
> Some piglit tests to check the TES behaviour would be nice.

Yeah, I figured. I'll have to go read up on how tessellation works.


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa: Implement ARB_shader_viewport_layer_array for i965

[Ilia Mirkin]

On Fri, Sep 9, 2016 at 7:14 PM, Dylan Baker  wrote:
> +EXT(ARB_shader_viewport_layer_array , 
> ARB_shader_viewport_layer_array, GLL, GLC,  x ,  x , 2015)

This relies on GL 3.2 at least, so I think this should just be GLC, not GLL.

Also, you mention it requires GL 3.1, but really it just requires
GL_ARB_viewport_array (which became core in GL 4.1). I think it'd be
safe to enable this on SNB+. Your call.

Core bits are

Reviewed-by: Ilia Mirkin 

Some piglit tests to check the TES behaviour would be nice.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev