[Mesa-dev,01/11] i965: Fix ctx->Texture.CubeMapSeamless

Submitted by Eduardo Lima Mitev on Feb. 10, 2015, 3:40 p.m.

Details

Message ID 1423582848-18526-2-git-send-email-elima@igalia.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Eduardo Lima Mitev Feb. 10, 2015, 3:40 p.m.
From: Iago Toral Quiroga <itoral@igalia.com>

The intel driver code, and apparently all other Mesa drivers, call
_mesa_initialize_context early in the CreateContext hook. That
function will end up calling _mesa_init_texture which will do:

ctx->Texture.CubeMapSeamless = _mesa_is_gles3(ctx);

But this won't work at this point, since _mesa_is_gles3 requires
ctx->Version to be set and that will not happen until late
in the CreateContext hook, when _mesa_compute_version is called.

We can't just move the call to _mesa_compute_version before
_mesa_initialize_context since it needs that available extensions
have been computed, which again requires other things to be
initialized, etc. Instead, we just re-compute
ctx->Texture.CubeMapSeamless after the version is known.

As far as I can see, this should affect all Mesa drivers, as it
seems that all call _mesa_initialize_context before _mesa_compute_version.
Separate patches can fix the problem for other drivers.

Fixes the following 190 dEQP tests:
dEQP-GLES3.functional.texture.filtering.cube.formats.*
dEQP-GLES3.functional.texture.filtering.cube.sizes.*
dEQP-GLES3.functional.texture.filtering.cube.combinations.*
dEQP-GLES3.functional.texture.mipmap.cube.*
dEQP-GLES3.functional.texture.vertex.cube.filtering.*
dEQP-GLES3.functional.texture.vertex.cube.wrap.*
---
 src/mesa/drivers/dri/i965/brw_context.c | 6 ++++++
 src/mesa/main/texstate.c                | 8 ++++++++
 2 files changed, 14 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
