[Mesa-dev] [PATCH 5/8] main: enable NV_primitive_restart extension by default

[Marek Olšák]

On Fri, May 11, 2012 at 8:37 PM, Jordan Justen <jljusten at gmail.com> wrote:
> On Fri, May 11, 2012 at 10:49 AM, Ian Romanick <idr at freedesktop.org> wrote:
>> On 05/11/2012 10:35 AM, Marek Olšák wrote:
>>> On Fri, May 11, 2012 at 7:17 PM, Brian Paul<brianp at vmware.com>  wrote:
>>>> So setting MESA_EXTENSION_OVERRIDE=-GL_NV_primitive_restart should be an
>>>> option.  The o(dummy_true) convention might prevent that, but I haven't
>>>> tested that...
>>>
>>> In that case, I think we should disable it in the state tracker if
>>> there's no driver support. If it's faster for svga, it's very likely
>>> faster for the other DX9-level drivers too.
>>
>> I can imagine that it's certainly faster for hardware TNL cards. Mapping an
>> index buffer that's probably in VRAM, scanning it on the CPU, then
>> submitting multiple draw calls doesn't even sound like it would be fast.
>
> Drivers can disable software support in one of two ways:
> 1. gallium: enable PIPE_CAP_PRIMITIVE_RESTART
> 2. set ctx->Const.PrimitiveRestartInSoftware = GL_FALSE,
>   (similar to patch 3/8 which implements #1 for gallium)
>
> The driver can still call vbo_sw_primitive_restart if they want to
> take the software path.
>
>> It also seems like the pure software paths might want to handle it directly
>> in their index processing loop rather than chopping up the draw call before
>> hand.
>
> Do you mean it is better to not advertise the extension and let GL API
> users handle this?
>
>> The real value of these software paths is handling cases that the hardware
>> can't do.  The GL spec allows any value to be specified for the restart
>> index, but some hardware (*cough*) only supports ~0.
>
> So, what is the consensus here?

FWIW it seems to me after this discussion that we should not enable
the extension by default. Instead, we should let drivers enable both
the extension and the software fallback if they want it.

>
> I believe that state_tracker enables the NV_primitive_restart
> extension for all gallium drivers currently, and enables software
> support for drivers that don't advertise the support.

Yes, and not changing the behavior for gallium is okay for this patch series.

Marek