Re: [PATCH] wined3d: Display state in state_undefined
On 16 February 2010 11:49, Christian Costa <titan.costa(a)wanadoo.fr> wrote:
- ERR("Undefined state.\n"); + ERR("Undefined state %d\n", state); Did you test this? "state" is supposed to be always 0. Also, debug_d3dstate() is much more readable than the state number.
Henri Verbeet a écrit :
On 16 February 2010 11:49, Christian Costa <titan.costa(a)wanadoo.fr> wrote:
- ERR("Undefined state.\n"); + ERR("Undefined state %d\n", state);
Did you test this? "state" is supposed to be always 0. Also, debug_d3dstate() is much more readable than the state number.
Yes I tested it. Indeed, it's 0. Ok. I will send a new patch using debug_d3dstate. Thanks. Christian
On 16 February 2010 12:25, Christian Costa <titan.costa(a)wanadoo.fr> wrote:
Did you test this? "state" is supposed to be always 0. Also, debug_d3dstate() is much more readable than the state number.
Yes I tested it. Indeed, it's 0. Ok. I will send a new patch using debug_d3dstate. Thanks.
That also means there's really no point in printing it.
Henri Verbeet a écrit :
On 16 February 2010 12:25, Christian Costa <titan.costa(a)wanadoo.fr> wrote:
Did you test this? "state" is supposed to be always 0. Also, debug_d3dstate() is much more readable than the state number.
Yes I tested it. Indeed, it's 0. Ok. I will send a new patch using debug_d3dstate. Thanks.
That also means there's really no point in printing it.
Ok. What about doing the following : - when state is 0, don't print any message because it's normal - when state is not 0, print the state to have a meaningfull message
On 16 February 2010 13:45, Christian Costa <titan.costa(a)wanadoo.fr> wrote:
Ok. What about doing the following : - when state is 0, don't print any message because it's normal - when state is not 0, print the state to have a meaningfull message
No, if you see that message at all, something is broken, that's why it's an ERR. If state is non-zero even more is broken, but that's pretty much irrelevant at that point.
Henri Verbeet a écrit :
On 16 February 2010 13:45, Christian Costa <titan.costa(a)wanadoo.fr> wrote:
Ok. What about doing the following : - when state is 0, don't print any message because it's normal - when state is not 0, print the state to have a meaningfull message
No, if you see that message at all, something is broken, that's why it's an ERR. If state is non-zero even more is broken, but that's pretty much irrelevant at that point.
So we should never enter this function at all. Right ? And what do you mean by "something is broken" ? Are you talking about wined3d code ? That said I don't understand why we should not display the state. The fact is we entered this code and it seems, according to you, that knowing the state which produces the error message makes the difference.
On 16 February 2010 19:32, Christian Costa <titan.costa(a)wanadoo.fr> wrote:
So we should never enter this function at all. Right ? Yes.
And what do you mean by "something is broken" ? Are you talking about wined3d code ? It means there's code that marks a state dirty that doesn't exist. Typically that's because the code doing that should check some limit or if certain features are available.
That said I don't understand why we should not display the state. The fact is we entered this code and it seems, according to you, that knowing the state which produces the error message makes the difference.
Yes, but you can't check that here, the state is always 0. You should check "rep" for zero in IWineD3DDeviceImpl_MarkStateDirty() and Context_MarkStateDirty(), and get a backtrace if it is. I.e., something like the attached patch. We can't add DebugBreak()'s like that to wined3d, and often the error is mostly harmless, but I've been thinking about adding extra validation code like that for e.g. the state table and locking behind some debugging define in order to share debugging/validation code.
Henri Verbeet a écrit :
On 16 February 2010 19:32, Christian Costa <titan.costa(a)wanadoo.fr> wrote:
So we should never enter this function at all. Right ?
Yes.
And what do you mean by "something is broken" ? Are you talking about wined3d code ?
It means there's code that marks a state dirty that doesn't exist. Typically that's because the code doing that should check some limit or if certain features are available.
That said I don't understand why we should not display the state. The fact is we entered this code and it seems, according to you, that knowing the state which produces the error message makes the difference.
Yes, but you can't check that here, the state is always 0. You should check "rep" for zero in IWineD3DDeviceImpl_MarkStateDirty() and Context_MarkStateDirty(), and get a backtrace if it is. I.e., something like the attached patch. We can't add DebugBreak()'s like that to wined3d, and often the error is mostly harmless, but I've been thinking about adding extra validation code like that for e.g. the state table and locking behind some debugging define in order to share debugging/validation code.
That's clearer. Thanks. I've just tested your patch. The incriminated state is WINED3DRS_TWEENFACTOR. I haven't investigated much though...
On 16 February 2010 21:55, Christian Costa <titan.costa(a)wanadoo.fr> wrote:
That's clearer. Thanks. I've just tested your patch. The incriminated state is WINED3DRS_TWEENFACTOR. I haven't investigated much though...
Tweening is indeed unimplemented. We should probably catch unimplemented render states in IWineD3DDeviceImpl_SetRenderState() though.
On Tuesday 16 February 2010 22:18:50 Henri Verbeet wrote:
Tweening is indeed unimplemented. We should probably catch unimplemented render states in IWineD3DDeviceImpl_SetRenderState() though. No, the pipeline part that would implement this(vertex) should set a state handler that prints a WARN for each defined but unimplemented stage. SetRenderState is not a good place for this, for the following reasons:
* Some pipelines implement some states while others do not * Some pipelines implement states depending on some GL extensions(e.g. WINED3DRS_BLENDFACTOR) * The pipeline interface already provides an indirection layer that allows us to catch unsupported state sets without a performance penalty. Originally I intended to catch unsupported states with representative 0, but that had other issues, so we'll have to explicitly mark unsupported states
On 17 February 2010 00:28, Stefan Dösinger <stefan(a)codeweavers.com> wrote:
No, the pipeline part that would implement this(vertex) should set a state handler that prints a WARN for each defined but unimplemented stage. Sure, but that doesn't mean you can't catch undefined states as well. E.g. something along the lines of the attached patch. The main use would be simplifying reporting bugs. The earlier posted debugging patch is more extensive of course, but that would require people to build with special compile flags if it ever enters wined3d in some form.
Henri Verbeet a écrit :
On 17 February 2010 00:28, Stefan Dösinger <stefan(a)codeweavers.com> wrote:
No, the pipeline part that would implement this(vertex) should set a state handler that prints a WARN for each defined but unimplemented stage.
Sure, but that doesn't mean you can't catch undefined states as well. E.g. something along the lines of the attached patch. The main use would be simplifying reporting bugs. The earlier posted debugging patch is more extensive of course, but that would require people to build with special compile flags if it ever enters wined3d in some form.
If we could get state_undefined to receive the real state this would work like a charm.
participants (3)
-
Christian Costa -
Henri Verbeet -
Stefan Dösinger