i965/msaa: Fix centroid interpolation of unlit pixels.

From the Ivy Bridge PRM, Vol 2 Part 1 p280-281 (3DSTATE_WM:
Barycentric Interpolation Mode):

    "Errata: When Centroid Barycentric mode is required, HW may
    produce incorrect interpolation results when a 2X2 pixels have
    unlit pixels."

To work around this problem, after doing centroid interpolation, we
replace the centroid-interpolated values for unlit pixels with
non-centroid-interpolated values (which are interpolated at pixel
centers).  This produces correct rendering at the expense of a slight
increase in shader execution time.

I've conditioned the workaround with a runtime flag
(brw->needs_unlit_centroid_workaround) in the hopes that we won't need
it in future chip generations.

Fixes piglit tests "EXT_framebuffer_multisample/interpolation {2,4}
{centroid-deriv,centroid-deriv-disabled}".  All MSAA interpolation
tests pass now.

Reviewed-by: Eric Anholt <eric@anholt.net>
This commit is contained in:
Paul Berry 2012-06-21 11:21:22 -07:00
parent 3f929efa28
commit 8313f44409
4 changed files with 39 additions and 4 deletions

View file

@ -296,6 +296,10 @@ brwCreateContext(int api,
brw->has_negative_rhw_bug = true; brw->has_negative_rhw_bug = true;
} }
if (intel->gen <= 7) {
brw->needs_unlit_centroid_workaround = true;
}
brw->prim_restart.in_progress = false; brw->prim_restart.in_progress = false;
brw->prim_restart.enable_cut_index = false; brw->prim_restart.enable_cut_index = false;

View file

@ -725,6 +725,15 @@ struct brw_context
bool has_pln; bool has_pln;
bool precompile; bool precompile;
/**
* Some versions of Gen hardware don't do centroid interpolation correctly
* on unlit pixels, causing incorrect values for derivatives near triangle
* edges. Enabling this flag causes the fragment shader to use
* non-centroid interpolation for unlit pixels, at the expense of two extra
* fragment shader instructions.
*/
bool needs_unlit_centroid_workaround;
struct { struct {
struct brw_state_flags dirty; struct brw_state_flags dirty;
} state; } state;

View file

@ -506,6 +506,18 @@ fs_visitor::emit_general_interpolation(ir_variable *ir)
struct brw_reg interp = interp_reg(location, k); struct brw_reg interp = interp_reg(location, k);
emit_linterp(attr, fs_reg(interp), interpolation_mode, emit_linterp(attr, fs_reg(interp), interpolation_mode,
ir->centroid); ir->centroid);
if (brw->needs_unlit_centroid_workaround && ir->centroid) {
/* Get the pixel/sample mask into f0 so that we know
* which pixels are lit. Then, for each channel that is
* unlit, replace the centroid data with non-centroid
* data.
*/
emit(FS_OPCODE_MOV_DISPATCH_TO_FLAGS, attr);
fs_inst *inst = emit_linterp(attr, fs_reg(interp),
interpolation_mode, false);
inst->predicated = true;
inst->predicate_inverse = true;
}
if (intel->gen < 6) { if (intel->gen < 6) {
emit(BRW_OPCODE_MUL, attr, attr, this->pixel_w); emit(BRW_OPCODE_MUL, attr, attr, this->pixel_w);
} }

View file

@ -130,7 +130,8 @@ brw_wm_non_glsl_emit(struct brw_context *brw, struct brw_wm_compile *c)
* (see enum brw_wm_barycentric_interp_mode) is needed by the fragment shader. * (see enum brw_wm_barycentric_interp_mode) is needed by the fragment shader.
*/ */
static unsigned static unsigned
brw_compute_barycentric_interp_modes(bool shade_model_flat, brw_compute_barycentric_interp_modes(struct brw_context *brw,
bool shade_model_flat,
const struct gl_fragment_program *fprog) const struct gl_fragment_program *fprog)
{ {
unsigned barycentric_interp_modes = 0; unsigned barycentric_interp_modes = 0;
@ -154,11 +155,18 @@ brw_compute_barycentric_interp_modes(bool shade_model_flat,
if (attr == FRAG_ATTRIB_WPOS || attr == FRAG_ATTRIB_FACE) if (attr == FRAG_ATTRIB_WPOS || attr == FRAG_ATTRIB_FACE)
continue; continue;
/* Determine the set (or sets) of barycentric coordinates needed to
* interpolate this variable. Note that when
* brw->needs_unlit_centroid_workaround is set, centroid interpolation
* uses PIXEL interpolation for unlit pixels and CENTROID interpolation
* for lit pixels, so we need both sets of barycentric coordinates.
*/
if (interp_qualifier == INTERP_QUALIFIER_NOPERSPECTIVE) { if (interp_qualifier == INTERP_QUALIFIER_NOPERSPECTIVE) {
if (is_centroid) { if (is_centroid) {
barycentric_interp_modes |= barycentric_interp_modes |=
1 << BRW_WM_NONPERSPECTIVE_CENTROID_BARYCENTRIC; 1 << BRW_WM_NONPERSPECTIVE_CENTROID_BARYCENTRIC;
} else { }
if (!is_centroid || brw->needs_unlit_centroid_workaround) {
barycentric_interp_modes |= barycentric_interp_modes |=
1 << BRW_WM_NONPERSPECTIVE_PIXEL_BARYCENTRIC; 1 << BRW_WM_NONPERSPECTIVE_PIXEL_BARYCENTRIC;
} }
@ -168,7 +176,8 @@ brw_compute_barycentric_interp_modes(bool shade_model_flat,
if (is_centroid) { if (is_centroid) {
barycentric_interp_modes |= barycentric_interp_modes |=
1 << BRW_WM_PERSPECTIVE_CENTROID_BARYCENTRIC; 1 << BRW_WM_PERSPECTIVE_CENTROID_BARYCENTRIC;
} else { }
if (!is_centroid || brw->needs_unlit_centroid_workaround) {
barycentric_interp_modes |= barycentric_interp_modes |=
1 << BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC; 1 << BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC;
} }
@ -289,7 +298,8 @@ bool do_wm_prog(struct brw_context *brw,
brw_init_compile(brw, &c->func, c); brw_init_compile(brw, &c->func, c);
c->prog_data.barycentric_interp_modes = c->prog_data.barycentric_interp_modes =
brw_compute_barycentric_interp_modes(c->key.flat_shade, &fp->program); brw_compute_barycentric_interp_modes(brw, c->key.flat_shade,
&fp->program);
if (prog && prog->_LinkedShaders[MESA_SHADER_FRAGMENT]) { if (prog && prog->_LinkedShaders[MESA_SHADER_FRAGMENT]) {
if (!brw_wm_fs_emit(brw, c, prog)) if (!brw_wm_fs_emit(brw, c, prog))