Re: [Piglit] [PATCH 1/3] arb_texture_multisample: don't use hard-coded sample counts

2017-12-05 Thread Brian Paul

On 12/05/2017 10:19 AM, Roland Scheidegger wrote:

Am 05.12.2017 um 17:43 schrieb Brian Paul:

Instead of using a fixed number of samples, query the
GL_MAX_COLOR_TEXTURE_SAMPLES or GL_MAX_DEPTH_TEXTURE_SAMPLES value
and use those.

Fixes failures with llvmpipe and VMware driver when MSAA is disabled.
---
  tests/spec/arb_texture_multisample/errors.c| 10 +-
  tests/spec/arb_texture_multisample/sample-depth.c  | 12 +++
  tests/spec/arb_texture_multisample/stencil-clear.c | 23 +-
  .../teximage-2d-multisample.c  | 13 
  .../teximage-3d-multisample.c  | 13 
  5 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/tests/spec/arb_texture_multisample/errors.c 
b/tests/spec/arb_texture_multisample/errors.c
index 42cc2c1..08e1696 100644
--- a/tests/spec/arb_texture_multisample/errors.c
+++ b/tests/spec/arb_texture_multisample/errors.c
@@ -44,6 +44,14 @@ piglit_init(int argc, char **argv)

  GLuint fbo;
  GLuint tex[2];
+GLint max_samples;
+
+glGetIntegerv(GL_MAX_COLOR_TEXTURE_SAMPLES, _samples);
+if (max_samples < 1) {
+   printf("GL_MAX_COLOR_TEXTURE_SAMPLES must be at least one\n");
+   piglit_report_result(PIGLIT_FAIL);
+}

Do you want to use the maximum possible or restrict that to something
smaller (in case the value returned is large)?


Whatever value is returned, it should work.  If not, either the driver 
is broken or perhaps the surface is too large (but all these tests use 
small textures).




(Same below, and in the other patches.)
Either way, for the series:
Reviewed-by: Roland Scheidegger 


Thanks.

-Brian





+
  glGenFramebuffers(1, );

  glBindFramebuffer(GL_FRAMEBUFFER, fbo);
@@ -51,7 +59,7 @@ piglit_init(int argc, char **argv)
  glGenTextures(2, tex);
  glBindTexture(GL_TEXTURE_2D_MULTISAMPLE_ARRAY, tex[0]);
  glTexImage3DMultisample(GL_TEXTURE_2D_MULTISAMPLE_ARRAY,
-4, GL_RGBA, 64, 64, 2, GL_TRUE);
+max_samples, GL_RGBA, 64, 64, 2, GL_TRUE);

  if (!piglit_check_gl_error(GL_NO_ERROR)) {
  printf("should be no error so far\n");
diff --git a/tests/spec/arb_texture_multisample/sample-depth.c 
b/tests/spec/arb_texture_multisample/sample-depth.c
index ef2be19..94bc63d 100644
--- a/tests/spec/arb_texture_multisample/sample-depth.c
+++ b/tests/spec/arb_texture_multisample/sample-depth.c
@@ -38,7 +38,6 @@ PIGLIT_GL_TEST_CONFIG_BEGIN

  PIGLIT_GL_TEST_CONFIG_END

-#define NUM_SAMPLES 4
  #define TEX_WIDTH 64
  #define TEX_HEIGHT 64

@@ -93,16 +92,21 @@ void
  piglit_init(int argc, char **argv)
  {
GLuint tex;
+   int num_samples;
+
piglit_require_extension("GL_ARB_texture_multisample");

+   /* Use the max number of samples for testing */
+   glGetIntegerv(GL_MAX_COLOR_TEXTURE_SAMPLES, _samples);
+
/* setup an fbo with multisample depth texture */

glGenTextures(1, );
glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, tex);
glTexImage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE,
-   NUM_SAMPLES, GL_DEPTH_COMPONENT24,
-   TEX_WIDTH, TEX_HEIGHT,
-   GL_TRUE);
+   num_samples, GL_DEPTH_COMPONENT24,
+   TEX_WIDTH, TEX_HEIGHT,
+   GL_TRUE);

