Re: [v2 1/2] wined3d: Implement primitive restart index.

[Józef Kucia]

On Fri, Nov 18, 2016 at 4:44 AM, Andrew Wesie  wrote:
> @@ -8602,6 +8602,16 @@ static void shader_glsl_init_context_state(struct wined3d_context *context)
>  gl_info->gl_ops.gl.p_glEnable(GL_PROGRAM_POINT_SIZE);
>  checkGLcall("GL_PROGRAM_POINT_SIZE");
> - if (gl_info->supported[ARB_ES3_COMPATIBILITY])
> - {
> -
>    /* We prefer this method because it correctly handles 16-bit and 32-bit indices. */
> -
>    gl_info->gl_ops.gl.p_glEnable(GL_PRIMITIVE_RESTART_FIXED_INDEX);
> -
>    checkGLcall("GL_PRIMITIVE_RESTART_FIXED_INDEX");
> - }

Is it really safe to enable the primitive restart for all D3D
versions? Shouldn't we enable it just for D3D10+? See, e.g.,
e47a735f762ebe1545bb366c7e9e2b90db7c7f80, if it's decided that this
should be enabled just for D3D10+.
> - else
> - {
> -
>    WARN("Missing support for primitive restart index.\n");
> - }

This is not important, but ideally this would be a FIXME() for D3D10+
feature level >= 10_0 and nothing otherwise.
> }
>
> static void shader_glsl_get_caps(const struct wined3d_gl_info *gl_info, struct shader_caps *caps)
> diff --git a/dlls/wined3d/wined3d_gl.h b/dlls/wined3d/wined3d_gl.h
> index b71190d..7bd2d6a 100644
> --- a/dlls/wined3d/wined3d_gl.h
> +++ b/dlls/wined3d/wined3d_gl.h
> @@ -55,6 +55,7 @@ enum wined3d_gl_extension
>      ARB_DRAW_ELEMENTS_BASE_VERTEX,
>      ARB_DRAW_INSTANCED,
>      ARB_ES2_COMPATIBILITY,
> - ARB_ES3_COMPATIBILITY,
> ARB_EXPLICIT_ATTRIB_LOCATION,
> ARB_FRAGMENT_COORD_CONVENTIONS,
> ARB_FRAGMENT_PROGRAM,

The extension has to be also added to the "core_extensions" table.

[v2 1/2] wined3d: Implement primitive restart index.

[Henri Verbeet]

On 21 November 2016 at 01:42, Józef Kucia  wrote:
> On Fri, Nov 18, 2016 at 4:44 AM, Andrew Wesie  wrote:
> > @@ -8602,6 +8602,16 @@ static void shader_glsl_init_context_state(struct wined3d_context *context)
> >  gl_info->gl_ops.gl.p_glEnable(GL_PROGRAM_POINT_SIZE);
> >  checkGLcall("GL_PROGRAM_POINT_SIZE");
> > - if (gl_info->supported[ARB_ES3_COMPATIBILITY])
> > - {
> > -
> >    /* We prefer this method because it correctly handles 16-bit and 32-bit indices. */
> > -
> >    gl_info->gl_ops.gl.p_glEnable(GL_PRIMITIVE_RESTART_FIXED_INDEX);
> > -
> >    checkGLcall("GL_PRIMITIVE_RESTART_FIXED_INDEX");
> > - }
>
> Is it really safe to enable the primitive restart for all D3D
> versions? Shouldn't we enable it just for D3D10+?

It's probably "safe" in the sense that if previous versions don't have
primitive restart, and that seems likely at this point, you'd probably
get undefined behaviour instead, and primitive restart is as good of
an undefined behaviour as any other. Unless it's a valid index of
course, which is certainly a possibility with 16-bit indices in
particular, if perhaps not likely.

But regardless, we want to know for sure.

[v2 1/2] wined3d: Implement primitive restart index.

[Józef Kucia]

On Tue, Nov 22, 2016 at 3:02 PM, Henri Verbeet  wrote:
> On 21 November 2016 at 01:42, Józef Kucia  wrote:
> > Is it really safe to enable the primitive restart for all D3D
> > versions? Shouldn't we enable it just for D3D10+?
>
> It's probably "safe" in the sense that if previous versions don't have
> primitive restart, and that seems likely at this point, you'd probably
> get undefined behaviour instead, and primitive restart is as good of
> an undefined behaviour as any other. Unless it's a valid index of
> course, which is certainly a possibility with 16-bit indices in
> particular, if perhaps not likely.
>
> But regardless, we want to know for sure.

I've written some tests and the results indicate that 0xffff is a
valid index in D3D8 and D3D9. I'll submit patches soon.