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 <imir...@alum.mit.edu> > --- > > 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 box not started > GL_OES_sample_shading not 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 meaning of the cap bit to include gl_SampleMaskIn. Then, if the other fixes from this patch get moved before patch 2, we could just have GL_OES_sample_variables extension string depend on the ARB_gpu_shader5 flag. Let me test it on our CI first... :) > /* 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)) > interpLocation[slot] = TGSI_INTERPOLATE_LOC_SAMPLE; > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev