Re: [Mesa-dev] [PATCH 4/4] st/mesa: add OES_sample_variables support

[Ilia Mirkin]

On Tue, Feb 16, 2016 at 1:42 PM, Ian Romanick  wrote:
> On 02/16/2016 09:12 AM, Ilia Mirkin wrote:
>> On Tue, Feb 16, 2016 at 12:02 PM, Ian Romanick  wrote:
>>> On 02/15/2016 10:31 PM, Ilia Mirkin wrote:
 Basically the same thing as ARB_sample_shading except that it also needs
 gl_SampleMaskIn support as well as not enable per-sample interpolation
 whenever doing per-sample shading. This is done explicitly in another
 extension.

 Signed-off-by: Ilia Mirkin 
 ---

 I get 16 failures with dEQP tests, these fall into 2 categories:

  - 1-sample multisample surfaces don't behave the way it likes (it 
 considers
them non-multisampled even though they're created through 
 gl*Multisample*

  - gl_SampleMaskIn is reporting the whole pixel's worth of mask rather than
just the fragment in question. Looking back, it appears that
ARB_gpu_shader5 also wants it for only the current fragment.
>
> I think that's correct.  GL_ARB_sample_shading also says:
>
> If the fragment shader is being evaluated at
> any frequency other than per-framgent, bits of the sample mask not
> corresponding to the current fragment shader invocation are ignored.
>
  docs/GL3.txt| 2 +-
  src/mesa/state_tracker/st_atom_rasterizer.c | 2 ++
  src/mesa/state_tracker/st_atom_shader.c | 2 ++
  src/mesa/state_tracker/st_extensions.c  | 4 
  src/mesa/state_tracker/st_program.c | 5 -
  5 files changed, 13 insertions(+), 2 deletions(-)

 diff --git a/docs/GL3.txt b/docs/GL3.txt
 index 26847b9..ae439f6 100644
 --- a/docs/GL3.txt
 +++ b/docs/GL3.txt
 @@ -248,7 +248,7 @@ GLES3.2, GLSL ES 3.2
GL_OES_gpu_shader5   not started (based 
 on parts of GL_ARB_gpu_shader5, which is done for some drivers)
GL_OES_primitive_bounding boxnot started
GL_OES_sample_shadingnot started (based 
 on parts of GL_ARB_sample_shading, which is done for some drivers)
 -  GL_OES_sample_variables  not started (based 
 on parts of GL_ARB_sample_shading, which is done for some drivers)
 +  GL_OES_sample_variables  DONE (nvc0, r600, 
 radeonsi)
GL_OES_shader_image_atomic   not started (based 
 on parts of GL_ARB_shader_image_load_store, which is done for some drivers)
GL_OES_shader_io_blocks  not started (based 
 on parts of GLSL 1.50, which is done)
GL_OES_shader_multisample_interpolation  not started (based 
 on parts of GL_ARB_gpu_shader5, which is done)
 diff --git a/src/mesa/state_tracker/st_atom_rasterizer.c 
 b/src/mesa/state_tracker/st_atom_rasterizer.c
 index c20cadf..d42d512 100644
 --- a/src/mesa/state_tracker/st_atom_rasterizer.c
 +++ b/src/mesa/state_tracker/st_atom_rasterizer.c
 @@ -31,6 +31,7 @@
*/

  #include "main/macros.h"
 +#include "main/context.h"
  #include "st_context.h"
  #include "st_atom.h"
  #include "st_debug.h"
 @@ -239,6 +240,7 @@ static void update_raster_state( struct st_context *st 
 )

 /* _NEW_MULTISAMPLE | _NEW_BUFFERS */
 raster->force_persample_interp =
 + !_mesa_is_gles(ctx) &&
   !st->force_persample_in_shader &&
   ctx->Multisample._Enabled &&
   ctx->Multisample.SampleShading &&
