[Piglit] [PATCH 2/3] all.tests: remove MSAA integer tests

[Marek Olšák]

On Mon, Oct 8, 2012 at 2:58 AM, Paul Berry <stereotype441 at gmail.com> wrote:
> On 7 October 2012 17:10, Marek Olšák <maraeo at gmail.com> wrote:
>>
>> On Mon, Oct 8, 2012 at 1:41 AM, Paul Berry <stereotype441 at gmail.com>
>> wrote:
>> > On 6 October 2012 18:49, Marek Olšák <maraeo at gmail.com> wrote:
>> >>
>> >> AFAIK, integer formats cannot be resolved, but only one sample is
>> >> chosen
>> >> and
>> >> copied to the destination. However the chosen sample might not be at
>> >> the
>> >> center
>> >> of the pixel, so the tests might never pass.
>> >
>> >
>> > According to my reading of the spec, integer format MSAA resolves should
>> > work fine--the samples should be averaged together, just as they are for
>> > fixed-point and floating-point formats.  So unless I'm misreading the
>> > spec,
>> > these tests should pass.  And in fact, on Intel hardware with Mesa they
>> > do.
>> >
>> > I'm really interested in keeping these tests around because on Intel
>> > hardware, the MSAA resolve is performed by a special-purpose shader
>> > that's
>> > generated on the fly by the driver.  That shader has to be subtly
>> > different
>> > when using integer formats (to avoid overflow, it has to use the integer
>> > AVG
>> > instruction, rather than floating point ADDs followed by a division).
>> > These
>> > piglit tests are, at the moment, the only thing verifying that we've
>> > handled
>> > the integer case correctly.
>> >
>> > Integer format MSAA is a rare, but not unheard of, use case (in fact, GL
>> > 3.2
>> > introduced a constant, GL_MAX_INTEGER_SAMPLES, to allow the driver to
>> > advertise its level of support for it).  So I'd really rather not drop
>> > these
>> > tests if we can avoid it.
>> >
>> > I'm guessing that you want to drop the tests because they're failing on
>> > a
>> > particular piece of hardware.  Have we ruled out the possibility of a
>> > driver
>> > bug?  If there's no driver bug (e.g. the tests are failing because of a
>> > hardware limitation), I'd recommend modifying the driver to report
>> > FRAMEBUFFER_UNSUPPORTED for those formats that don't work (we currently
>> > do
>> > this on Intel chipsets to work around some hardware bugs).  The
>> > multisample
>> > formats tests will automatically skip any formats that report
>> > FRAMEBUFFER_UNSUPPORTED.
>>
>> It's not a driver bug. The tests are failing because my hardware
>> cannot resolve integer textures. The format must be blendable for MSAA
>> resolving to work. The hardware can fetch from those textures just
>> fine though. I guess I can try to set the UNORM format for the source
>> and destination resources and see what happens. I am sceptic it will
>> work with the 32-bits-per-channel formats though.
>
>
> Ah, ok.  I wondered whether your hardware (r600, right?) had dedicated MSAA
> resolving logic built into it.  I'm envious :)  I'll be curious to hear if
> the UNORM idea works out.

Yes, r600. And yes, changing the format to UNORM works with 8-bit and
16-bit formats. For the 32-bit case, the hardware seems to only copy
one sample even though I told it to do MSAA resolve.

>
>>
>>
>> Anyway, I don't plan to implement MSAA resolving in the fragment shader.
>
>
> I don't blame you--I wouldn't have done MSAA resolving in the fragment
> shader on Intel chips if there had been an alternative.
>
> Assuming the UNORM idea doesn't work, are you ok with NAKing this patch and
> instead reporting FRAMEBUFFER_UNSUPPORTED for the formats where r600 can't
> do an MSAA resolve?  That would be best from my perspective, because it
> would allow the Intel logic to still be tested.

I am okay with NAKing the patch.

Marek