[Mesa-dev] [PATCH 15/15] intel: Advertise multisample DRI2 configs on gen >= 6

[Chad Versace]

On 08/06/2012 07:49 PM, Eric Anholt wrote:
> Chad Versace <chad.versace at linux.intel.com> writes:
>> +   /* Generate multisample configs.
>> +    *
>> +    * This loop breaks early, and hence is a no-op, on gen < 6.
>> +    *
>> +    * Multisample configs must follow the singlesample configs in order to
>> +    * work around an X server bug present in 1.12. The X server chooses to
>> +    * associate the first listed RGBA888-Z24S8 config, regardless of its
>> +    * sample count, with the 32-bit depth visual used for compositing.
>> +    *
>> +    * Only doublebuffer configs with GLX_SWAP_UNDEFINED_OML behavior are
>> +    * supported. Singlebuffer configs are not supported because that would
>> +    * require that rendering be eventually written to the singlesample buffer
>> +    * even if DRI2Flush is never called; yet we downsample to the singlesample
>> +    * buffer only on DRI2Flush.  GLX_SWAP_COPY_OML is not supported because we
>> +    * have no tests for its interaction with MSAA.
>> +    */
> 
> We actually need to remove our claiming of GLX_SWAP_COPY_OML in general,
> because pageflipping means that we don't actually support SWAP_COPY.  We
> only do UNDEFINED.
> 
> I'd say instead "singlebuffer configs are not supported because nobody
> wants them".

I'll update the comments to say:

    * Only doublebuffer configs with GLX_SWAP_UNDEFINED_OML behavior are
    * supported.  Singlebuffer configs are not supported because no one wants
    * them. GLX_SWAP_COPY_OML is not supported due to page flipping.
    */

I'll follow up later with a patch that removes advertising of SWAP_COPY configs.

> I think all you need is (pessimistically) intel_downsample_for_dri2_flush
> in intel_flush_front() to make front buffer rendering actually work, and
> it's a problem that exists even in a doublebuffer config.

Ah, I failed to realize that this problem exists even in a doublebuffer
config. I'll follow up later with a patch that downsamples in intel_flush_front.

> I think this concludes my review.  Great work!  I'm excited to see this
> finally land.

Thanks. Is this an implicit r-b on this patch?