>>>
>>> Is this change necessary?  I would have thought that
>>> ctx->Multisample.SampleShading couldn't get set without using features
>>> from OES_sample_shading.
>>
>> We don't want per-sample interp when sample shading, as far as I can
>> tell, based on the various OES text.
>
> With just OES_sample_shading, this is true.  Based on the existing
> expression, you can only get per-sample interpolation if
> ctx->Multisample.SampleShading is set.  That can only be set by
> glEnable(GL_SAMPLE_SHADING), and that is gated on 'if
> (!_mesa_is_desktop_gl(ctx)) goto invalid_enum_error;'  We'll need to
> change that when we add support for GL_OES_sample_shading. :)
>
> So... I'm pretty sure that it's already impossible for
> raster->force_persample_interp to get set to true in a GLES context.
> This hunk just adds some code that will have to be removed when we
> (you?) add GL_OES_sample_shading support.
>
 diff --git a/src/mesa/state_tracker/st_atom_shader.c 
 b/src/mesa/state_tracker/st_atom_shader.c
 index a88f035..8cfe756 100644
 --- a/src/mesa/state_tracker/st_atom_shader.c
 +++ b/src/mesa/state_tracker/st_atom_shader.c
 @@ -36,6 +36,7 @@
   */

  #include "main/imports.h"
 +#include "main/context.h"
  #include "main/mtypes.h"
  #include 

Re: [Mesa-dev] [PATCH 4/4] st/mesa: add OES_sample_variables support

[Ian Romanick]

On 02/16/2016 09:12 AM, Ilia Mirkin wrote:
> On Tue, Feb 16, 2016 at 12:02 PM, Ian Romanick  wrote:
>> On 02/15/2016 10:31 PM, Ilia Mirkin wrote:
>>> Basically the same thing as ARB_sample_shading except that it also needs
>>> gl_SampleMaskIn support as well as not enable per-sample interpolation
>>> whenever doing per-sample shading. This is done explicitly in another
>>> extension.
>>>
>>> Signed-off-by: Ilia Mirkin 
>>> ---
>>>
>>> I get 16 failures with dEQP tests, these fall into 2 categories:
>>>
>>>  - 1-sample multisample surfaces don't behave the way it likes (it considers
>>>them non-multisampled even though they're created through gl*Multisample*
>>>
>>>  - gl_SampleMaskIn is reporting the whole pixel's worth of mask rather than
>>>just the fragment in question. Looking back, it appears that
>>>ARB_gpu_shader5 also wants it for only the current fragment.

I think that's correct.  GL_ARB_sample_shading also says:

If the fragment shader is being evaluated at
any frequency other than per-framgent, bits of the sample mask not
corresponding to the current fragment shader invocation are ignored.

