From 139f2c662978f3727777ddd90068b66a161a2f0a Mon Sep 17 00:00:00 2001 From: Erik Faye-Lund Date: Fri, 11 Apr 2025 16:52:01 +0200 Subject: [PATCH] mesa/main: contain mutlisample advanced limitations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The AMD_framebuffer_multisample_advanced extension interacts kinda badly with ARB_internalformat_query and GLES 3. The problem is that ARB_internalformat_query etc untangles MAX_SAMPLES and per-internalformat sample limits, but the multisample advanced spec explicitly requires that we can't go above the global maximum. This isn't the biggest problem in the world; one could just deal with these limitations for the multisample_advanced functions... if it wasn't for the fact that we by mistake apply the multisample_advanced limitations for functions where they shouldn't be taken into account. Whoops. So let's make sure we don't do that. Reviewed-by: Marek Olšák Part-of: --- src/mesa/main/fbobject.c | 35 +++++++++-- src/mesa/main/multisample.c | 116 +++++++++++++++++++----------------- src/mesa/main/multisample.h | 7 ++- src/mesa/main/teximage.c | 2 +- 4 files changed, 98 insertions(+), 62 deletions(-) diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c index 4afd7ec7c29..a9fab8c0818 100644 --- a/src/mesa/main/fbobject.c +++ b/src/mesa/main/fbobject.c @@ -2759,8 +2759,12 @@ _mesa_renderbuffer_storage(struct gl_context *ctx, struct gl_renderbuffer *rb, if (samples != 0) { assert(samples > 0); assert(_mesa_check_sample_count(ctx, GL_RENDERBUFFER, - internalFormat, samples, - storageSamples) == GL_NO_ERROR); + internalFormat, samples) == GL_NO_ERROR); + } + if (samples != storageSamples) { + assert(_mesa_check_storage_sample_count(ctx, + internalFormat, samples, + storageSamples) == GL_NO_ERROR); } FLUSH_VERTICES(ctx, _NEW_BUFFERS, 0); @@ -2850,7 +2854,7 @@ renderbuffer_storage(struct gl_context *ctx, struct gl_renderbuffer *rb, * note: driver may choose to use more samples than what's requested */ sample_count_error = _mesa_check_sample_count(ctx, GL_RENDERBUFFER, - internalFormat, samples, storageSamples); + internalFormat, samples); /* Section 2.5 (GL Errors) of OpenGL 3.0 specification, page 16: * @@ -2951,7 +2955,6 @@ renderbuffer_storage_target(GLenum target, GLenum internalFormat, height, samples, storageSamples, func); } - void GLAPIENTRY _mesa_EGLImageTargetRenderbufferStorageOES(GLenum target, GLeglImageOES image) { @@ -3037,6 +3040,17 @@ _mesa_RenderbufferStorageMultisampleAdvancedAMD( GLenum target, GLsizei samples, GLsizei storageSamples, GLenum internalFormat, GLsizei width, GLsizei height) { + GET_CURRENT_CONTEXT(ctx); + GLenum sample_count_error = + _mesa_check_storage_sample_count(ctx, internalFormat, samples, + storageSamples); + if (sample_count_error != GL_NO_ERROR) { + _mesa_error(ctx, sample_count_error, + "glRenderbufferStorageMultisampleAdvancedAMD(samples=%d)", + samples); + return; + } + renderbuffer_storage_target(target, internalFormat, width, height, samples, storageSamples, "glRenderbufferStorageMultisampleAdvancedAMD"); @@ -3113,6 +3127,17 @@ _mesa_NamedRenderbufferStorageMultisampleAdvancedAMD( GLuint renderbuffer, GLsizei samples, GLsizei storageSamples, GLenum internalformat, GLsizei width, GLsizei height) { + GET_CURRENT_CONTEXT(ctx); + GLenum sample_count_error = + _mesa_check_storage_sample_count(ctx, internalformat, samples, + storageSamples); + if (sample_count_error != GL_NO_ERROR) { + _mesa_error(ctx, sample_count_error, + "glNamedRenderbufferStorageMultisampleAdvancedAMD(samples=%d)", + samples); + return; + } + renderbuffer_storage_named(renderbuffer, internalformat, width, height, samples, storageSamples, "glNamedRenderbufferStorageMultisampleAdvancedAMD"); @@ -4043,7 +4068,7 @@ check_samples(struct gl_context *ctx, struct gl_texture_object *texObj, assert(texImage); GLenum sample_count_error = _mesa_check_sample_count(ctx, target, texImage->InternalFormat, - samples, samples); + samples); if (sample_count_error != GL_NO_ERROR) { _mesa_error(ctx, sample_count_error, "%s(samples=%d)", caller, samples); diff --git a/src/mesa/main/multisample.c b/src/mesa/main/multisample.c index 9dceb29734b..55d505edb0a 100644 --- a/src/mesa/main/multisample.c +++ b/src/mesa/main/multisample.c @@ -231,8 +231,7 @@ _mesa_MinSampleShading(GLclampf value) */ GLenum _mesa_check_sample_count(struct gl_context *ctx, GLenum target, - GLenum internalFormat, GLsizei samples, - GLsizei storageSamples) + GLenum internalFormat, GLsizei samples) { /* Section 4.4 (Framebuffer objects), page 198 of the OpenGL ES 3.0.0 * specification says: @@ -248,58 +247,6 @@ _mesa_check_sample_count(struct gl_context *ctx, GLenum target, return GL_INVALID_OPERATION; } - if (_mesa_has_AMD_framebuffer_multisample_advanced(ctx) && - target == GL_RENDERBUFFER) { - if (!_mesa_is_depth_or_stencil_format(internalFormat)) { - /* From the AMD_framebuffer_multisample_advanced spec: - * - * "An INVALID_OPERATION error is generated if - * is a color format and is greater than - * the implementation-dependent limit MAX_COLOR_FRAMEBUFFER_- - * STORAGE_SAMPLES_AMD." - */ - if (samples > ctx->Const.MaxColorFramebufferSamples) - return GL_INVALID_OPERATION; - - /* From the AMD_framebuffer_multisample_advanced spec: - * - * "An INVALID_OPERATION error is generated if - * is a color format and is greater than - * the implementation-dependent limit MAX_COLOR_FRAMEBUFFER_- - * STORAGE_SAMPLES_AMD." - */ - if (storageSamples > ctx->Const.MaxColorFramebufferStorageSamples) - return GL_INVALID_OPERATION; - - /* From the AMD_framebuffer_multisample_advanced spec: - * - * "An INVALID_OPERATION error is generated if is - * greater than ." - */ - if (storageSamples > samples) - return GL_INVALID_OPERATION; - - /* Color renderbuffer sample counts are now fully validated - * according to AMD_framebuffer_multisample_advanced. - */ - return GL_NO_ERROR; - } else { - /* From the AMD_framebuffer_multisample_advanced spec: - * - * "An INVALID_OPERATION error is generated if is - * a depth or stencil format and is not equal to - * ." - */ - if (storageSamples != samples) - return GL_INVALID_OPERATION; - } - } else { - /* If the extension is unsupported, it's not possible to set - * storageSamples differently. - */ - assert(samples == storageSamples); - } - /* If ARB_internalformat_query is supported, then treat its highest * returned sample count as the absolute maximum for this format; it is * allowed to exceed MAX_SAMPLES. @@ -374,6 +321,67 @@ _mesa_check_sample_count(struct gl_context *ctx, GLenum target, ? GL_INVALID_VALUE : GL_NO_ERROR; } +/** + * Helper for checking a requested sample count against the limit + * for a particular (target, internalFormat) pair. The limit imposed, + * and the error generated, both depend on which extensions are supported. + * + * Returns a GL error enum, or GL_NO_ERROR if the requested sample count is + * acceptable. + */ +GLenum +_mesa_check_storage_sample_count(struct gl_context *ctx, GLenum internalFormat, + GLsizei samples, GLsizei storageSamples) +{ + assert(_mesa_has_AMD_framebuffer_multisample_advanced(ctx)); + + if (!_mesa_is_depth_or_stencil_format(internalFormat)) { + /* From the AMD_framebuffer_multisample_advanced spec: + * + * "An INVALID_OPERATION error is generated if + * is a color format and is greater than + * the implementation-dependent limit MAX_COLOR_FRAMEBUFFER_- + * STORAGE_SAMPLES_AMD." + */ + if (samples > ctx->Const.MaxColorFramebufferSamples) + return GL_INVALID_OPERATION; + + /* From the AMD_framebuffer_multisample_advanced spec: + * + * "An INVALID_OPERATION error is generated if + * is a color format and is greater than + * the implementation-dependent limit MAX_COLOR_FRAMEBUFFER_- + * STORAGE_SAMPLES_AMD." + */ + if (storageSamples > ctx->Const.MaxColorFramebufferStorageSamples) + return GL_INVALID_OPERATION; + + /* From the AMD_framebuffer_multisample_advanced spec: + * + * "An INVALID_OPERATION error is generated if is + * greater than ." + */ + if (storageSamples > samples) + return GL_INVALID_OPERATION; + + /* Color renderbuffer sample counts are now fully validated + * according to AMD_framebuffer_multisample_advanced. + */ + return GL_NO_ERROR; + } else { + /* From the AMD_framebuffer_multisample_advanced spec: + * + * "An INVALID_OPERATION error is generated if is + * a depth or stencil format and is not equal to + * ." + */ + if (storageSamples != samples) + return GL_INVALID_OPERATION; + } + + return GL_NO_ERROR; +} + void GLAPIENTRY _mesa_AlphaToCoverageDitherControlNV_no_error(GLenum mode) { diff --git a/src/mesa/main/multisample.h b/src/mesa/main/multisample.h index 5bb4ed68697..b7c21826bb7 100644 --- a/src/mesa/main/multisample.h +++ b/src/mesa/main/multisample.h @@ -39,7 +39,10 @@ _mesa_init_multisample(struct gl_context *ctx); extern GLenum _mesa_check_sample_count(struct gl_context *ctx, GLenum target, - GLenum internalFormat, GLsizei samples, - GLsizei storageSamples); + GLenum internalFormat, GLsizei samples); + +extern GLenum +_mesa_check_storage_sample_count(struct gl_context *ctx, GLenum internalFormat, + GLsizei samples, GLsizei storageSamples); #endif diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c index ee452ebaa79..4d2959443f3 100644 --- a/src/mesa/main/teximage.c +++ b/src/mesa/main/teximage.c @@ -7131,7 +7131,7 @@ texture_image_multisample(struct gl_context *ctx, GLuint dims, } sample_count_error = _mesa_check_sample_count(ctx, target, - internalformat, samples, samples); + internalformat, samples); samplesOK = sample_count_error == GL_NO_ERROR; /* Page 254 of OpenGL 4.4 spec says: