[Mesa-dev] [PATCH V3 1/2] i965/blorp: Add bilinear filtering of samples for multisample scaled blits

[Anuj Phogat]

>> >>
>> >> +void
>> >> +brw_blorp_blit_program::manual_blend_linear(unsigned num_samples)
>> >> +{
>> >> +   if (key->tex_layout == INTEL_MSAA_LAYOUT_CMS)
>> >> +      mcs_fetch();
>> >
>> >
>> > This won't work.  The MCS value we fetch has to match up with the pixel
>> > that we're sampling from.  Since this function samples from different pixels
>> > in each iteration of the "for" loop below, the call to mcs_fetch() needs to
>> > go inside the loop, and it needs to happen after storing the coordinates in
>> > X and Y.
>> >
>> I think MCS value fetch will not be required anymore as we are anyway
>> getting rid of optimization which compares mcs value to zero.
>
>
> MCS fetch is still needed, since the MCS value needs to be passed into the
> ld2dms message that is used to read the samples from the surface.

Yes, you are right. My piglit test case for scaled blitting uses multisample
framebuffer with texture attachment. Debugging the test case showed that
multisample textures use INTEL_MSAA_LAYOUT_UMS. That's the reason
test case continued passing even after deleting mcs fetch code.

So, I modified the test case to use multisample framebuffer with renderbuffer
attachment as well for testing. Test continued failing without mcs fetch code.
Test passed after moving the mcs_fetch() inside the 'for' loop just after
computing the pixel coordinates. I think this verifies that mcs_fetch() is
now happening correctly. You can find my piglit tests (blit-scaled-glsl.cpp
and negative-blit-scaled.cpp) at:
https://github.com/aphogat/piglit.git, branch: 'blit-3'

As suggested by you, I'll also try developing a more exhaustive test to verify
mcs fetch value. In the mean time, please take a look at my piglit test cases
and suggest if they're good enough to verify the implementation and enable
the extension on intel hardware with my latest patch (V5).

>>
>> >>
>> >> +
>> >> +   /* We do this computation by performing the following operations:
>> >> +    *
>> >> +    * In case of 4x, 8x MSAA:
>> >> +    * - Compute the pixel coordinates and sample numbers (a, b, c, d)
>> >> +    *   which are later used for interpolation
>> >> +    * - linearly interpolate samples a and b in X
>> >> +    * - linearly interpolate samples c and d in X
>> >> +    * - linearly interpolate the results of last two operations in Y
>> >> +    *
>> >> +    *   result = lrp(lrp(a + b) + lrp(c + d))
>> >> +    */
>> >> +   struct brw_reg Xp_f = retype(Xp, BRW_REGISTER_TYPE_F);
>> >> +   struct brw_reg Yp_f = retype(Yp, BRW_REGISTER_TYPE_F);
>> >> +   struct brw_reg t1_f = retype(t1, BRW_REGISTER_TYPE_F);
>> >> +   struct brw_reg t2_f = retype(t2, BRW_REGISTER_TYPE_F);
>> >> +
>> >> +   for (unsigned i = 0; i < 4; ++i) {
>> >> +      assert(i < ARRAY_SIZE(texture_data));
>> >> +      s_is_zero = false;
>> >> +
>> >> +      /* Compute pixel coordinates */
>> >> +      brw_ADD(&func, vec16(x_sample_coords), Xp_f,
>> >> +              brw_imm_f((float)(i & 0x1) * 0.5));
>> >> +      brw_ADD(&func, vec16(y_sample_coords), Yp_f,
>> >> +              brw_imm_f((float)(i & 0x2) * (1.0 / num_samples)));
>> >> +      brw_MOV(&func, vec16(X), x_sample_coords);
>> >> +      brw_MOV(&func, vec16(Y), y_sample_coords);
>> >> +