>>>  docs/GL3.txt| 2 +-
>>>  src/mesa/state_tracker/st_atom_rasterizer.c | 2 ++
>>>  src/mesa/state_tracker/st_atom_shader.c | 2 ++
>>>  src/mesa/state_tracker/st_extensions.c  | 4 
>>>  src/mesa/state_tracker/st_program.c | 5 -
>>>  5 files changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/docs/GL3.txt b/docs/GL3.txt
>>> index 26847b9..ae439f6 100644
>>> --- a/docs/GL3.txt
>>> +++ b/docs/GL3.txt
>>> @@ -248,7 +248,7 @@ GLES3.2, GLSL ES 3.2
>>>GL_OES_gpu_shader5   not started (based 
>>> on parts of GL_ARB_gpu_shader5, which is done for some drivers)
>>>GL_OES_primitive_bounding boxnot started
>>>GL_OES_sample_shadingnot started (based 
>>> on parts of GL_ARB_sample_shading, which is done for some drivers)
>>> -  GL_OES_sample_variables  not started (based 
>>> on parts of GL_ARB_sample_shading, which is done for some drivers)
>>> +  GL_OES_sample_variables  DONE (nvc0, r600, 
>>> radeonsi)
>>>GL_OES_shader_image_atomic   not started (based 
>>> on parts of GL_ARB_shader_image_load_store, which is done for some drivers)
>>>GL_OES_shader_io_blocks  not started (based 
>>> on parts of GLSL 1.50, which is done)
>>>GL_OES_shader_multisample_interpolation  not started (based 
>>> on parts of GL_ARB_gpu_shader5, which is done)
>>> diff --git a/src/mesa/state_tracker/st_atom_rasterizer.c 
>>> b/src/mesa/state_tracker/st_atom_rasterizer.c
>>> index c20cadf..d42d512 100644
>>> --- a/src/mesa/state_tracker/st_atom_rasterizer.c
>>> +++ b/src/mesa/state_tracker/st_atom_rasterizer.c
>>> @@ -31,6 +31,7 @@
>>>*/
>>>
>>>  #include "main/macros.h"
>>> +#include "main/context.h"
>>>  #include "st_context.h"
>>>  #include "st_atom.h"
>>>  #include "st_debug.h"
>>> @@ -239,6 +240,7 @@ static void update_raster_state( struct st_context *st )
>>>
>>> /* _NEW_MULTISAMPLE | _NEW_BUFFERS */
>>> raster->force_persample_interp =
>>> + !_mesa_is_gles(ctx) &&
>>>   !st->force_persample_in_shader &&
>>>   ctx->Multisample._Enabled &&
>>>   ctx->Multisample.SampleShading &&
>>
>> Is this change necessary?  I would have thought that
>> ctx->Multisample.SampleShading couldn't get set without using features
>> from OES_sample_shading.
> 
> We don't want per-sample interp when sample shading, as far as I can
> tell, based on the various OES text.

With just OES_sample_shading, this is true.  Based on the existing
expression, you can only get per-sample interpolation if
ctx->Multisample.SampleShading is set.  That can only be set by
glEnable(GL_SAMPLE_SHADING), and that is gated on 'if
(!_mesa_is_desktop_gl(ctx)) goto invalid_enum_error;'  We'll need to
change that when we add support for GL_OES_sample_shading. :)

So... I'm pretty sure that it's already impossible for
raster->force_persample_interp to get set to true in a GLES context.
This hunk just adds some code that will have to be removed when we
(you?) add GL_OES_sample_shading support.

>>> diff --git a/src/mesa/state_tracker/st_atom_shader.c 
>>> b/src/mesa/state_tracker/st_atom_shader.c
>>> index a88f035..8cfe756 100644
>>> --- a/src/mesa/state_tracker/st_atom_shader.c
>>> +++ b/src/mesa/state_tracker/st_atom_shader.c
>>> @@ -36,6 +36,7 @@
>>>   */
>>>
>>>  #include "main/imports.h"
>>> +#include "main/context.h"
>>>  #include "main/mtypes.h"
>>>  #include "program/program.h"
>>>
>>> @@ -76,6 +77,7 @@ update_fp( struct st_context *st )
>>>  * Ignore sample qualifier while computing this flag.
>>>  */
>>> key.persample_shading =
>>> +  !_mesa_is_gles(st->ctx) &&

Re: [Mesa-dev] [PATCH 4/4] st/mesa: add OES_sample_variables support

[Ilia Mirkin]

