<p dir="ltr"><br>
On Jul 9, 2015 7:57 AM, "Francisco Jerez" <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>> wrote:<br>
><br>
> We were passing src0 alpha and oMask in reverse order.  There seems to<br>
> be no good way to pass them in the correct order to the new-style<br>
> LOAD_PAYLOAD (how surprising) because src0 alpha is per-channel while<br>
> oMask is not.  Just split src0 alpha in fixed-width registers and pass<br>
> them to LOAD_PAYLOAD as if they were part of the header as work-around<br>
> for now.</p>
<p dir="ltr">Bah... I came across this when I did the LOAD_PAYLOAD rework but thought it was only theoretical.  I wasn't very familiar with what omask actually did and, since piglit didn't hit it, I wasn't sure if it was a real problem or not.  I probably should have done more digging and written a piglit test at the time. My bad.</p>
<p dir="ltr">One solution that I proposed at the time was to turn header_size into header_mask in the obvious way. We can still use 8 bits because we should never have a header source higher than 8.</p>
<p dir="ltr">Thoughts?<br>
--Jason</p>
<p dir="ltr">> I've written a piglit test that demonstrates the problem by using<br>
> gl_SampleMask from a fragment shader with multiple color outputs [1].<br>
><br>
> [1] <a href="http://lists.freedesktop.org/archives/piglit/2015-July/016499.html">http://lists.freedesktop.org/archives/piglit/2015-July/016499.html</a><br>
> ---<br>
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 26 +++++++++++++++++---------<br>
>  1 file changed, 17 insertions(+), 9 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp<br>
> index 94d6a58..304ae74 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp<br>
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp<br>
> @@ -1535,6 +1535,19 @@ fs_visitor::emit_single_fb_write(const fs_builder &bld,<br>
>        length++;<br>
>     }<br>
><br>
> +   if (src0_alpha.file != BAD_FILE && color0.file != BAD_FILE) {<br>
> +      /* Neat, we need to chop the src0 alpha component and pass it as part of<br>
> +       * the header even though it has per-channel semantics, because the next<br>
> +       * optional field is header-like and LOAD_PAYLOAD requires all such<br>
> +       * fields to form a contiguous segment at the beginning of the message.<br>
> +       */<br>
> +      for (unsigned i = 0; i < exec_size / 8; i++) {<br>
> +         setup_color_payload(&sources[length], src0_alpha, 1, 8,<br>
> +                             use_2nd_half || i == 1);<br>
> +         length++;<br>
> +      }<br>
> +   }<br>
> +<br>
>     prog_data->uses_omask =<br>
>        prog->OutputsWritten & BITFIELD64_BIT(FRAG_RESULT_SAMPLE_MASK);<br>
>     if (prog_data->uses_omask) {<br>
> @@ -1561,19 +1574,14 @@ fs_visitor::emit_single_fb_write(const fs_builder &bld,<br>
>                               offset(this->outputs[0], bld, 3),<br>
>                               1, exec_size, false);<br>
>        length += 4;<br>
> -   } else if (color1.file == BAD_FILE) {<br>
> -      if (src0_alpha.file != BAD_FILE) {<br>
> -         setup_color_payload(&sources[length], src0_alpha, 1, exec_size, false);<br>
> -         length++;<br>
> -      }<br>
> -<br>
> -      setup_color_payload(&sources[length], color0, components,<br>
> -                          exec_size, use_2nd_half);<br>
> -      length += 4;<br>
>     } else {<br>
>        setup_color_payload(&sources[length], color0, components,<br>
>                            exec_size, use_2nd_half);<br>
>        length += 4;<br>
> +<br>
> +   }<br>
> +<br>
> +   if (color1.file != BAD_FILE) {<br>
>        setup_color_payload(&sources[length], color1, components,<br>
>                            exec_size, use_2nd_half);<br>
>        length += 4;<br>
> --<br>
> 2.4.3<br>
><br>
</p>