Re: [PATCH v2 1/8] wined3d: Alpha test emulation for core profile contexts.

[Henri Verbeet]

On 19 April 2016 at 18:56, Matteo Bruni  wrote:
> - /* alpha_func is the PASS condition, not the DISCARD condition. Instead of
> -
> * flipping all the operators here, just negate the comparison below. */

As an aside, note that in general inverting operators isn't safe if
there are potential NaNs involved.
> - static const char *comparison_operator[] =
> - {
> -
>    "",   /* WINED3D_CMP_NEVER */
> -
>    "<",  /* WINED3D_CMP_LESS */
> -
>    "==", /* WINED3D_CMP_EQUAL */
> -
>    "<=", /* WINED3D_CMP_LESSEQUAL */
> -
>    ">",  /* WINED3D_CMP_GREATER */
> -
>    "!=", /* WINED3D_CMP_NOTEQUAL */
> -
>    ">=", /* WINED3D_CMP_GREATEREQUAL */
> -
>    ""    /* WINED3D_CMP_ALWAYS */
> - };

I should have noticed the first time, but I think you'll want to make
the string pointers const as well.
> @@ -3091,6 +3091,13 @@ void find_ps_compile_args(const struct wined3d_state *state, const struct wined3
>      args->pointsprite = state->render_states[WINED3D_RS_POINTSPRITEENABLE]
>              && state->gl_primitive_type == GL_POINTS;
> - if (gl_info->supported[WINED3D_GL_LEGACY_CONTEXT])
> -
>    args->alpha_test_func = WINED3D_CMP_ALWAYS - 1;
> - else
> -
>    args->alpha_test_func = (state->render_states[WINED3D_RS_ALPHATESTENABLE]
> -
>            ? wined3d_sanitize_cmp_func(state->render_states[WINED3D_RS_ALPHAFUNC])
> -
>            : WINED3D_CMP_ALWAYS) - 1;

For what it's worth, what I originally had in mind was mapping
WINED3D_CMP_ALWAYS to 0. I.e., masking with 7. This works as well.

[PATCH v2 1/8] wined3d: Alpha test emulation for core profile contexts.

[Matteo Bruni]

2016-04-20 12:50 GMT+02:00 Henri Verbeet :
> On 19 April 2016 at 18:56, Matteo Bruni  wrote:
> > - /* alpha_func is the PASS condition, not the DISCARD condition. Instead of
> > -
> > * flipping all the operators here, just negate the comparison below. */
>
> As an aside, note that in general inverting operators isn't safe if
> there are potential NaNs involved.

I'm not sure that matter here, although I can certainly swap the
operators and drop the negation.
> > - static const char *comparison_operator[] =
> > - {
> > -
> >    "",   /* WINED3D_CMP_NEVER */
> > -
> >    "<",  /* WINED3D_CMP_LESS */
> > -
> >    "==", /* WINED3D_CMP_EQUAL */
> > -
> >    "<=", /* WINED3D_CMP_LESSEQUAL */
> > -
> >    ">",  /* WINED3D_CMP_GREATER */
> > -
> >    "!=", /* WINED3D_CMP_NOTEQUAL */
> > -
> >    ">=", /* WINED3D_CMP_GREATEREQUAL */
> > -
> >    ""    /* WINED3D_CMP_ALWAYS */
> > - };
>
> I should have noticed the first time, but I think you'll want to make
> the string pointers const as well.

Right, will fix.
> > @@ -3091,6 +3091,13 @@ void find_ps_compile_args(const struct wined3d_state *state, const struct wined3
> >      args->pointsprite = state->render_states[WINED3D_RS_POINTSPRITEENABLE]
> >              && state->gl_primitive_type == GL_POINTS;
> > - if (gl_info->supported[WINED3D_GL_LEGACY_CONTEXT])
> > -
> >    args->alpha_test_func = WINED3D_CMP_ALWAYS - 1;
> > - else
> > -
> >    args->alpha_test_func = (state->render_states[WINED3D_RS_ALPHATESTENABLE]
> > -
> >            ? wined3d_sanitize_cmp_func(state->render_states[WINED3D_RS_ALPHAFUNC])
> > -
> >            : WINED3D_CMP_ALWAYS) - 1;
>
> For what it's worth, what I originally had in mind was mapping
> WINED3D_CMP_ALWAYS to 0. I.e., masking with 7. This works as well.

Ah, right. I can mask WINED3D_CMP_ALWAYS instead if you prefer
(although maybe in that case converting back to enum wined3d_cmp_func
would be a bit more convoluted?)

[PATCH v2 1/8] wined3d: Alpha test emulation for core profile contexts.

[Henri Verbeet]

On 20 April 2016 at 17:52, Matteo Bruni  wrote:
> 2016-04-20 12:50 GMT+02:00 Henri Verbeet :
> > On 19 April 2016 at 18:56, Matteo Bruni  wrote:
> > > - /* alpha_func is the PASS condition, not the DISCARD condition. Instead of
> > > -
> > > * flipping all the operators here, just negate the comparison below. */
> >
> > As an aside, note that in general inverting operators isn't safe if
> > there are potential NaNs involved.
>
> I'm not sure that matter here, although I can certainly swap the
> operators and drop the negation.

No, I think it's correct like this. It was more of a general note that
e.g. "pass(x < y);" isn't necessarily equivalent to "fail(x >= y);",
while "fail(!(x < y));" is.
> > > @@ -3091,6 +3091,13 @@ void find_ps_compile_args(const struct wined3d_state *state, const struct wined3
> > >      args->pointsprite = state->render_states[WINED3D_RS_POINTSPRITEENABLE]
> > >              && state->gl_primitive_type == GL_POINTS;
> > > - if (gl_info->supported[WINED3D_GL_LEGACY_CONTEXT])
> > > -
> > >    args->alpha_test_func = WINED3D_CMP_ALWAYS - 1;
> > > - else
> > > -
> > >    args->alpha_test_func = (state->render_states[WINED3D_RS_ALPHATESTENABLE]
> > > -
> > >            ? wined3d_sanitize_cmp_func(state->render_states[WINED3D_RS_ALPHAFUNC])
> > > -
> > >            : WINED3D_CMP_ALWAYS) - 1;
> >
> > For what it's worth, what I originally had in mind was mapping
> > WINED3D_CMP_ALWAYS to 0. I.e., masking with 7. This works as well.
>
> Ah, right. I can mask WINED3D_CMP_ALWAYS instead if you prefer
> (although maybe in that case converting back to enum wined3d_cmp_func
> would be a bit more convoluted?)

I think it shouldn't be too bad, but it's not something I care strongly about.