On Tue, Feb 16, 2016 at 12:02 PM, Ian Romanick  wrote:
> On 02/15/2016 10:31 PM, Ilia Mirkin wrote:
>> Basically the same thing as ARB_sample_shading except that it also needs
>> gl_SampleMaskIn support as well as not enable per-sample interpolation
>> whenever doing per-sample shading. This is done explicitly in another
>> extension.
>>
>> Signed-off-by: Ilia Mirkin 
>> ---
>>
>> I get 16 failures with dEQP tests, these fall into 2 categories:
>>
>>  - 1-sample multisample surfaces don't behave the way it likes (it considers
>>them non-multisampled even though they're created through gl*Multisample*
>>
>>  - gl_SampleMaskIn is reporting the whole pixel's worth of mask rather than
>>just the fragment in question. Looking back, it appears that
>>ARB_gpu_shader5 also wants it for only the current fragment.
>>
>>  docs/GL3.txt| 2 +-
>>  src/mesa/state_tracker/st_atom_rasterizer.c | 2 ++
>>  src/mesa/state_tracker/st_atom_shader.c | 2 ++
>>  src/mesa/state_tracker/st_extensions.c  | 4 
>>  src/mesa/state_tracker/st_program.c | 5 -
>>  5 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/GL3.txt b/docs/GL3.txt
>> index 26847b9..ae439f6 100644
>> --- a/docs/GL3.txt
>> +++ b/docs/GL3.txt
>> @@ -248,7 +248,7 @@ GLES3.2, GLSL ES 3.2
>>GL_OES_gpu_shader5   not started (based 
>> on parts of GL_ARB_gpu_shader5, which is done for some drivers)
>>GL_OES_primitive_bounding boxnot started
>>GL_OES_sample_shadingnot started (based 
>> on parts of GL_ARB_sample_shading, which is done for some drivers)
>> -  GL_OES_sample_variables  not started (based 
>> on parts of GL_ARB_sample_shading, which is done for some drivers)
>> +  GL_OES_sample_variables  DONE (nvc0, r600, 
>> radeonsi)
>>GL_OES_shader_image_atomic   not started (based 
>> on parts of GL_ARB_shader_image_load_store, which is done for some drivers)
>>GL_OES_shader_io_blocks  not started (based 
>> on parts of GLSL 1.50, which is done)
>>GL_OES_shader_multisample_interpolation  not started (based 
>> on parts of GL_ARB_gpu_shader5, which is done)
>> diff --git a/src/mesa/state_tracker/st_atom_rasterizer.c 
>> b/src/mesa/state_tracker/st_atom_rasterizer.c
>> index c20cadf..d42d512 100644
>> --- a/src/mesa/state_tracker/st_atom_rasterizer.c
>> +++ b/src/mesa/state_tracker/st_atom_rasterizer.c
>> @@ -31,6 +31,7 @@
>>*/
>>
>>  #include "main/macros.h"
>> +#include "main/context.h"
>>  #include "st_context.h"
>>  #include "st_atom.h"
>>  #include "st_debug.h"
>> @@ -239,6 +240,7 @@ static void update_raster_state( struct st_context *st )
>>
>> /* _NEW_MULTISAMPLE | _NEW_BUFFERS */
>> raster->force_persample_interp =
>> + !_mesa_is_gles(ctx) &&
>>   !st->force_persample_in_shader &&
>>   ctx->Multisample._Enabled &&
>>   ctx->Multisample.SampleShading &&
>
> Is this change necessary?  I would have thought that
> ctx->Multisample.SampleShading couldn't get set without using features
> from OES_sample_shading.

We don't want per-sample interp when sample shading, as far as I can
tell, based on the various OES text.

>
>> diff --git a/src/mesa/state_tracker/st_atom_shader.c 
>> b/src/mesa/state_tracker/st_atom_shader.c
>> index a88f035..8cfe756 100644
>> --- a/src/mesa/state_tracker/st_atom_shader.c
>> +++ b/src/mesa/state_tracker/st_atom_shader.c
>> @@ -36,6 +36,7 @@
>>   */
>>
>>  #include "main/imports.h"
>> +#include "main/context.h"
>>  #include "main/mtypes.h"
>>  #include "program/program.h"
>>
>> @@ -76,6 +77,7 @@ update_fp( struct st_context *st )
>>  * Ignore sample qualifier while computing this flag.
>>  */
>> key.persample_shading =
>> +  !_mesa_is_gles(st->ctx) &&
>>st->force_persample_in_shader &&
>>!(stfp->Base.Base.SystemValuesRead & (SYSTEM_BIT_SAMPLE_ID |
>>  SYSTEM_BIT_SAMPLE_POS)) &&
>
> Is this correct? While not normative, the overview section of the OES
> extension does say
>
> This means that where these features are used (gl_SampleID and
> gl_SamplePosition), implementations must run the fragment shader
> for each sample.
>
> I haven't dug into the body of the spec to find supporting text there.

