Re: [Mesa-dev] [PATCH] mesa: Adds missing error condition in _mesa_check_sample_count()
On 23/02/15 11:24, Eduardo Lima Mitev wrote: On 02/23/2015 09:48 AM, Martin Peres wrote: On 23/02/15 10:33, Eduardo Lima Mitev wrote: Hi, can someone take a look? It should be very straight forward. Eduardo You are not supposed to return INVALID_OPERATION if samples 0. Also, samples == 0 is valid [0], so this is a NAK for me! The correct patch has already been posted to the ML: [PATCH 03/16] main: fix the validation of the number of samples Hi Martin, this is a different error check, coming from this wording: Section 4.4 (Framebuffer objects), page 198 of the OpenGL 3.0.0 specification says: If internalformat is a signed or unsigned integer format and samples is greater than zero, then the error INVALID_OPERATION is generated. See the original patch 19252fee46b835cb4f6b1cce18d7737d62b64a2e which missed the check for samples being greater than zero: I see, but I cannot find any mention of this in OGL 3.0, from August 11, 2008: https://www.opengl.org/registry/doc/glspec30.20080811.pdf In this document, Section 4.4 starts at page 278. Which document are you referring to? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Adds missing error condition in _mesa_check_sample_count()
On 02/23/2015 12:41 PM, Martin Peres wrote: Section 4.4 (Framebuffer objects), page 198 of the OpenGL 3.0.0 specification says: If internalformat is a signed or unsigned integer format and samples is greater than zero, then the error INVALID_OPERATION is generated. See the original patch 19252fee46b835cb4f6b1cce18d7737d62b64a2e which missed the check for samples being greater than zero: I see, but I cannot find any mention of this in OGL 3.0, from August 11, 2008: https://www.opengl.org/registry/doc/glspec30.20080811.pdf In this document, Section 4.4 starts at page 278. Which document are you referring to? I'm referring to OpenGL ES 3.0.0, so that's an error :). Good catch. The commit log does says GLES3 though (also see the _mesa_is_gles3() conditional). I will change this patch to fix that too. In any case, the issue here is the missing check for samples 0, which will likely cause regressions. Thanks for looking at it! Eduardo ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Adds missing error condition in _mesa_check_sample_count()
Hi, can someone take a look? It should be very straight forward. Eduardo On 02/20/2015 02:09 PM, Eduardo Lima Mitev wrote: This corrects a trivial error introduced in commit 19252fee46b835cb4f6b1cce18d7737d62b64a2e. That patch was merged recently and omits one condition (that 'samples' is greater than zero) in one of the error checks. That error will definitely cause regressions. --- src/mesa/main/multisample.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/multisample.c b/src/mesa/main/multisample.c index b696de9..50a8a11 100644 --- a/src/mesa/main/multisample.c +++ b/src/mesa/main/multisample.c @@ -155,7 +155,8 @@ _mesa_check_sample_count(struct gl_context *ctx, GLenum target, * If internalformat is a signed or unsigned integer format and samples * is greater than zero, then the error INVALID_OPERATION is generated. */ - if (_mesa_is_gles3(ctx) _mesa_is_enum_format_integer(internalFormat)) { + if (_mesa_is_gles3(ctx) _mesa_is_enum_format_integer(internalFormat) +samples 0) { return GL_INVALID_OPERATION; } ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Adds missing error condition in _mesa_check_sample_count()
On 23/02/15 10:33, Eduardo Lima Mitev wrote: Hi, can someone take a look? It should be very straight forward. Eduardo You are not supposed to return INVALID_OPERATION if samples 0. Also, samples == 0 is valid [0], so this is a NAK for me! The correct patch has already been posted to the ML: [PATCH 03/16] main: fix the validation of the number of samples [0]: http://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/main/fbobject.c#n1804 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Adds missing error condition in _mesa_check_sample_count()
On 02/23/2015 09:48 AM, Martin Peres wrote: On 23/02/15 10:33, Eduardo Lima Mitev wrote: Hi, can someone take a look? It should be very straight forward. Eduardo You are not supposed to return INVALID_OPERATION if samples 0. Also, samples == 0 is valid [0], so this is a NAK for me! The correct patch has already been posted to the ML: [PATCH 03/16] main: fix the validation of the number of samples Hi Martin, this is a different error check, coming from this wording: Section 4.4 (Framebuffer objects), page 198 of the OpenGL 3.0.0 specification says: If internalformat is a signed or unsigned integer format and samples is greater than zero, then the error INVALID_OPERATION is generated. See the original patch 19252fee46b835cb4f6b1cce18d7737d62b64a2e which missed the check for samples being greater than zero: cheers, Eduardo ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: Adds missing error condition in _mesa_check_sample_count()
This corrects a trivial error introduced in commit 19252fee46b835cb4f6b1cce18d7737d62b64a2e. That patch was merged recently and omits one condition (that 'samples' is greater than zero) in one of the error checks. That error will definitely cause regressions. --- src/mesa/main/multisample.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/multisample.c b/src/mesa/main/multisample.c index b696de9..50a8a11 100644 --- a/src/mesa/main/multisample.c +++ b/src/mesa/main/multisample.c @@ -155,7 +155,8 @@ _mesa_check_sample_count(struct gl_context *ctx, GLenum target, * If internalformat is a signed or unsigned integer format and samples * is greater than zero, then the error INVALID_OPERATION is generated. */ - if (_mesa_is_gles3(ctx) _mesa_is_enum_format_integer(internalFormat)) { + if (_mesa_is_gles3(ctx) _mesa_is_enum_format_integer(internalFormat) +samples 0) { return GL_INVALID_OPERATION; } -- 2.1.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev