On Thu, Aug 3, 2017 at 3:12 PM, Marek Olšák <mar...@gmail.com> wrote:
> I'm OK with the patch. I think most drivers support nr_samples of 0
> and 1, which have the same behavior.
That's definitely true for nouveau, in fact we've gone to some lengths
to support nr_samples == 1...
>
> Reviewed-by: Marek Olšák <marek.ol...@amd.com>
>
> Marek
>
> On Wed, Aug 2, 2017 at 7:07 PM, Brian Paul <bri...@vmware.com> wrote:
>> We pretty much 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.
>>
>> This patch changes the sample count checks in st_AllocTextureStorage()
>> and st_renderbuffer_alloc_storage() to test for samples > 0 instead of > 1.
>> As it is, if samples==1 we skip the search for the next higher number of
>> supported samples and ask the gallium driver to create a MSAA surface with
>> one sample, which no driver supports (AFAIK). Instead, drivers create a
>> non-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 real 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.
>> ---
>> src/mesa/main/mtypes.h | 2 +-
>> src/mesa/state_tracker/st_cb_fbo.c | 3 +--
>> src/mesa/state_tracker/st_cb_texture.c | 2 +-
>> 3 files changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index 404d586..1dec89c 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 23cbcdc..6986eaa 100644
>> --- a/src/mesa/state_tracker/st_cb_fbo.c
>> +++ b/src/mesa/state_tracker/st_cb_fbo.c
>> @@ -156,9 +156,8 @@ 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++) {
>> diff --git a/src/mesa/state_tracker/st_cb_texture.c
>> b/src/mesa/state_tracker/st_cb_texture.c
>> index db2913e..de6b57e 100644
>> --- a/src/mesa/state_tracker/st_cb_texture.c
>> +++ b/src/mesa/state_tracker/st_cb_texture.c
>> @@ -2680,7 +2680,7 @@ 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;
>>
>> --
>> 1.9.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