'Correct' is a strong word to use. Not sure. But I think so :) Based
on my reading of the various OES texts (and note that I haven't dug
into the EXT or NV/whatever ones), GL ES does away with the whole
auto-interpolate-to-sample when sample shading. Basically the point of
this persample_shading shader key is to force the interpolation
position to be 'sample' for drivers that can't auto-do it via the
rasterizer setting (above).

>
>> diff --git 

Re: [Mesa-dev] [PATCH 4/4] st/mesa: add OES_sample_variables support

[Ian Romanick]

On 02/15/2016 10:31 PM, Ilia Mirkin wrote:
> Basically the same thing as ARB_sample_shading except that it also needs
> gl_SampleMaskIn support as well as not enable per-sample interpolation
> whenever doing per-sample shading. This is done explicitly in another
> extension.
> 
> Signed-off-by: Ilia Mirkin 
> ---
> 
> I get 16 failures with dEQP tests, these fall into 2 categories:
> 
>  - 1-sample multisample surfaces don't behave the way it likes (it considers
>them non-multisampled even though they're created through gl*Multisample*
> 
>  - gl_SampleMaskIn is reporting the whole pixel's worth of mask rather than
>just the fragment in question. Looking back, it appears that
>ARB_gpu_shader5 also wants it for only the current fragment.
> 
>  docs/GL3.txt| 2 +-
>  src/mesa/state_tracker/st_atom_rasterizer.c | 2 ++
>  src/mesa/state_tracker/st_atom_shader.c | 2 ++
>  src/mesa/state_tracker/st_extensions.c  | 4 
>  src/mesa/state_tracker/st_program.c | 5 -
>  5 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/GL3.txt b/docs/GL3.txt
> index 26847b9..ae439f6 100644
> --- a/docs/GL3.txt
> +++ b/docs/GL3.txt
> @@ -248,7 +248,7 @@ GLES3.2, GLSL ES 3.2
>GL_OES_gpu_shader5   not started (based on 
> parts of GL_ARB_gpu_shader5, which is done for some drivers)
>GL_OES_primitive_bounding boxnot started
>GL_OES_sample_shadingnot started (based on 
> parts of GL_ARB_sample_shading, which is done for some drivers)
> -  GL_OES_sample_variables  not started (based on 
> parts of GL_ARB_sample_shading, which is done for some drivers)
> +  GL_OES_sample_variables  DONE (nvc0, r600, 
> radeonsi)
>GL_OES_shader_image_atomic   not started (based on 
> parts of GL_ARB_shader_image_load_store, which is done for some drivers)
>GL_OES_shader_io_blocks  not started (based on 
> parts of GLSL 1.50, which is done)
>GL_OES_shader_multisample_interpolation  not started (based on 
> parts of GL_ARB_gpu_shader5, which is done)
> diff --git a/src/mesa/state_tracker/st_atom_rasterizer.c 
> b/src/mesa/state_tracker/st_atom_rasterizer.c
> index c20cadf..d42d512 100644
> --- a/src/mesa/state_tracker/st_atom_rasterizer.c
> +++ b/src/mesa/state_tracker/st_atom_rasterizer.c
> @@ -31,6 +31,7 @@
>*/
>   
>  #include "main/macros.h"
> +#include "main/context.h"
>  #include "st_context.h"
>  #include "st_atom.h"
>  #include "st_debug.h"
> @@ -239,6 +240,7 @@ static void update_raster_state( struct st_context *st )
>  
> /* _NEW_MULTISAMPLE | _NEW_BUFFERS */
> raster->force_persample_interp =
> + !_mesa_is_gles(ctx) &&
>   !st->force_persample_in_shader &&
>   ctx->Multisample._Enabled &&
>   ctx->Multisample.SampleShading &&

