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.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.