i965: Change 8x multisample positions

There are no standard sample positions defined in OpenGL and OpenGL
ES specs. Implementations have the freedom to pick the positions
which give plausible results. But the Vulkan 1.0 spec does define
standard sample positions for different sample counts. Defined
positions in Vulkan for all the sample counts except 8X match with
the positions we set in i965. We have an upcoming plan to share the
blorp code between OpenGL and Vulkan driver in near future. Keeping
the 8X sample positions same on both the drivers will help us move
in that direction.

Here is an argument by Neil Roberts (from commit 20250e85) against
any advantage of current 8X sample positions over the new ones:

"The comment above for the 8x sample positions says that the hardware
implements centroid interpolation by picking the centre-most sample
that is inside the primitive. That implies that it might be worthwhile
to pick a pattern that includes 0.5,0.5. However by experimentation
this doesn't seem to actually be the case. With the sample positions
in this patch, if I modify the piglit test below so that it instead
reports the centroid position, it reports 0.492188,0.421875 which
doesn't match any of the positions. If I modify the sample positions
so that they include one at exactly 0.5,0.5 it doesn't help and it
reports another position which is even further from the center for
some reason.

arb_gpu_shader5-interpolateAtSample-different

Kenneth Graunke experimented with some other patterns that have a
higher standard deviation but I think after some discussion it was
decided that it would be better to pick the same pattern as the other
graphics API in case there are games that rely on this pattern."

Observed no regressions in jenkins testing.

Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>

diff --git a/src/mesa/drivers/dri/i965/brw_multisample_state.h b/src/mesa/drivers/dri/i965/brw_multisample_state.h
index 42a7fd35121cb9a5ab4f8131322bb00a10b41768..db59af2affb3120901937d7e917e1c4eebfa7cd9 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_multisample_state.h
+++ b/src/mesa/drivers/dri/i965/brw_multisample_state.h
@@ -23,6 +23,15 @@
 
 #include <stdint.h>
 
+/**
+ * Note: There are no standard multisample positions defined in OpenGL
+ * specifications. Implementations have the freedom to pick the positions
+ * which give plausible results. But the Vulkan specification does define
+ * standard sample positions. So, we decided to pick the same pattern in
+ * OpenGL as in Vulkan to keep it uniform across drivers and also to avoid
+ * breaking applications which rely on this standard pattern.
+ */
+
 /**
  * 1x MSAA has a single sample at the center: (0.5, 0.5) -> (0x8, 0x8).
  *
@@ -46,22 +55,10 @@ static const uint32_t
 brw_multisample_positions_4x = 0xae2ae662;
 
 /**
- * Sample positions are based on a solution to the "8 queens" puzzle.
- * Rationale: in a solution to the 8 queens puzzle, no two queens share
- * a row, column, or diagonal.  This is a desirable property for samples
- * in a multisampling pattern, because it ensures that the samples are
- * relatively uniformly distributed through the pixel.
- *
- * There are several solutions to the 8 queens puzzle (see
- * http://en.wikipedia.org/wiki/Eight_queens_puzzle).  This solution was
- * chosen because it has a queen close to the center; this should
- * improve the accuracy of centroid interpolation, since the hardware
- * implements centroid interpolation by choosing the centermost sample
- * that overlaps with the primitive being drawn.
+ * Sample positions:
  *
- * Note: from the Ivy Bridge PRM, Vol2 Part1 p304 (3DSTATE_MULTISAMPLE:
+ * From the Ivy Bridge PRM, Vol2 Part1 p304 (3DSTATE_MULTISAMPLE:
  * Programming Notes):
- *
  *     "When programming the sample offsets (for NUMSAMPLES_4 or _8 and
  *     MSRASTMODE_xxx_PATTERN), the order of the samples 0 to 3 (or 7
  *     for 8X) must have monotonically increasing distance from the
@@ -70,17 +67,17 @@ brw_multisample_positions_4x = 0xae2ae662;
  *
  * Sample positions:
  *   1 3 5 7 9 b d f
- * 1     5
- * 3           2
- * 5               6
- * 7 4
- * 9       0
- * b             3
- * d         1
- * f   7
+ * 1               7
+ * 3     3
+ * 5         0
+ * 7 5
+ * 9             2
+ * b       1
+ * d   4
+ * f           6
  */
 static const uint32_t
-brw_multisample_positions_8x[] = { 0xdbb39d79, 0x3ff55117 };
+brw_multisample_positions_8x[] = { 0x53d97b95, 0xf1bf173d };
 
 /**
  * Sample positions: