[Mesa-dev] [PATCH 3/3] mesa: use _mesa_check_sample_count() for multisample textures

[Brian Paul]

On 02/17/2013 03:35 AM, Chris Forbes wrote:
> Extends _mesa_check_sample_count() to properly support the
> TEXTURE_2D_MULTISAMPLE and TEXTURE_2D_MULTISAMPLE_ARRAY targets, which
> have subtly different limits than renderbuffers:
>
> The ARB_texture_multisample spec (or GL3.2) says, when describing the
> operation of TexImage*DMultisample:
>
>     "The error INVALID_OPERATION may be generated if any of the following
>     are true:
>
>     *<internalformat>  is a depth/stencil-renderable format and<samples>
>       is greater than the value of MAX_DEPTH_TEXTURE_SAMPLES
>     *<internalformat>  is a color-renderable format and<samples>  is
>       greater than the value of MAX_COLOR_TEXTURE_SAMPLES
>     *<internalformat>  is a signed or unsigned integer format and
>       <samples>  is greater than the value of MAX_INTEGER_SAMPLES"
>
>     And additionally, slightly later:
>
>     "... or if<samples>  is greater than MAX_SAMPLES, then the error
>     INVALID_VALUE is generated."
>
> If ARB_internalformat_query (or GL4.2) is supported, all of these limits
> are replaced by:
>
>     "The error INVALID_OPERATION will be generated if<samples>  is
>     greater than the maximum number of samples supported for this
>     <target>  and<internalformat>, which can be determined by calling
>     GetInternalformativ with a<pname>  of SAMPLES (see section 6.X)."
>

Shouldn't spec quotes like that go into the code instead for future 
reference?


> This resolves the remaining TODO in the implementation of
> TexImage*DMultisample.
>
> Signed-off-by: Chris Forbes<chrisf at ijw.co.nz>
> ---
>   src/mesa/main/fbobject.c | 11 +++++++++++
>   src/mesa/main/teximage.c | 30 +++++-------------------------
>   2 files changed, 16 insertions(+), 25 deletions(-)
>
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index 50339c2..208cf0d 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -1471,6 +1471,17 @@ _mesa_check_sample_count(struct gl_context *ctx, GLenum target,
>      if (ctx->Extensions.ARB_texture_multisample) {
>         if (_mesa_is_enum_format_integer(internalFormat))
>            return samples>  ctx->Const.MaxIntegerSamples ? GL_INVALID_OPERATION : GL_NO_ERROR;
> +
> +      if (target == GL_TEXTURE_2D_MULTISAMPLE ||
> +          target == GL_TEXTURE_2D_MULTISAMPLE_ARRAY) {
> +
> +         if (_mesa_is_depth_or_stencil_format(internalFormat))
> +            return samples>  ctx->Const.MaxDepthTextureSamples
> +               ? GL_INVALID_OPERATION : GL_NO_ERROR;
> +         else
> +            return samples>  ctx->Const.MaxColorTextureSamples
> +               ? GL_INVALID_OPERATION : GL_NO_ERROR;
> +      }
>      }
>
>      /* No more specific limit is available, so just use MAX_SAMPLES */
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index dc9543f..edf0a4c 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -4216,35 +4216,15 @@ teximagemultisample(GLuint dims, GLenum target, GLsizei samples,
>         return;
>      }
>
> -   if (_mesa_is_enum_format_integer(internalformat)) {
> -      if (samples>  ctx->Const.MaxIntegerSamples) {
> -         _mesa_error(ctx, GL_INVALID_OPERATION,
> -               "glTexImage%uDMultisample(samples>GL_MAX_INTEGER_SAMPLES)",
> -               dims);
> -         return;
> -      }
> -   }
> -   else if (_mesa_is_depth_or_stencil_format(internalformat)) {
> -      if (samples>  ctx->Const.MaxDepthTextureSamples) {
> -         _mesa_error(ctx, GL_INVALID_OPERATION,
> -               "glTexImage%uDMultisample(samples>GL_MAX_DEPTH_TEXTURE_SAMPLES)",
> -               dims);
> -         return;
> -      }
> -   }
> -   else {
> -      if (samples>  ctx->Const.MaxColorTextureSamples) {
> -         _mesa_error(ctx, GL_INVALID_OPERATION,
> -               "glTexImage%uDMultisample(samples>GL_MAX_COLOR_TEXTURE_SAMPLES)",
> -               dims);
> +   {
> +      GLenum sample_count_error = _mesa_check_sample_count(ctx, target,
> +            internalformat, samples);
> +      if (sample_count_error != GL_NO_ERROR) {
> +         _mesa_error(ctx, sample_count_error, "glTexImage%uMultisample(samples)", dims);
>            return;
>         }
>      }
>
> -   /* TODO: should ask the driver for the exact limit for this internalformat
> -    * once IDR's internalformat_query bits land
> -    */
> -
>      texObj = _mesa_get_current_tex_object(ctx, target);
>      texImage = _mesa_get_tex_image(ctx, texObj, 0, 0);
>

Other than my nits, looks good!

Reviewed-by: Brian Paul <brianp at vmware.com>