From fb1bc5007deebfb1441a24ecacdfa11927ac9b12 Mon Sep 17 00:00:00 2001 From: Anuj Phogat Date: Tue, 9 Aug 2016 15:41:24 -0700 Subject: [PATCH] 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 Reviewed-by: Jason Ekstrand --- src/mesa/drivers/dri/i965/brw_multisample_state.h | 43 +++++++++++------------ 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_multisample_state.h b/src/mesa/drivers/dri/i965/brw_multisample_state.h index 42a7fd35121..db59af2affb 100644 --- a/src/mesa/drivers/dri/i965/brw_multisample_state.h +++ b/src/mesa/drivers/dri/i965/brw_multisample_state.h @@ -24,6 +24,15 @@ #include /** + * 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). * * 2x MSAA sample positions are (0.25, 0.25) and (0.75, 0.75): @@ -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: -- 2.11.0