Re: [PATCH 5/5] wined3d: Always enable GL_PROGRAM_POINT_SIZE in the GLSL backend.

[Henri Verbeet]

On 28 May 2015 at 23:23, Matteo Bruni  wrote:
> @@ -6987,6 +7006,13 @@ static void shader_glsl_select(void *shader_priv, struct wined3d_context *contex
>      GLuint program_id = 0, prev_id = 0;
>      GLenum old_vertex_color_clamp, current_vertex_color_clamp;
> - if (!ctx_data->backend_init_done)
> - {
> -
>    gl_info->gl_ops.gl.p_glEnable(GL_PROGRAM_POINT_SIZE);
> -
>    checkGLcall("GL_PROGRAM_POINT_SIZE");
> -
>    ctx_data->backend_init_done = TRUE;
> - }
> -

Is this shared state between contexts? Otherwise you need to do this
once for every context. Also, shader_glsl_select() is kind of a hot
path, it may be better to just add an entry to struct
wined3d_shader_backend_ops for this kind of initialization.

[PATCH 5/5] wined3d: Always enable GL_PROGRAM_POINT_SIZE in the GLSL backend.

[Matteo Bruni]

2015-05-28 23:43 GMT+02:00 Henri Verbeet :
> On 28 May 2015 at 23:23, Matteo Bruni  wrote:
> > @@ -6987,6 +7006,13 @@ static void shader_glsl_select(void *shader_priv, struct wined3d_context *contex
> >      GLuint program_id = 0, prev_id = 0;
> >      GLenum old_vertex_color_clamp, current_vertex_color_clamp;
> > - if (!ctx_data->backend_init_done)
> > - {
> > -
> >    gl_info->gl_ops.gl.p_glEnable(GL_PROGRAM_POINT_SIZE);
> > -
> >    checkGLcall("GL_PROGRAM_POINT_SIZE");
> > -
> >    ctx_data->backend_init_done = TRUE;
> > - }
> > -
>
> Is this shared state between contexts? Otherwise you need to do this
> once for every context. Also, shader_glsl_select() is kind of a hot
> path, it may be better to just add an entry to struct
> wined3d_shader_backend_ops for this kind of initialization.

It's not shared but it should already be executed for each context
(the flag is stored in struct wined3d_context).
It would be surprising to me if that additional flag check has any
performance impact since it is next to ctx_data->glsl_program which is
accessed anyway just a bit later. It's probably cleaner to add a
separate function though so I'm going to try that.

[PATCH 5/5] wined3d: Always enable GL_PROGRAM_POINT_SIZE in the GLSL backend.

[Henri Verbeet]

On 28 May 2015 at 23:53, Matteo Bruni  wrote:
> It's not shared but it should already be executed for each context
> (the flag is stored in struct wined3d_context).

I may have been a bit too quick to write a reply. :)
> It would be surprising to me if that additional flag check has any
> performance impact since it is next to ctx_data->glsl_program which is
> accessed anyway just a bit later.

Yeah, I suppose, although it still seems better to not add unnecessary
code in that path. Something similar applies to struct
vs_compile_args, the two extra BOOLs probably don't hurt too much, but
it seems better to merge them with "clip_enabled" as flags.