glGenFramebuffers(1, );
glBindFramebuffer(GL_DRAW_FRAMEBUFFER, fbo);
diff --git a/tests/spec/arb_texture_multisample/stencil-clear.c 
b/tests/spec/arb_texture_multisample/stencil-clear.c
index ca0fd81..413aa41 100644
--- a/tests/spec/arb_texture_multisample/stencil-clear.c
+++ b/tests/spec/arb_texture_multisample/stencil-clear.c
@@ -108,7 +108,7 @@ piglit_display(void)
return pass ? PIGLIT_PASS : PIGLIT_FAIL;
  }

-GLuint create_fbo(unsigned num_samples)
+GLuint create_fbo(unsigned num_color_samples, unsigned num_depth_samples)
  {
GLenum tex_target;
GLuint texColor;
@@ -119,17 +119,17 @@ GLuint create_fbo(unsigned num_samples)
glGenTextures(1, );
glGenTextures(1, );

-   if (num_samples != 0) {
+   if (num_color_samples != 0) {
tex_target = GL_TEXTURE_2D_MULTISAMPLE;

glBindTexture(tex_target, texZS);
glTexImage2DMultisample(
-   tex_target, num_samples, GL_DEPTH32F_STENCIL8,
+   tex_target, num_depth_samples, GL_DEPTH32F_STENCIL8,
TEX_WIDTH, TEX_HEIGHT, GL_TRUE);

glBindTexture(tex_target, texColor);
glTexImage2DMultisample(
-   tex_target, num_samples, GL_RGBA8,
+   tex_target, num_color_samples, GL_RGBA8,
TEX_WIDTH, TEX_HEIGHT, GL_TRUE);
} else {
tex_target = GL_TEXTURE_2D;
@@ -167,22 +167,27 @@ GLuint create_fbo(unsigned num_samples)
  void
  piglit_init(int argc, 

Re: [Piglit] [PATCH 1/3] arb_texture_multisample: don't use hard-coded sample counts

2017-12-05 Thread Roland Scheidegger
Am 05.12.2017 um 17:43 schrieb Brian Paul:
> Instead of using a fixed number of samples, query the
> GL_MAX_COLOR_TEXTURE_SAMPLES or GL_MAX_DEPTH_TEXTURE_SAMPLES value
> and use those.
> 
> Fixes failures with llvmpipe and VMware driver when MSAA is disabled.
> ---
>  tests/spec/arb_texture_multisample/errors.c| 10 +-
>  tests/spec/arb_texture_multisample/sample-depth.c  | 12 +++
>  tests/spec/arb_texture_multisample/stencil-clear.c | 23 
> +-
>  .../teximage-2d-multisample.c  | 13 
>  .../teximage-3d-multisample.c  | 13 
>  5 files changed, 49 insertions(+), 22 deletions(-)
> 
> diff --git a/tests/spec/arb_texture_multisample/errors.c 
> b/tests/spec/arb_texture_multisample/errors.c
> index 42cc2c1..08e1696 100644
> --- a/tests/spec/arb_texture_multisample/errors.c
> +++ b/tests/spec/arb_texture_multisample/errors.c
> @@ -44,6 +44,14 @@ piglit_init(int argc, char **argv)
>  
>  GLuint fbo;
>  GLuint tex[2];
> +GLint max_samples;
> +
> +glGetIntegerv(GL_MAX_COLOR_TEXTURE_SAMPLES, _samples);
> +if (max_samples < 1) {
> +   printf("GL_MAX_COLOR_TEXTURE_SAMPLES must be at least one\n");
> +   piglit_report_result(PIGLIT_FAIL);
> +}
Do you want to use the maximum possible or restrict that to something
smaller (in case the value returned is large)?
(Same below, and in the other patches.)
Either way, for the series:
Reviewed-by: Roland Scheidegger 


> +
>  glGenFramebuffers(1, );
>  
>  glBindFramebuffer(GL_FRAMEBUFFER, fbo);
> @@ -51,7 +59,7 @@ piglit_init(int argc, char **argv)
>  glGenTextures(2, tex);
>  glBindTexture(GL_TEXTURE_2D_MULTISAMPLE_ARRAY, tex[0]);
>  glTexImage3DMultisample(GL_TEXTURE_2D_MULTISAMPLE_ARRAY,
> -4, GL_RGBA, 64, 64, 2, GL_TRUE);
> +max_samples, GL_RGBA, 64, 64, 2, GL_TRUE);
>  
>  if (!piglit_check_gl_error(GL_NO_ERROR)) {
>  printf("should be no error so far\n");
> diff --git a/tests/spec/arb_texture_multisample/sample-depth.c 
> b/tests/spec/arb_texture_multisample/sample-depth.c
> index ef2be19..94bc63d 100644
> --- a/tests/spec/arb_texture_multisample/sample-depth.c
> +++ b/tests/spec/arb_texture_multisample/sample-depth.c
> @@ -38,7 +38,6 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
>  
>  PIGLIT_GL_TEST_CONFIG_END
>  
> -#define NUM_SAMPLES 4
>  #define TEX_WIDTH 64
>  #define TEX_HEIGHT 64
>  
> @@ -93,16 +92,21 @@ void
>  piglit_init(int argc, char **argv)
>  {
>   GLuint tex;
> + int num_samples;
> +
>   piglit_require_extension("GL_ARB_texture_multisample");
>  
> + /* Use the max number of samples for testing */
> + glGetIntegerv(GL_MAX_COLOR_TEXTURE_SAMPLES, _samples);
> +
>   /* setup an fbo with multisample depth texture */
>  
>   glGenTextures(1, );
>   glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, tex);
>   glTexImage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE,
> - NUM_SAMPLES, GL_DEPTH_COMPONENT24,
> - TEX_WIDTH, TEX_HEIGHT,
> - GL_TRUE);
> + num_samples, GL_DEPTH_COMPONENT24,
> + TEX_WIDTH, TEX_HEIGHT,
> + GL_TRUE);
>  
>   glGenFramebuffers(1, );
>   glBindFramebuffer(GL_DRAW_FRAMEBUFFER, fbo);
> diff --git a/tests/spec/arb_texture_multisample/stencil-clear.c 
> b/tests/spec/arb_texture_multisample/stencil-clear.c
> index ca0fd81..413aa41 100644
> --- a/tests/spec/arb_texture_multisample/stencil-clear.c
> +++ b/tests/spec/arb_texture_multisample/stencil-clear.c
> @@ -108,7 +108,7 @@ piglit_display(void)
>   return pass ? PIGLIT_PASS : PIGLIT_FAIL;
>  }
>  
> -GLuint create_fbo(unsigned num_samples)
> +GLuint create_fbo(unsigned num_color_samples, unsigned num_depth_samples)
>  {
>   GLenum tex_target;
>   GLuint texColor;
> @@ -119,17 +119,17 @@ GLuint create_fbo(unsigned num_samples)
>   glGenTextures(1, );
>   glGenTextures(1, );
>  
> - if (num_samples != 0) {
> + if (num_color_samples != 0) {
>   tex_target = GL_TEXTURE_2D_MULTISAMPLE;
>  
>   glBindTexture(tex_target, texZS);
>   glTexImage2DMultisample(
> - tex_target, num_samples, GL_DEPTH32F_STENCIL8,
> + tex_target, num_depth_samples, GL_DEPTH32F_STENCIL8,
>   TEX_WIDTH, TEX_HEIGHT, GL_TRUE);
>  
>   glBindTexture(tex_target, texColor);
>   glTexImage2DMultisample(
> - tex_target, num_samples, GL_RGBA8,
> + tex_target, num_color_samples, GL_RGBA8,
>   TEX_WIDTH, TEX_HEIGHT, GL_TRUE);
>   } else {
>   tex_target = GL_TEXTURE_2D;
> @@ -167,22 +167,27 @@ GLuint create_fbo(unsigned num_samples)
>  void
>  piglit_init(int argc, char **argv)
>  {
> - unsigned num_samples = 4;
> + GLint num_color_samples,