Is this change necessary?  I would have thought that
ctx->Multisample.SampleShading couldn't get set without using features
from OES_sample_shading.

> diff --git a/src/mesa/state_tracker/st_atom_shader.c 
> b/src/mesa/state_tracker/st_atom_shader.c
> index a88f035..8cfe756 100644
> --- a/src/mesa/state_tracker/st_atom_shader.c
> +++ b/src/mesa/state_tracker/st_atom_shader.c
> @@ -36,6 +36,7 @@
>   */
>  
>  #include "main/imports.h"
> +#include "main/context.h"
>  #include "main/mtypes.h"
>  #include "program/program.h"
>  
> @@ -76,6 +77,7 @@ update_fp( struct st_context *st )
>  * Ignore sample qualifier while computing this flag.
>  */
> key.persample_shading =
> +  !_mesa_is_gles(st->ctx) &&
>st->force_persample_in_shader &&
>!(stfp->Base.Base.SystemValuesRead & (SYSTEM_BIT_SAMPLE_ID |
>  SYSTEM_BIT_SAMPLE_POS)) &&

Is this correct? While not normative, the overview section of the OES
extension does say

This means that where these features are used (gl_SampleID and
gl_SamplePosition), implementations must run the fragment shader
for each sample.

I haven't dug into the body of the spec to find supporting text there.

> diff --git a/src/mesa/state_tracker/st_extensions.c 
> b/src/mesa/state_tracker/st_extensions.c
> index eff3a2d..49d5a2c 100644
> --- a/src/mesa/state_tracker/st_extensions.c
> +++ b/src/mesa/state_tracker/st_extensions.c
> @@ -861,6 +861,10 @@ void st_init_extensions(struct pipe_screen *screen,
>extensions->OES_copy_image = GL_TRUE;
> }
>  
> +   /* Needs PIPE_CAP_SAMPLE_SHADING + gl_SampleMaskIn.
> +*/
> +   extensions->OES_sample_variables = extensions->ARB_gpu_shader5;
> +

Just based on the comment... shouldn't this be
extensions->ARB_sample_shading && extensions->ARB_gpu_shader5?

Does any Gallium driver support PIPE_CAP_SAMPLE_SHADING and not support
gl_SampleMaskIn?  I wonder if it would be better to extend the 

[Mesa-dev] [PATCH 4/4] st/mesa: add OES_sample_variables support

[Ilia Mirkin]

Basically the same thing as ARB_sample_shading except that it also needs
gl_SampleMaskIn support as well as not enable per-sample interpolation
whenever doing per-sample shading. This is done explicitly in another
extension.

Signed-off-by: Ilia Mirkin 
---

I get 16 failures with dEQP tests, these fall into 2 categories:

 - 1-sample multisample surfaces don't behave the way it likes (it considers
   them non-multisampled even though they're created through gl*Multisample*

 - gl_SampleMaskIn is reporting the whole pixel's worth of mask rather than
   just the fragment in question. Looking back, it appears that
   ARB_gpu_shader5 also wants it for only the current fragment.

 docs/GL3.txt| 2 +-
 src/mesa/state_tracker/st_atom_rasterizer.c | 2 ++
 src/mesa/state_tracker/st_atom_shader.c | 2 ++
 src/mesa/state_tracker/st_extensions.c  | 4 
 src/mesa/state_tracker/st_program.c | 5 -
 5 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/docs/GL3.txt b/docs/GL3.txt
