[Mesa-dev] [PATCH 2/2] i965/fs: Re-use output 0 for outputs without a static write.

[Paul Berry]

According to section 15.2.3 (Shader Outputs) of the GL 4.4 spec (and
similar text in older GL specs), failing to write to gl_FragData[n]
results in undefined data being sent to color buffer attachment n:

    "Writing to gl_FragColor specifies the fragment color (color
    number zero) that will be used by subsequent stages of the
    pipeline. Writing to gl_FragData[n] specifies the value of
    fragment color number n. Any colors, or color components,
    associated with a fragment that are not written by the fragment
    shader are undefined."

Prior to commit 6a2baf3 (glsl/linker: Make update_array_sizes apply to
just uniforms), if a fragment shader only wrote to gl_FragData[0], by
a lucky coincidence the data would be replicated to all color buffer
attachments.  Proprietary drivers for AMD and NVIDIA seem to have this
behaviour too, and at least one app (Dota 2) relies on it.

It is not all that surprising that an app writer might assume
gl_FragData[0] would behave this way, since for all intents and
purposes, gl_FragData[0] behaves like gl_FragColor, and gl_FragColor
*does* get replicated to all color buffer attachments.

So let's go ahead and implement this behaviour on purpose.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67887
---
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index ee7728c..31df57e 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -93,6 +93,9 @@ fs_visitor::visit(ir_variable *ir)
 
 	 /* General color output. */
 	 for (unsigned int i = 0; i < MAX2(1, ir->type->length); i++) {
+            /* Don't bother setting up registers for unused outputs */
+            if (!(fp->Base.OutputsWritten & BITFIELD64_BIT(ir->location + i)))
+               continue;
 	    int output = ir->location - FRAG_RESULT_DATA0 + i;
 	    this->outputs[output] = *reg;
 	    this->outputs[output].reg_offset += vector_elements * i;
@@ -2373,8 +2376,18 @@ fs_visitor::emit_fb_writes()
          write_color_mrf = color_mrf + reg_width;
       }
 
-      for (unsigned i = 0; i < this->output_components[target]; i++)
-         emit_color_write(target, i, write_color_mrf);
+      int output_src = target;
+      if (this->outputs[target].file == BAD_FILE) {
+         /* The shader doesn't have a static write to the given output, but it
+          * does have a static write to output 0.  The behaviour in this case
+          * is undefined, but some applications erroneously rely on output 0
+          * getting replicated to all render targets.  So just go ahead and
+          * write the data from output 0.
+          */
+         output_src = 0;
+      }
+      for (unsigned i = 0; i < this->output_components[output_src]; i++)
+         emit_color_write(output_src, i, write_color_mrf);
 
       bool eot = false;
       if (target == c->key.nr_color_regions - 1) {
-- 
1.8.3.4