On Thu, Aug 11, 2016 at 10:53 AM, Jason Ekstrand <ja...@jlekstrand.net> wrote: > > > On Thu, Aug 11, 2016 at 10:41 AM, Anuj Phogat <anuj.pho...@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. > They all passed because I missed changing sample mapping pattern in blorp and the piglit tests. So, the incorrect test image perfectly matched the incorrect reference image. I'll send out patches to fix these issues.
> Thanks for working on this! It's going to make the blorp transition way > easier. > >> >> Signed-off-by: Anuj Phogat <anuj.pho...@gmail.com> >> Cc: Jason Ekstrand <ja...@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. I'll add the Vulkan reference 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 >> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
