From: Brian Paul Date: Wed, 2 Aug 2017 03:28:25 +0000 (-0600) Subject: st/mesa: fix handling of NumSamples=1 (v2) X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=6839d3369905eb02151334ea7b4cd39ddcfa6770;p=mesa.git st/mesa: fix handling of NumSamples=1 (v2) In Mesa we use the convention that if gl_renderbuffer::NumSamples or gl_texture_image::NumSamples is zero, it's a non-MSAA surface. Otherwise, it's an MSAA surface. But in gallium nr_samples=1 is a non-MSAA surface. Before, if the user called glRenderbufferStorageMultisample() or glTexImage2DMultisample() with samples=1 we skipped the search for the next higher number of supported samples and asked the gallium driver to create a surface with nr_samples=1. So we got a non-MSAA surface. This failed to meet the expection of the user making those calls. This patch changes the sample count checks in st_AllocTextureStorage() and st_renderbuffer_alloc_storage() to test for samples > 0 instead of > 1. And we now start querying for MSAA support at samples=2 since gallium has no concept of a 1x MSAA surface. A specific example of this problem is the Piglit arb_framebuffer_srgb-blit test. It calls glRenderbufferStorageMultisample() with samples=1 to request an MSAA renderbuffer with the minimum supported number of MSAA samples. Instead of creating a 4x or 8x, etc. MSAA surface, we wound up creating a non-MSAA surface. Finally, add a comment on the gl_renderbuffer::NumSamples field. There is one piglit regression with the VMware driver: ext_framebuffer_multisample-blit-mismatched-formats fails because now we're actually creating 4x MSAA surfaces (the requested sample count is 1) and we're hitting some sort of bug in the blitter code. That will have to be fixed separately. Other drivers may find regressions too now that MSAA surfaces are really being created. v2: start quering for MSAA support with samples=2 instead of 1. Reviewed-by: Roland Scheidegger Reviewed-by: Marek Olšák --- diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 404d586ff7b..1dec89c5445 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -3303,7 +3303,7 @@ struct gl_renderbuffer * called without a rb->TexImage. */ GLboolean NeedsFinishRenderTexture; - GLubyte NumSamples; + GLubyte NumSamples; /**< zero means not multisampled */ GLenum InternalFormat; /**< The user-specified format */ GLenum _BaseFormat; /**< Either GL_RGB, GL_RGBA, GL_DEPTH_COMPONENT or GL_STENCIL_INDEX. */ diff --git a/src/mesa/state_tracker/st_cb_fbo.c b/src/mesa/state_tracker/st_cb_fbo.c index 23cbcdc2a19..afc7700306e 100644 --- a/src/mesa/state_tracker/st_cb_fbo.c +++ b/src/mesa/state_tracker/st_cb_fbo.c @@ -156,12 +156,11 @@ st_renderbuffer_alloc_storage(struct gl_context * ctx, * by the implementation. * * So let's find the supported number of samples closest to NumSamples. - * (NumSamples == 1) is treated the same as (NumSamples == 0). */ - if (rb->NumSamples > 1) { + if (rb->NumSamples > 0) { unsigned i; - for (i = rb->NumSamples; i <= ctx->Const.MaxSamples; i++) { + for (i = MAX2(2, rb->NumSamples); i <= ctx->Const.MaxSamples; i++) { format = st_choose_renderbuffer_format(st, internalFormat, i); if (format != PIPE_FORMAT_NONE) { diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c index db2913ed9e3..f1eb548bb87 100644 --- a/src/mesa/state_tracker/st_cb_texture.c +++ b/src/mesa/state_tracker/st_cb_texture.c @@ -2680,10 +2680,13 @@ st_AllocTextureStorage(struct gl_context *ctx, bindings = default_bindings(st, fmt); /* Raise the sample count if the requested one is unsupported. */ - if (num_samples > 1) { + if (num_samples > 0) { enum pipe_texture_target ptarget = gl_target_to_pipe(texObj->Target); boolean found = FALSE; + /* start the query with at least two samples */ + num_samples = MAX2(num_samples, 2); + for (; num_samples <= ctx->Const.MaxSamples; num_samples++) { if (screen->is_format_supported(screen, fmt, ptarget, num_samples,