Re: [PATCH] wined3d: Do not try to use not available texture stages in set_tex_op_nvr

[Stefan Dösinger]

> - if (stage > (gl_info->limits.texture_stages-1))
> - {
> -
>     ERR("Texture stage %d invalid (only %d availables)\n", stage, gl_info->limits.texture_stages);
> -
>     return;
> - }

In which situations does this occur? I guess its on some older nvidia GPU.

I don't like the ERR here. If this occurs because an app ignores the d3d limits, then this should be a WARN. If this code is called because some other wined3d code doesn't honor the limits this should be fixed elsewhere. If this code is called correctly, and catching the limit violation in the state setter is the correct thing to do this should be a TRACE(e.g. the state handler is called because device creation dirtifies all states).

[PATCH] wined3d: Do not try to use not available texture stages in set_tex_op_nvr

[Christian Costa]

Stefan Dösinger a écrit :
> > - if (stage > (gl_info->limits.texture_stages-1))
> > - {
> > -
> >     ERR("Texture stage %d invalid (only %d availables)\n", stage, gl_info->limits.texture_stages);
> > -
> >     return;
> > - }
>
> In which situations does this occur? I guess its on some older nvidia GPU.
>
> I don't like the ERR here. If this occurs because an app ignores the d3d limits, then this should be a WARN. If this code is called because some other wined3d code doesn't honor the limits this should be fixed elsewhere. If this code is called correctly, and catching the limit violation in the state setter is the correct thing to do this should be a TRACE(e.g. the state handler is called because device creation dirtifies all states).

Yeah. A rather old one... ;-)

I don't know this code much but I would say it is partly 2 and 3. Is a 
FIXME suitable then ? This would be much more informative that just GL 
errors.

[PATCH] wined3d: Do not try to use not available texture stages in set_tex_op_nvr

[Stefan Dösinger]

> Yeah. A rather old one... ;-)
>
> I don't know this code much but I would say it is partly 2 and 3. Is a FIXME suitable then ? This would be much more informative that just GL errors.

I'd prefer if we could track down the real issue. There might still be some confusion between texture stages, number of simultanous textures and texture coordinates(currently we assume texture coordinates == texture stages, which is a bug in itself, but I think not the issue here)

A few questions that could provide hints:

Is a non 1:1 texture mapping used? Look for "Using non 1:1 mapping" WARNs

Does the call come from alpha or color? Color filters out unused stages and doesn't call set_tex_op_nvrc at all. Alpha doesn't do that because the colorop setup disables the texture unit, but this could cause GL errors, and I am not sure if it is correct with NVRC.

[PATCH] wined3d: Do not try to use not available texture stages in set_tex_op_nvr

[Henri Verbeet]

A +d3d,+d3d_caps log would probably be a good start.

[PATCH] wined3d: Do not try to use not available texture stages in set_tex_op_nvr

[Christian Costa]

Here it is. The fixme message is :
fixme:d3d:set_tex_op_nvrc >>>>>>>>>>>>>>>>> GL_INVALID_ENUM (0x500) from 
set_tex_op_nvrc() @ nvidia_texture_shader.c / 460

To answer Stefan, yes it's a 1:1 mapping.

Thanks for taking a look.

A+
Christian

Henri Verbeet a écrit :
> A +d3d,+d3d_caps log would probably be a good start.

[PATCH] wined3d: Do not try to use not available texture stages in set_tex_op_nvr

[Henri Verbeet]

2010/1/5 Christian Costa :
> Here it is. The fixme message is :
> fixme:d3d:set_tex_op_nvrc >>>>>>>>>>>>>>>>> GL_INVALID_ENUM (0x500) from
> set_tex_op_nvrc() @ nvidia_texture_shader.c / 460

Looks like the context creation code invalidates those states simply
because they exist. Does the attached patch make the errors go away?

[PATCH] wined3d: Do not try to use not available texture stages in set_tex_op_nvr

[Christian Costa]

Henri Verbeet a écrit :
> 2010/1/5 Christian Costa :
> > Here it is. The fixme message is :
> > fixme:d3d:set_tex_op_nvrc >>>>>>>>>>>>>>>>> GL_INVALID_ENUM (0x500) from
> > set_tex_op_nvrc() @ nvidia_texture_shader.c / 460
>
> Looks like the context creation code invalidates those states simply
> because they exist. Does the attached patch make the errors go away?

The fixmes disapear but I get tons of err:d3d:state_undefined Undefined 
state which is worst. Log attached.

[PATCH] wined3d: Do not try to use not available texture stages in set_tex_op_nvr

[Henri Verbeet]

2010/1/6 Christian Costa :
> The fixmes disapear but I get tons of err:d3d:state_undefined Undefined
> state which is worst. Log attached.

How does the attached patch work?

[PATCH] wined3d: Do not try to use not available texture stages in set_tex_op_nvr

[Christian Costa]

Henri Verbeet a écrit :
> 2010/1/6 Christian Costa :
> > The fixmes disapear but I get tons of err:d3d:state_undefined Undefined
> > state which is worst. Log attached.
>
> How does the attached patch work?

That's better but there are still "err:d3d:state_undefined Undefined 
state." errors.
6 in total. One for each unavailable stage.

[PATCH] wined3d: Do not try to use not available texture stages in set_tex_op_nvr

[Henri Verbeet]

2010/1/11 Christian Costa :
> That's better but there are still "err:d3d:state_undefined Undefined state."
> errors.
> 6 in total. One for each unavailable stage.

That's weird, it should hit the DebugBreak() in either
Context_MarkStateDirty() or IWineD3DDeviceImpl_MarkStateDirty() when
it tries to mark those states dirty.

[PATCH] wined3d: Do not try to use not available texture stages in set_tex_op_nvr

[Stefan Dösinger]

Am 11.01.2010 um 15:11 schrieb Henri Verbeet:
> 2010/1/11 Christian Costa :
> > That's better but there are still "err:d3d:state_undefined Undefined state."
> > errors.
> > 6 in total. One for each unavailable stage.
>
> That's weird, it should hit the DebugBreak() in either
> Context_MarkStateDirty() or IWineD3DDeviceImpl_MarkStateDirty() when
> it tries to mark those states dirty.

Some other state handler might call the state handler from the state table. Probably put a DebugBreak into state_indefined() and get a backtrace.