[Mesa-dev] [PATCH] mesa/main: TexImage2DMultisample needs to pass OpenGL3.3 conformance test.
Timothy Arceri
t_arceri at yahoo.com.au
Thu Nov 26 13:31:59 PST 2015
On Thu, 2015-11-26 at 11:00 +0000, Predut, Marius wrote:
> > -----Original Message-----
> > From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
> > Behalf Of
> > Timothy Arceri
> > Sent: Wednesday, November 25, 2015 6:02 PM
> > To: Palli, Tapani; Predut, Marius; mesa-dev at lists.freedesktop.org
> > Subject: Re: [Mesa-dev] [PATCH] mesa/main: TexImage2DMultisample
> > needs to pass
> > OpenGL3.3 conformance test.
> >
> > On Wed, 2015-11-25 at 17:13 +0200, Tapani Pälli wrote:
> > > On 11/25/2015 04:00 PM, Predut, Marius wrote:
> > > > > -----Original Message-----
> > > > > From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org
> > > > > ] On
> > > > > Behalf Of Timothy Arceri
> > > > > Sent: Wednesday, November 25, 2015 1:12 PM
> > > > > To: Palli, Tapani; Predut, Marius;
> > > > > mesa-dev at lists.freedesktop.org
> > > > > Subject: Re: [Mesa-dev] [PATCH] mesa/main:
> > > > > TexImage2DMultisample
> > > > > needs to pass
> > > > > OpenGL3.3 conformance test.
> > > > >
> > > > > On Wed, 2015-11-25 at 12:47 +0200, Tapani Pälli wrote:
> > > > > > Hi;
> > > > > >
> > > > > > On 11/25/2015 01:15 PM, Marius Predut wrote:
> > > > > > > Open GL 3.3 reference document says:
> > > > > > > samples must be in the range zero to GL_MAX_TEXTURE_SIZE
> > > > > > > - 1.
> > > > > > > Open GL.4 clearly states:
> > > > > > > An INVALID_VALUE error is generated if samples is zero.
> > > > > See my comment in bugzilla [1] I believe this is just a bug
> > > > > in the
> > > > > reference pages, we implement things in Mesa going by what
> > > > > the
> > > > > spec says and the spec says nothing about samples being 0 in
> > > > > the
> > > > > 3.2 spec in fact it doen't even say anything in the 4.0 spec
> > > > > which
> > > > > you have changed the check to.
> > > > >
> > > > > Also the 4.5 reference pages also conflict with the spec so
> > > > > this
> > > > > is even more reason I think this change is wrong.
> > > > >
> > > > > [1] https://bugs.freedesktop.org/show_bug.cgi?id=91670
> > > > >
> > > > I don't think it is a bug in specs because in this case also
> > > > the CTS
> > > > and the piglit test is wrong:
> > > >
> > > > With this patch 2 things are fixed:
> > > > 1.Khronos CTS conformance tests for OpenGL 3.3 2. The piglit
> > > > test
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=93100 (Or
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=91670)
> > > >
> > > > The patch is based on this spec:
> > > > https://www.opengl.org/sdk/docs/man3/xhtml/glTexImage3DMultisam
> > > > ple.
> > > > xml
> > > >
> > > > We can't believe or "suppose" something here, the specs need to
> > > > be
> > > > as an axioma.
> > > > Can someone confirm that this reference includes the wrong
> > > > specs?
> > >
> > > That's not a specification but a manual page. Timothy is pointing
> > > to
> > > OpenGL specifications (available at www.opengl.org/registry). It
> > > is
> > > true that for example 3.3 Core does not mention this error case
> > > which
> > > means using 0 was allowed there. IMO either we should allow to
> > > use 0
> > > (and bump it to 1?) when running on 3.x context since it's not
> > > forbidden or maybe just locally patch this whenever running 3.x
> > > conformance.
> >
> > Hi Marius,
> >
> > Please provide the test CTS test that you are trying to fix so that
> > we can
> > take a look at what the CTS is trying to test.
> >
> > While the OpenGL 3.2 -> 4.2 specs seem to allow zero due simple to
> > not
> > mentioning it seems odd to me that the CTS would specificly test
> > for this. As
> > far as I understand it a value of 0 would result in undefined
> > behaviour, so it
> > doesn't seem right to allow this for these versions of OpenGL. IMO
> > we should
> > be going with the later specs that fix this oversight for all
> > versions of GL.
> >
> > One thing I can see the CTS doing in various tests is querying the
> > GL
> > implementation for values, its possible we are passing a value of 0
> > back to
> > the tests somewhere and its trying to used this with the
> > multisample
> > functions. The only way to know whats going on is if you tell us
> > which test
> > you are trying to fix.
> >
> > Tim
>
>
> Hi Tim
>
> On my case, in file otc_gen_graphics-khronos
> -glconform/framework/opengl/gluStateReset.cpp
> I have something like :
>
> if (type >= CONTEXTTYPE_GL32_CORE)
> {
> // Reset 2D multisample texture.
> gl.bindTexture
> (GL_TEXTURE_2D_MULTISAMPLE, 0);
> gl.texImage2DMultisample
> (GL_TEXTURE_2D_MULTISAMPLE, 0, GL_RGBA8, 0, 0, GL_TRUE);
> // Reset 2D multisample array texture.
> gl.bindTexture
> (GL_TEXTURE_2D_MULTISAMPLE_ARRAY, 0);
> gl.texImage3DMultisample
> (GL_TEXTURE_2D_MULTISAMPLE_ARRAY, 0, GL_RGBA8, 0, 0, 0, GL_TRUE);
> }
Hi Marius,
It may be time for an update of your git repo mine is up to date and
this file has the following for me.
// Reset 2D multisample texture.
gl.bindTexture (GL_TEXTURE_2D_MULTISAMPLE,
0);
gl.texImage2DMultisample (GL_TEXTURE_2D_MULTISAMPLE,
1, GL_RGBA8, 0, 0, GL_TRUE);
// Reset 2D multisample array texture.
gl.bindTexture
(GL_TEXTURE_2D_MULTISAMPLE_ARRAY, 0);
gl.texImage3DMultisample
(GL_TEXTURE_2D_MULTISAMPLE_ARRAY, 1, GL_RGBA8, 0, 0, 0, GL_TRUE);
Looks like its been fixed.
> I run the piglit tests, and with 0 for sample I don’t see
> regressions.
Looks like we need a text to make sure 0 isn't valid :)
> (also this suppose rollback the piglit gl-3.2-layered-rendering
> -framebuffertexture test case)
> (also I think the CTS needs update like (type >=
> CONTEXTTYPE_GL32_CORE && type < CONTEXTTYPE_GL40_CORE))
> The command I used is : ./cts-runner --type=gl33 --logdir=.
>
>
> This patch involve TexImage2DMultisample, TexImage3DMultisample,
> TexStorage2DMultisample, TexStorage3DMultisample
>
> >
> > >
> > > (It seems OpenGL 4.2 is the first spec to state the INVALID_VALUE
> > > error case for 0.)
> > > > > > OpenGL ES 3.1 spec also says "An INVALID_VALUE error is
> > > > > > generated if samples is zero.". You'll need to change you
> > > > > > check
> > > > > > below to include also ES 3.1.
> > > > > >
> > > > > > > Fixing the piglit test case gl-3.2-layered-rendering
> > > > > > > -framebuffertexture.
> > > > > > >
> > > > > > > Bugzilla:
> > > > > > > https://bugs.freedesktop.org/show_bug.cgi?id=93100
> > > > > > >
> > > > > > > Signed-off-by: Marius Predut <marius.predut at intel.com>
> > > > > > > ---
> > > > > > > src/mesa/main/teximage.c | 2 +-
> > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/src/mesa/main/teximage.c
> > > > > > > b/src/mesa/main/teximage.c index d9453e3..69634ff 100644
> > > > > > > --- a/src/mesa/main/teximage.c
> > > > > > > +++ b/src/mesa/main/teximage.c
> > > > > > > @@ -5211,7 +5211,7 @@ texture_image_multisample(struct
> > > > > > > gl_context *ctx, GLuint dims,
> > > > > > > return;
> > > > > > > }
> > > > > > >
> > > > > > > - if (samples < 1) {
> > > > > > > + if (samples < 1 && ctx->API == API_OPENGL_CORE && ctx
> > > > > > > ->Version
> > > > > > > > = 40) {
> > > > > > > _mesa_error(ctx, GL_INVALID_VALUE, "%s(samples <
> > > > > > > 1)",
> > > > > > > func);
> > > > > > > return;
> > > > > > > }
> > > > > > _______________________________________________
> > > > > > mesa-dev mailing list
> > > > > > mesa-dev at lists.freedesktop.org
> > > > > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > > > > _______________________________________________
> > > > > mesa-dev mailing list
> > > > > mesa-dev at lists.freedesktop.org
> > > > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list