index 26847b9..ae439f6 100644
--- a/docs/GL3.txt
+++ b/docs/GL3.txt
@@ -248,7 +248,7 @@ GLES3.2, GLSL ES 3.2
   GL_OES_gpu_shader5   not started (based on 
parts of GL_ARB_gpu_shader5, which is done for some drivers)
   GL_OES_primitive_bounding boxnot started
   GL_OES_sample_shadingnot started (based on 
parts of GL_ARB_sample_shading, which is done for some drivers)
-  GL_OES_sample_variables  not started (based on 
parts of GL_ARB_sample_shading, which is done for some drivers)
+  GL_OES_sample_variables  DONE (nvc0, r600, 
radeonsi)
   GL_OES_shader_image_atomic   not started (based on 
parts of GL_ARB_shader_image_load_store, which is done for some drivers)
   GL_OES_shader_io_blocks  not started (based on 
parts of GLSL 1.50, which is done)
   GL_OES_shader_multisample_interpolation  not started (based on 
parts of GL_ARB_gpu_shader5, which is done)
diff --git a/src/mesa/state_tracker/st_atom_rasterizer.c 
b/src/mesa/state_tracker/st_atom_rasterizer.c
index c20cadf..d42d512 100644
--- a/src/mesa/state_tracker/st_atom_rasterizer.c
+++ b/src/mesa/state_tracker/st_atom_rasterizer.c
@@ -31,6 +31,7 @@
   */
  
 #include "main/macros.h"
+#include "main/context.h"
 #include "st_context.h"
 #include "st_atom.h"
 #include "st_debug.h"
@@ -239,6 +240,7 @@ static void update_raster_state( struct st_context *st )
 
/* _NEW_MULTISAMPLE | _NEW_BUFFERS */
raster->force_persample_interp =
+ !_mesa_is_gles(ctx) &&
  !st->force_persample_in_shader &&
  ctx->Multisample._Enabled &&
  ctx->Multisample.SampleShading &&
diff --git a/src/mesa/state_tracker/st_atom_shader.c 
b/src/mesa/state_tracker/st_atom_shader.c
index a88f035..8cfe756 100644
--- a/src/mesa/state_tracker/st_atom_shader.c
+++ b/src/mesa/state_tracker/st_atom_shader.c
@@ -36,6 +36,7 @@
  */
 
 #include "main/imports.h"
+#include "main/context.h"
 #include "main/mtypes.h"
 #include "program/program.h"
 
@@ -76,6 +77,7 @@ update_fp( struct st_context *st )
 * Ignore sample qualifier while computing this flag.
 */
key.persample_shading =
+  !_mesa_is_gles(st->ctx) &&
   st->force_persample_in_shader &&
   !(stfp->Base.Base.SystemValuesRead & (SYSTEM_BIT_SAMPLE_ID |
 SYSTEM_BIT_SAMPLE_POS)) &&
diff --git a/src/mesa/state_tracker/st_extensions.c 
b/src/mesa/state_tracker/st_extensions.c
index eff3a2d..49d5a2c 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -861,6 +861,10 @@ void st_init_extensions(struct pipe_screen *screen,
   extensions->OES_copy_image = GL_TRUE;
}
 
+   /* Needs PIPE_CAP_SAMPLE_SHADING + gl_SampleMaskIn.
+*/
+   extensions->OES_sample_variables = extensions->ARB_gpu_shader5;
+
/* Maximum sample count. */
{
   enum pipe_format color_formats[] = {
diff --git a/src/mesa/state_tracker/st_program.c 
b/src/mesa/state_tracker/st_program.c
index 2e21d02..de628d7 100644
--- a/src/mesa/state_tracker/st_program.c
+++ b/src/mesa/state_tracker/st_program.c
@@ -32,6 +32,7 @@
 
 
 #include "main/imports.h"
+#include "main/context.h"
 #include "main/hash.h"
 #include "main/mtypes.h"
 #include "program/prog_parameter.h"
@@ -573,7 +574,9 @@ st_translate_fragment_program(struct st_context *st,
  else
 interpLocation[slot] = TGSI_INTERPOLATE_LOC_CENTER;
 
- if (stfp->Base.Base.SystemValuesRead & (SYSTEM_BIT_SAMPLE_ID |
+ /* GLES specifies that only the sample keyword alters interpolation */
+ if (!_mesa_is_gles(st->ctx) &&
+ stfp->Base.Base.SystemValuesRead & (SYSTEM_BIT_SAMPLE_ID |
  SYSTEM_BIT_SAMPLE_POS))