[Mesa-dev] [PATCH] i915: fallback when point sprite is enabled while handling varying inputs

[Yuanhan Liu]

On Thu, Mar 08, 2012 at 02:30:30PM +0800, Yuanhan Liu wrote:
> The current code would use tex coord to implement varying inputs. If
> point sprite is enabled(always enabled in chrome and firefox), the tex
> coord would be replaced with the value (x, y, 0, 1) where x and y vary
> from 0 to 1. Thus you will find that the value of the varying inputs
> doesn't work anymore.
> 
> Why chrome(and firefox) would always enable GL_POINT_SPRITE to enable
> webglc, if you would ask, here is the answer I find from the code of
> chrome at file gpu/command_buffer/service/gles2_cmd_decoder.cc:
> 
>   // OpenGL ES 2.0 implicitly enables the desktop GL capability
>   // VERTEX_PROGRAM_POINT_SIZE and doesn't expose this enum. This fact
>   // isn't well documented; it was discovered in the Khronos OpenGL ES
>   // mailing list archives. It also implicitly enables the desktop GL
>   // capability GL_POINT_SPRITE to provide access to the gl_PointCoord
>   // variable in fragment shaders.
>   if (gfx::GetGLImplementation() != gfx::kGLImplementationEGLGLES2) {
>     glEnable(GL_VERTEX_PROGRAM_POINT_SIZE);
>     glEnable(GL_POINT_SPRITE);
>   }
> 
> So, fallback when point sprite is enabled while handling varying inputs
> before finding a better way to not use tex coord to implement varying
> inputs.
> 
> This would _really_ fix the following webglc case on pineview this time:
> https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/conformance-suites/1.0.1/conformance/rendering/point-size.html
> 
> NOTE: This is a candidate for stable release branches.
> 
> Signed-off-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>
> ---
>  src/mesa/drivers/dri/i915/i915_fragprog.c |   23 ++++++++++++++++++++---
>  1 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i915/i915_fragprog.c b/src/mesa/drivers/dri/i915/i915_fragprog.c
> index 5b7e93e..c2390fe 100644
> --- a/src/mesa/drivers/dri/i915/i915_fragprog.c
> +++ b/src/mesa/drivers/dri/i915/i915_fragprog.c
> @@ -132,9 +132,26 @@ src_vector(struct i915_fragment_program *p,
>        case FRAG_ATTRIB_VAR0 + 5:
>        case FRAG_ATTRIB_VAR0 + 6:
>        case FRAG_ATTRIB_VAR0 + 7:
> -         src = i915_emit_decl(p, REG_TYPE_T,
> -                              T_TEX0 + (source->Index - FRAG_ATTRIB_VAR0),
> -                              D0_CHANNEL_ALL);
> +         /*
> +          * The current code would use tex coord to implement varying inputs.
> +          * If point sprite is enabled(always enabled in chrome and firefox),
> +          * the tex coord would be replaced with the value (x, y, 0, 1) where
> +          * x and y vary from 0 to 1. Thus you will find that the value of the
> +          * varying inputs doesn't work anymore.
> +          *
> +          * So, fallback when point sprite is enabled.
> +          *
> +          * FIXME: a better way to not use tex coord to add the support of
> +          *        varying inputs?
> +          */
> +         if (p->ctx->Point.PointSprite) {
> +             i915_program_error(p, "Point Sprite is enabled while using "
> +                                   "tex coord to implement varying inputs");
> +         } else {
> +             src = i915_emit_decl(p, REG_TYPE_T,
> +                                  T_TEX0 + (source->Index - FRAG_ATTRIB_VAR0),
> +                                  D0_CHANNEL_ALL);
> +         }


Honestly, I don't like this patch myself: it makes lots of webglc test
case fallback to swrast. Though I somehow understand why you use tex
coord to implement varying inputs. But I still don't figure a better
way to handle varying inputs. 

Then I tried to get rid of such fallback and came the following patch,
which I think is much better than this one.

----