On Tue 18 Nov 2014, Chris Forbes wrote:
Two things were broken here:
- The depth/stencil surface dimensions were broken for MSAA.
- Sample count was programmed incorrectly.

Result was the depth resolve didn't work correctly on MSAA surfaces, and
so sampling the surface later produced garbage.

Fixes the new piglit test arb_texture_multisample-sample-depth, and
various artifacts in 'tesseract' with msaa=4 glineardepth=0.

Not observed any piglit regressions on Haswell.

v2: Just set brw_hiz_op_params::dst.num_samples rather than adding a
   helper function (Ken).

Signed-off-by: Chris Forbes <chr...@ijw.co.nz>
---
src/mesa/drivers/dri/i965/brw_blorp.cpp  |  1 +
src/mesa/drivers/dri/i965/gen7_blorp.cpp | 11 +++++++++++
2 files changed, 12 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp 
b/src/mesa/drivers/dri/i965/brw_blorp.cpp
index 20ce7b7..0ecf330 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp
@@ -323,6 +323,7 @@ brw_hiz_op_params::brw_hiz_op_params(struct 
intel_mipmap_tree *mt,
    */
   depth.width = ALIGN(depth.width, 8);
   depth.height = ALIGN(depth.height, 4);
+   dst.num_samples = mt->num_samples;

   x1 = depth.width;
   y1 = depth.height;
diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp 
b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
index 206a6ff..03fc9c8 100644
--- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
+++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
@@ -663,6 +663,17 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context 
*brw,
       */
      surfwidth = params->depth.width;
      surfheight = params->depth.height;
+
+      if (params->dst.num_samples > 1) {
+         /* If this is an MSAA + HIZ op, we need to program the
+          * aligned logical size of the depth surface.
+          */
+         surfwidth = ALIGN(params->depth.mt->logical_width0, 8);
+         surfheight = ALIGN(params->depth.mt->logical_height0, 4);
+      } else {
+         surfwidth = params->depth.width;
+         surfheight = params->depth.height;
+      }

I think the code that aligns up to 8x4 should go into hunk above that touches brw_hiz_op_params::brw_hiz_op_params(), because:

   1. That function already does the 8x4 alignment for the non-MSAA
      case. It doesn't make sense to do the 8x4 alignment for MSAA as
      a special case in a different file.
2. The lengthy comment that explains how alignment works and why it's needed is in that function. This is tricky delicate stuff, so let's put the special cases near the documentation.

   } else {
      surfwidth = params->depth.mt->logical_width0;
      surfheight = params->depth.mt->logical_height0;
--
2.1.3

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to