Re: [Mesa-dev] [PATCH] mesa: Adds missing error condition in _mesa_check_sample_count()

2015-02-23 Thread Martin Peres


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()

2015-02-23 Thread Eduardo Lima Mitev
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()

2015-02-23 Thread Eduardo Lima Mitev
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()

2015-02-23 Thread Martin Peres


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()

2015-02-23 Thread Eduardo Lima Mitev
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()

2015-02-20 Thread Eduardo Lima Mitev
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