Bug 93957 - [HSW] Mishandling of sample count when using an attachment-less framebuffer (assertion error)
Summary: [HSW] Mishandling of sample count when using an attachment-less framebuffer (...
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Ian Romanick
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-01 17:18 UTC by Ilia Mirkin
Modified: 2016-02-05 11:17 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ilia Mirkin 2016-02-01 17:18:25 UTC
This manifests in the dEQP test: dEQP-GLES31.functional.fbo.no_attachments.multisample.samples2

gen6_multisample_state.c:150: gen6_emit_3dstate_multisample: Assertion `!"Unrecognized num_samples in gen6_emit_3dstate_multisample"' failed.
(gdb) p brw->num_samples
$1 = 2

But of course the minimum number of samples handled is 4. You're allowed to upgrade this in the driver, just like for glRenderbufferStorageMultisample. See the ARB_framebuffer_no_attachments spec:

"""
    The number of samples in the
    framebuffer is derived from the value of FRAMEBUFFER_DEFAULT_SAMPLES in an
    implementation-dependent manner similar to that described for the command
    RenderbufferStorageMultisample (section 4.4.2).
"""
Comment 1 Ilia Mirkin 2016-02-01 19:11:56 UTC
Looking a bit closer, in brw_upload_pipeline_state, we have:

   unsigned int fb_samples = _mesa_geometric_samples(ctx->DrawBuffer);

...

   if (brw->num_samples != fb_samples) {
      brw->num_samples = fb_samples;
      brw->ctx.NewDriverState |= BRW_NEW_NUM_SAMPLES;
   }

Presumably you can't make an actual fb attachment with the "wrong" number of samples, but there's nothing to validate that here for the no-attachment case. Not sure if the correct fix is to apply the correction here, or if it should be handled in gen6_multisample_state.c directly.
Comment 2 Neil Roberts 2016-02-03 18:08:39 UTC
I started looking into this and made a Piglit test for it here:

http://patchwork.freedesktop.org/patch/72536/

The test shows up more problems in that the value of glGetInteger(GL_SAMPLES) and also gl_NumSamples in the fragment shader is always 0 with a framebuffer with no attachments.

I think the solution would be to make these two variables use _mesa_geometric_samples and then somehow make that function give the driver a chance to return a quantized result. That would also fix the driver side when programming the multisample state because it already calls _mesa_geometric_samples to determine the sample count.
Comment 3 Ilia Mirkin 2016-02-03 18:13:32 UTC
Actually I wrote a test that will also hit it: http://patchwork.freedesktop.org/patch/72388/

But I never did an odd number of samples, that's just odd :)

I think there is a big bit missing, which is something in between the default geometry's samples setting and the "real" one. Something like DefaultGeometry._Samples which gets set at _NEW_BUFFERS validation time maybe? And maybe by each driver directly.
Comment 4 Neil Roberts 2016-02-04 16:13:28 UTC
Thanks for the suggestion. I've posted a patch for it here:

http://patchwork.freedesktop.org/patch/72634/
Comment 5 Neil Roberts 2016-02-05 11:17:19 UTC
I've pushed all the patches here:

http://cgit.freedesktop.org/mesa/mesa/commit/?id=eb9cf3cfc99db0cff0a5a584941e8b

Ilia, thanks for the review and for confirming the Piglit test the NVidia driver.


bug/show.html.tmpl processed on Feb 25, 2017 at 20:35:12.
(provided by the Example extension).