Bug 94847

Summary:	[ES3.1CTS] es31-cts.draw_buffers_indexed.color_masks fails
Product:	Mesa	Reporter:	Mark Janes <mark.a.janes>
Component:	Drivers/DRI/i965	Assignee:	Kenneth Graunke <kenneth>
Status:	RESOLVED FIXED	QA Contact:	Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity:	normal
Priority:	medium	CC:	jljusten, kenneth
Version:	git
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Bug Depends on:
Bug Blocks:	92778

Description Mark Janes 2016-04-06 16:49:09 UTC

This conformance test began failing with:

mesa e0e16830873b945a24880ae515466bf7f9165f42 
mesa: add GL_OES/EXT_draw_buffers_indexed support

This is the same ext as ARB_draw_buffers_blend (plus some core
functionality that already exists). Add the alias entrypoints.

Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Dave Airlie <airlied@redhat.com>

Based on the commit message, the test probably skipped before the extension was enabled.  However, it must be fixed.

Comment 1 Mark Janes 2016-04-06 17:30:32 UTC

This bug affects BDW, BSW, KBL, SKL.  Those are all the platforms tested for ES3.1.

Comment 2 Kenneth Graunke 2016-04-06 21:34:54 UTC

I believe I've tracked down the problem.

The application is binding multiple draw buffers.  Each draw buffer has associated color mask and blending state.  (In Mesa, ctx->Color.ColorMask[i] corresponds to the color mask for GL_DRAW_BUFFERi.)

It then calls glClearBufferfv to clear an individual draw buffer.  This is handled by _mesa_meta_glsl_Clear (fast clears are skipped due to color masking).  In order to clear a single draw buffer, _mesa_meta_drawbuffers_from_bitfield() calls glDrawBuffers() to try and disable any attachments that we don't want to clear.  It also leaves the color masking in place, because it should take effect.

However, this has a problem: it binds all the same color attachments, but in a different order.  So, what was GL_DRAW_BUFFER5 will now be bound as GL_DRAW_BUFFER0.  This means that Meta uses ctx->Color.ColorMask[0] instead of ctx->Color.ColorMask[5]...which is the wrong state.

Comment 3 Kenneth Graunke 2016-04-07 02:02:31 UTC

Although the problem is fairly simple, trying to set up drawbuffers correctly is immensely painful.  I'm not currently working on a fix, so if someone else would like to step up and do it, that would be fantastic.

It might be worth refactoring the core Mesa color drawbuffer data structures, as they can be pretty awful to work with.  Of course, if someone manages to find a solution without doing that...then we can limp along and put that off for another day.

Comment 4 Kenneth Graunke 2016-04-12 20:42:35 UTC

I had a bit of inspiration today and ended up fixing it:

https://lists.freedesktop.org/archives/mesa-dev/2016-April/112536.html
https://lists.freedesktop.org/archives/mesa-dev/2016-April/112537.html

Comment 5 Matt Turner 2016-04-25 20:02:37 UTC

I think Ken fixed this with

commit c092f9b96a0bdc415fec1760b8b345166b115483
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Tue Apr 12 10:19:09 2016 -0700

    meta: Don't botch color masks when changing drawbuffers.

Ken, please confirm.

Comment 6 Kenneth Graunke 2016-04-25 23:31:54 UTC

Yep, sorry, I forgot to close this.  Since then we've also switched away from that code entirely in favor of BLORP, which also handles this correctly.

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.