[Mesa-dev] [PATCH] i965: Change 8x multisample positions

[Jason Ekstrand]

On Thu, Aug 11, 2016 at 10:41 AM, Anuj Phogat <anuj.phogat at gmail.com> wrote:

> 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.
>

Did you try the scaled blit tests?  I bet we have to rework 8x scaled blit
for the new positions.

Thanks for working on this!  It's going to make the blorp transition way
easier.


> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> Cc: Jason Ekstrand <jason at jlekstrand.net>
> ---
>  src/mesa/drivers/dri/i965/brw_multisample_state.h | 33
> ++++++++---------------
>  1 file changed, 11 insertions(+), 22 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_multisample_state.h
> b/src/mesa/drivers/dri/i965/brw_multisample_state.h
> index 42a7fd3..78fb03a 100644
> --- a/src/mesa/drivers/dri/i965/brw_multisample_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_multisample_state.h
> @@ -46,20 +46,9 @@ 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:
>

Might be nice to reference the Vulkan spec here.


>   *
> - * 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
> @@ -70,17 +59,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.5.5
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160811/0f21f3d3/attachment-0001.html>