Re: [Mesa-dev] [PATCH] mesa: default DepthMode to GL_RED on ES 3.0

[Ilia Mirkin]

On Wed, Feb 17, 2016 at 4:00 PM, Ian Romanick  wrote:
> I think we need to change DepthMode in GLES when the texture image is
> set (by glTexImage or glTexStorage).  I think doing the mode swizzle in
> the driver means glGetTexParameteriv(GL_DEPTH_MODE) will return the
> wrong thing.

As you pointed out on IRC, GLES doesn't have this. So it should be
fine either way...

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

Re: [Mesa-dev] [PATCH] mesa: default DepthMode to GL_RED on ES 3.0

[Ian Romanick]

On 02/16/2016 11:14 PM, Kenneth Graunke wrote:
> On Tuesday, February 16, 2016 6:29:39 PM PST Ilia Mirkin wrote:
>> See commit 9db2098d which did it internally to the i965 driver. No
>> reason not to have this more globally set though.
>>
>> This fixes depth in a bunch of dEQP EXT_texture_border_clamp tests. And
>> probably other items as well.
>>
>> Signed-off-by: Ilia Mirkin 
>> Cc: Ian Romanick 
>> Cc: mesa-sta...@lists.freedesktop.org
>> ---
>>  src/mesa/main/texobj.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
>> index d8407f0..2b9c80a 100644
>> --- a/src/mesa/main/texobj.c
>> +++ b/src/mesa/main/texobj.c
>> @@ -320,7 +320,8 @@ _mesa_initialize_texture_object( struct gl_context *ctx,
>> obj->Sampler.MaxAnisotropy = 1.0;
>> obj->Sampler.CompareMode = GL_NONE; /* ARB_shadow */
>> obj->Sampler.CompareFunc = GL_LEQUAL;   /* ARB_shadow */
>> -   obj->DepthMode = ctx->API == API_OPENGL_CORE ? GL_RED : GL_LUMINANCE;
>> +   obj->DepthMode = (ctx->API == API_OPENGL_CORE || _mesa_is_gles3(ctx)) ?
>> +  GL_RED : GL_LUMINANCE;
>> obj->StencilSampling = false;
>> obj->Sampler.CubeMapSeamless = GL_FALSE;
>> obj->Swizzle[0] = GL_RED;
>>
> 
> Now I'm a bit weirded out - three years later I can't recall why I wrote
> an i965 specific patch for this.  Fixing it in core Mesa seems way
> better.  I wonder why I didn't do that in the first place.
> 
> I don't think this is quite right, though...won't this default depth
> mode to GL_RED for *all* formats?  The commit you and Ian cited explains
> that we should default to GL_RED (X, 0, 0, 1) for *sized* formats, but
> leave it as GL_LUMINANCE (X, X, X, 1) for the *unsized* ones.

I think we need to change DepthMode in GLES when the texture image is
set (by glTexImage or glTexStorage).  I think doing the mode swizzle in
the driver means glGetTexParameteriv(GL_DEPTH_MODE) will return the
wrong thing.

Defaulting to GL_LUMINANCE and possibly changing to GL_RED based on
whether or not a sized format is used for image creation should be the
right answer.

> We were sort of painted into a corner here...GLES 2 can be silently
> promoted to GLES 3...and GLES 2 already specified this as GL_LUMINANCE,
> but GLES 3 specified things as GL_RED...so there was a compromise.
> 
> Incidentally, we should figure out the GL 4.2 interaction I decided to
> put off in 2013, since we're finally there :)

Do you remember what that interaction was?

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




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

Re: [Mesa-dev] [PATCH] mesa: default DepthMode to GL_RED on ES 3.0

[Ilia Mirkin]

On Feb 17, 2016 2:14 AM, "Kenneth Graunke"  wrote:
>
> On Tuesday, February 16, 2016 6:29:39 PM PST Ilia Mirkin wrote:
> > See commit 9db2098d which did it internally to the i965 driver. No
> > reason not to have this more globally set though.
> >
> > This fixes depth in a bunch of dEQP EXT_texture_border_clamp tests. And
> > probably other items as well.
> >
> > Signed-off-by: Ilia Mirkin 
> > Cc: Ian Romanick 
> > Cc: mesa-sta...@lists.freedesktop.org
> > ---
> >  src/mesa/main/texobj.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
> > index d8407f0..2b9c80a 100644
> > --- a/src/mesa/main/texobj.c
> > +++ b/src/mesa/main/texobj.c
> > @@ -320,7 +320,8 @@ _mesa_initialize_texture_object( struct gl_context
*ctx,
> > obj->Sampler.MaxAnisotropy = 1.0;
> > obj->Sampler.CompareMode = GL_NONE; /* ARB_shadow */
> > obj->Sampler.CompareFunc = GL_LEQUAL;   /* ARB_shadow */
> > -   obj->DepthMode = ctx->API == API_OPENGL_CORE ? GL_RED :
GL_LUMINANCE;
> > +   obj->DepthMode = (ctx->API == API_OPENGL_CORE ||
_mesa_is_gles3(ctx)) ?
> > +  GL_RED : GL_LUMINANCE;
> > obj->StencilSampling = false;
> > obj->Sampler.CubeMapSeamless = GL_FALSE;
> > obj->Swizzle[0] = GL_RED;
> >
>
> Now I'm a bit weirded out - three years later I can't recall why I wrote
> an i965 specific patch for this.  Fixing it in core Mesa seems way
> better.  I wonder why I didn't do that in the first place.
>
> I don't think this is quite right, though...won't this default depth
> mode to GL_RED for *all* formats?  The commit you and Ian cited explains
> that we should default to GL_RED (X, 0, 0, 1) for *sized* formats, but
> leave it as GL_LUMINANCE (X, X, X, 1) for the *unsized* ones.

That'd be why you did it in the driver then, since you don't have the
format at texobj creation time. I missed that little detail, will redo with
the fixup in the st.

>
> We were sort of painted into a corner here...GLES 2 can be silently
> promoted to GLES 3...and GLES 2 already specified this as GL_LUMINANCE,
> but GLES 3 specified things as GL_RED...so there was a compromise.
>
> Incidentally, we should figure out the GL 4.2 interaction I decided to
> put off in 2013, since we're finally there :)

Not 100% sure what the interaction is, but probably beyond the scope of
this change :)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa: default DepthMode to GL_RED on ES 3.0

[Kenneth Graunke]

On Tuesday, February 16, 2016 6:29:39 PM PST Ilia Mirkin wrote:
> See commit 9db2098d which did it internally to the i965 driver. No
> reason not to have this more globally set though.
> 
> This fixes depth in a bunch of dEQP EXT_texture_border_clamp tests. And
> probably other items as well.
> 
> Signed-off-by: Ilia Mirkin 
> Cc: Ian Romanick 
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/mesa/main/texobj.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
> index d8407f0..2b9c80a 100644
> --- a/src/mesa/main/texobj.c
> +++ b/src/mesa/main/texobj.c
> @@ -320,7 +320,8 @@ _mesa_initialize_texture_object( struct gl_context *ctx,
> obj->Sampler.MaxAnisotropy = 1.0;
> obj->Sampler.CompareMode = GL_NONE; /* ARB_shadow */
> obj->Sampler.CompareFunc = GL_LEQUAL;   /* ARB_shadow */
> -   obj->DepthMode = ctx->API == API_OPENGL_CORE ? GL_RED : GL_LUMINANCE;
> +   obj->DepthMode = (ctx->API == API_OPENGL_CORE || _mesa_is_gles3(ctx)) ?
> +  GL_RED : GL_LUMINANCE;
> obj->StencilSampling = false;
> obj->Sampler.CubeMapSeamless = GL_FALSE;
> obj->Swizzle[0] = GL_RED;
> 

Now I'm a bit weirded out - three years later I can't recall why I wrote
an i965 specific patch for this.  Fixing it in core Mesa seems way
better.  I wonder why I didn't do that in the first place.

I don't think this is quite right, though...won't this default depth
mode to GL_RED for *all* formats?  The commit you and Ian cited explains
that we should default to GL_RED (X, 0, 0, 1) for *sized* formats, but
leave it as GL_LUMINANCE (X, X, X, 1) for the *unsized* ones.

We were sort of painted into a corner here...GLES 2 can be silently
promoted to GLES 3...and GLES 2 already specified this as GL_LUMINANCE,
but GLES 3 specified things as GL_RED...so there was a compromise.

Incidentally, we should figure out the GL 4.2 interaction I decided to
put off in 2013, since we're finally there :)


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

[Mesa-dev] [PATCH] mesa: default DepthMode to GL_RED on ES 3.0

[Ilia Mirkin]

See commit 9db2098d which did it internally to the i965 driver. No
reason not to have this more globally set though.

This fixes depth in a bunch of dEQP EXT_texture_border_clamp tests. And
probably other items as well.

Signed-off-by: Ilia Mirkin 
Cc: Ian Romanick 
Cc: mesa-sta...@lists.freedesktop.org
---
 src/mesa/main/texobj.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
index d8407f0..2b9c80a 100644
--- a/src/mesa/main/texobj.c
+++ b/src/mesa/main/texobj.c
@@ -320,7 +320,8 @@ _mesa_initialize_texture_object( struct gl_context *ctx,
obj->Sampler.MaxAnisotropy = 1.0;
obj->Sampler.CompareMode = GL_NONE; /* ARB_shadow */
obj->Sampler.CompareFunc = GL_LEQUAL;   /* ARB_shadow */
-   obj->DepthMode = ctx->API == API_OPENGL_CORE ? GL_RED : GL_LUMINANCE;
+   obj->DepthMode = (ctx->API == API_OPENGL_CORE || _mesa_is_gles3(ctx)) ?
+  GL_RED : GL_LUMINANCE;
obj->StencilSampling = false;
obj->Sampler.CubeMapSeamless = GL_FALSE;
obj->Swizzle[0] = GL_RED;
-- 
2.4.10

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