index e20da0b..f5b0624 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -859,6 +859,12 @@  brwCreateContext(gl_api api,
 
    _mesa_compute_version(ctx);
 
+   /* This is done in _mesa_init_texture called from _mesa_initialize_context
+    * above, but it won't work until we have computed the GL version, so
+    * do it again here.
+    */
+   ctx->Texture.CubeMapSeamless = _mesa_is_gles3(ctx);
+
    _mesa_initialize_dispatch_tables(ctx);
    _mesa_initialize_vbo_vtxfmt(ctx);
 
diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
index 66fd718..f03862e 100644
--- a/src/mesa/main/texstate.c
+++ b/src/mesa/main/texstate.c
@@ -882,6 +882,14 @@  _mesa_init_texture(struct gl_context *ctx)
     *     "OpenGL ES 3.0 requires that all cube map filtering be
     *     seamless. OpenGL ES 2.0 specified that a single cube map face be
     *     selected and used for filtering."
+    *
+    * FIXME: the call to _mesa_is_gles3 below will only work as intended if
+    * the driver has already computed and set ctx->Version, however drivers
+    * seem to call _mesa_initialize_context (which calls this) early
+    * in the CreateContext hook and _mesa_compute_version much later (since
+    * it needs information about available extensions), which means that this
+    * won't actually work unless we figure out a way to compute the GL version
+    * before _mesa_initialize_context is called.
     */
    ctx->Texture.CubeMapSeamless = _mesa_is_gles3(ctx);
 

Comments

On 02/10/2015 07:40 AM, Eduardo Lima Mitev wrote:
> From: Iago Toral Quiroga <itoral@igalia.com>
> 
> The intel driver code, and apparently all other Mesa drivers, call
> _mesa_initialize_context early in the CreateContext hook. That
> function will end up calling _mesa_init_texture which will do:
> 
> ctx->Texture.CubeMapSeamless = _mesa_is_gles3(ctx);
> 
> But this won't work at this point, since _mesa_is_gles3 requires
> ctx->Version to be set and that will not happen until late
> in the CreateContext hook, when _mesa_compute_version is called.
> 
> We can't just move the call to _mesa_compute_version before
> _mesa_initialize_context since it needs that available extensions
> have been computed, which again requires other things to be
> initialized, etc. Instead, we just re-compute
> ctx->Texture.CubeMapSeamless after the version is known.
> 
> As far as I can see, this should affect all Mesa drivers, as it
> seems that all call _mesa_initialize_context before _mesa_compute_version.
> Separate patches can fix the problem for other drivers.
> 
> Fixes the following 190 dEQP tests:
> dEQP-GLES3.functional.texture.filtering.cube.formats.*
> dEQP-GLES3.functional.texture.filtering.cube.sizes.*
> dEQP-GLES3.functional.texture.filtering.cube.combinations.*
> dEQP-GLES3.functional.texture.mipmap.cube.*
> dEQP-GLES3.functional.texture.vertex.cube.filtering.*
> dEQP-GLES3.functional.texture.vertex.cube.wrap.*
> ---
>  src/mesa/drivers/dri/i965/brw_context.c | 6 ++++++
>  src/mesa/main/texstate.c                | 8 ++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index e20da0b..f5b0624 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -859,6 +859,12 @@ brwCreateContext(gl_api api,
>  
>     _mesa_compute_version(ctx);
>  
> +   /* This is done in _mesa_init_texture called from _mesa_initialize_context
> +    * above, but it won't work until we have computed the GL version, so
> +    * do it again here.
> +    */
> +   ctx->Texture.CubeMapSeamless = _mesa_is_gles3(ctx);
> +
>     _mesa_initialize_dispatch_tables(ctx);
>     _mesa_initialize_vbo_vtxfmt(ctx);
>  
> diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
> index 66fd718..f03862e 100644
> --- a/src/mesa/main/texstate.c
> +++ b/src/mesa/main/texstate.c
> @@ -882,6 +882,14 @@ _mesa_init_texture(struct gl_context *ctx)
>      *     "OpenGL ES 3.0 requires that all cube map filtering be
>      *     seamless. OpenGL ES 2.0 specified that a single cube map face be
>      *     selected and used for filtering."
> +    *
> +    * FIXME: the call to _mesa_is_gles3 below will only work as intended if
> +    * the driver has already computed and set ctx->Version, however drivers
> +    * seem to call _mesa_initialize_context (which calls this) early
> +    * in the CreateContext hook and _mesa_compute_version much later (since
> +    * it needs information about available extensions), which means that this
> +    * won't actually work unless we figure out a way to compute the GL version
> +    * before _mesa_initialize_context is called.
>      */
>     ctx->Texture.CubeMapSeamless = _mesa_is_gles3(ctx);

We at least know the API at this point, right?  My recollection is that
ES2 is a imprecise in the sort of cubemap filtering you get.  Further, I
believe that some implementations have always given seamless filtering
in ES2.  It might be better to change this to

   ctx->Texture.CubeMapSeamless = ctx->API == API_OPENGLES2;

This could cause problems for drivers that can't do seamless, I suppose...
On Tue, 2015-02-10 at 09:33 -0800, Ian Romanick wrote:
> On 02/10/2015 07:40 AM, Eduardo Lima Mitev wrote:
> > From: Iago Toral Quiroga <itoral@igalia.com>
> > 
> > The intel driver code, and apparently all other Mesa drivers, call
> > _mesa_initialize_context early in the CreateContext hook. That
> > function will end up calling _mesa_init_texture which will do:
> > 
> > ctx->Texture.CubeMapSeamless = _mesa_is_gles3(ctx);
> > 
> > But this won't work at this point, since _mesa_is_gles3 requires
> > ctx->Version to be set and that will not happen until late
> > in the CreateContext hook, when _mesa_compute_version is called.
> > 
> > We can't just move the call to _mesa_compute_version before
> > _mesa_initialize_context since it needs that available extensions
> > have been computed, which again requires other things to be
> > initialized, etc. Instead, we just re-compute
> > ctx->Texture.CubeMapSeamless after the version is known.
> > 
> > As far as I can see, this should affect all Mesa drivers, as it
> > seems that all call _mesa_initialize_context before _mesa_compute_version.
> > Separate patches can fix the problem for other drivers.
> > 
> > Fixes the following 190 dEQP tests:
> > dEQP-GLES3.functional.texture.filtering.cube.formats.*
> > dEQP-GLES3.functional.texture.filtering.cube.sizes.*
> > dEQP-GLES3.functional.texture.filtering.cube.combinations.*
> > dEQP-GLES3.functional.texture.mipmap.cube.*
> > dEQP-GLES3.functional.texture.vertex.cube.filtering.*
> > dEQP-GLES3.functional.texture.vertex.cube.wrap.*
> > ---
> >  src/mesa/drivers/dri/i965/brw_context.c | 6 ++++++
> >  src/mesa/main/texstate.c                | 8 ++++++++
> >  2 files changed, 14 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> > index e20da0b..f5b0624 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.c
> > +++ b/src/mesa/drivers/dri/i965/brw_context.c
> > @@ -859,6 +859,12 @@ brwCreateContext(gl_api api,
> >  
> >     _mesa_compute_version(ctx);
> >  
> > +   /* This is done in _mesa_init_texture called from _mesa_initialize_context
> > +    * above, but it won't work until we have computed the GL version, so
> > +    * do it again here.
> > +    */
> > +   ctx->Texture.CubeMapSeamless = _mesa_is_gles3(ctx);
> > +
> >     _mesa_initialize_dispatch_tables(ctx);
> >     _mesa_initialize_vbo_vtxfmt(ctx);
> >  
> > diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
> > index 66fd718..f03862e 100644
> > --- a/src/mesa/main/texstate.c
> > +++ b/src/mesa/main/texstate.c
> > @@ -882,6 +882,14 @@ _mesa_init_texture(struct gl_context *ctx)
> >      *     "OpenGL ES 3.0 requires that all cube map filtering be
> >      *     seamless. OpenGL ES 2.0 specified that a single cube map face be
> >      *     selected and used for filtering."
> > +    *
> > +    * FIXME: the call to _mesa_is_gles3 below will only work as intended if
> > +    * the driver has already computed and set ctx->Version, however drivers
> > +    * seem to call _mesa_initialize_context (which calls this) early
> > +    * in the CreateContext hook and _mesa_compute_version much later (since
> > +    * it needs information about available extensions), which means that this
> > +    * won't actually work unless we figure out a way to compute the GL version
> > +    * before _mesa_initialize_context is called.
> >      */
> >     ctx->Texture.CubeMapSeamless = _mesa_is_gles3(ctx);
> 
> We at least know the API at this point, right?  My recollection is that
> ES2 is a imprecise in the sort of cubemap filtering you get.  Further, I
> believe that some implementations have always given seamless filtering
> in ES2.  It might be better to change this to
> 
>    ctx->Texture.CubeMapSeamless = ctx->API == API_OPENGLES2;
> 
> This could cause problems for drivers that can't do seamless, I suppose...

These drivers can still force ctx->Texture.CubeMapSeamless = false in
their context initialization routine I guess... If nobody opposes to the
change I'll update the patch to follow your suggestion.

Iago

> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev