> -----Original Message----- > From: Timothy Arceri [mailto:t_arc...@yahoo.com.au] > Sent: Thursday, November 26, 2015 11:32 PM > To: Predut, Marius; Palli, Tapani; mesa-dev@lists.freedesktop.org > Subject: Re: [Mesa-dev] [PATCH] mesa/main: TexImage2DMultisample needs to pass > OpenGL3.3 conformance test. > > On Thu, 2015-11-26 at 11:00 +0000, Predut, Marius wrote: > > > -----Original Message----- > > > From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On > > > Behalf Of Timothy Arceri > > > Sent: Wednesday, November 25, 2015 6:02 PM > > > To: Palli, Tapani; Predut, Marius; mesa-dev@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-boun...@lists.freedesktop.org > > > > > > ] On > > > > > > Behalf Of Timothy Arceri > > > > > > Sent: Wednesday, November 25, 2015 1:12 PM > > > > > > To: Palli, Tapani; Predut, Marius; > > > > > > mesa-dev@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 :)
OK, in this case, can you clarify this with Kronos and may be they will decide to fix wrong reference https://www.opengl.org/sdk/docs/man3/xhtml/glTexImage3DMultisample.xml Or if this is correct then needs to rollback the CTS. thanks marius > > > (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.pre...@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@lists.freedesktop.org > > > > > > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > > > > _______________________________________________ > > > > > > mesa-dev mailing list > > > > > > mesa-dev@lists.freedesktop.org > > > > > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > > > > > _______________________________________________ > > > mesa-dev mailing list > > > mesa-dev@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
