Re: [WINED3D] QueryInterface corrections
On 25/04/06, Ivan Gyurdiev <ivg2(a)cornell.edu> wrote:
(2) write a NULL argument on no match.
This patch corrects those issues, and also changes GetContainer() for surfaces and volumes to use the return value as MSDN specifies (WINED3D_INVALIDCALL), not E_NOINTERFACE on failure. Did you verify this on Windows? MSDN tends to lie about these kind of things.
Also not that te value that wined3d returns isn't necessarily the same as what the respective d3d8/d3d9 call returns, although in practice it often is.
H. Verbeet wrote:
On 25/04/06, Ivan Gyurdiev <ivg2(a)cornell.edu> wrote:
(2) write a NULL argument on no match.
This patch corrects those issues, and also changes GetContainer() for surfaces and volumes to use the return value as MSDN specifies (WINED3D_INVALIDCALL), not E_NOINTERFACE on failure.
Did you verify this on Windows? MSDN tends to lie about these kind of things.
No, I'll admit I did not (because I don't have Windows). If somebody could check I'd appreciate it.
On 25/04/06, Ivan Gyurdiev <ivg2(a)cornell.edu> wrote:
H. Verbeet wrote:
On 25/04/06, Ivan Gyurdiev <ivg2(a)cornell.edu> wrote:
(2) write a NULL argument on no match.
This patch corrects those issues, and also changes GetContainer() for surfaces and volumes to use the return value as MSDN specifies (WINED3D_INVALIDCALL), not E_NOINTERFACE on failure.
Did you verify this on Windows? MSDN tends to lie about these kind of things.
No, I'll admit I did not (because I don't have Windows). If somebody could check I'd appreciate it.
I was under the impression that patches are supposed to be verified to be correct before being submitted. Seeing this got commited, (together with some changes that weren't in the original diff?), perhaps I was wrong. Either way, I should probably have been somewhat more explicit: That part of the patch is simply wrong. - At least for d3d9, GetContainer returns E_NOINTERFACE on Windows. I verified that when I rewrote part of the function some time ago, and I just did again. I originally intended to add a test for that, but well, guess I forgot about it. - Even if GetContainer should return D3DERR_INVALIDCALL (not WINED3D_INVALIDCALL!), this wouldn't be the right place to change it, since d3d8/9 doesn't just forward to this function, but uses it internally to get the wined3d container parent. It then gets the d3d8/9 object from that. The return value for d3d8/9 is the result of a QueryInterface on that object.
H. Verbeet wrote:
On 25/04/06, Ivan Gyurdiev <ivg2(a)cornell.edu> wrote:
H. Verbeet wrote:
On 25/04/06, Ivan Gyurdiev <ivg2(a)cornell.edu> wrote:
(2) write a NULL argument on no match.
This patch corrects those issues, and also changes GetContainer() for surfaces and volumes to use the return value as MSDN specifies (WINED3D_INVALIDCALL), not E_NOINTERFACE on failure.
Did you verify this on Windows? MSDN tends to lie about these kind of things.
No, I'll admit I did not (because I don't have Windows). If somebody could check I'd appreciate it.
I was under the impression that patches are supposed to be verified to be correct before being submitted. You are right. Since it seems I can't rely on the spec to be correct, and I don't have Windows, in the future I'll only send no-op patches, or patches that I can observe the effect of....
- At least for d3d9, GetContainer returns E_NOINTERFACE on Windows. I verified that when I rewrote part of the function some time ago, and I just did again. I originally intended to add a test for that, but well, guess I forgot about it. Can you add a comment to the code when the spec is verified to be wrong. Otherwise programmers like me will make a mistake and assume the spec to be valid.
On 06/05/06, Ivan Gyurdiev <ivg2(a)cornell.edu> wrote:
- At least for d3d9, GetContainer returns E_NOINTERFACE on Windows. I verified that when I rewrote part of the function some time ago, and I just did again. I originally intended to add a test for that, but well, guess I forgot about it. Can you add a comment to the code when the spec is verified to be wrong. Otherwise programmers like me will make a mistake and assume the spec to be valid. Sure, but you shouldn't use MSDN for anything more than getting a general idea of what a function is supposed to do in the first place.It is particularly bad for things like return codes, refcounts and whether things are checked for NULL / written with NULL on error, to name a few things.
- Even if GetContainer should return D3DERR_INVALIDCALL (not WINED3D_INVALIDCALL!) ... Well, WINED3DERR_INVALIDCALL == D3DERR_INVALIDCALL. I've added those return values to the wined3d headers so I can check in ddraw for the return values. A lot of retvals from wined3d are in d3d8 / d3d9 and do not exist in ddraw.
Just wanted to point that out. Stefan
On 07/05/06, Stefan Dösinger <stefan(a)codeweavers.com> wrote:
- Even if GetContainer should return D3DERR_INVALIDCALL (not WINED3D_INVALIDCALL!) ... Well, WINED3DERR_INVALIDCALL == D3DERR_INVALIDCALL. I've added those return values to the wined3d headers so I can check in ddraw for the return values. A lot of retvals from wined3d are in d3d8 / d3d9 and do not exist in ddraw.
Just wanted to point that out.
Stefan
Yes, it does work. But it's still technically wrong, just like S_OK vs WINED3D_OK is. The correct way would be to translate wined3d return codes in d3d7/8/9, although it could be considered a bit excessive.
Well, WINED3DERR_INVALIDCALL == D3DERR_INVALIDCALL. I've added those return values to the wined3d headers so I can check in ddraw for the return values. A lot of retvals from wined3d are in d3d8 / d3d9 and do not exist in ddraw.
Just wanted to point that out.
Stefan
Yes, it does work. But it's still technically wrong, just like S_OK vs WINED3D_OK is. The correct way would be to translate wined3d return codes in d3d7/8/9, although it could be considered a bit excessive. In ddraw I have to do that, because some return values are different, e.g. WINED3DERR_INVALIDCALL vs DDERR_INVALIDPARAMS. I will code code a small macro which translates the different values, with a default of just returning the wined3d number. This could be put in d3d8 and d3d9 as a no-op macro / inline function, and be extended if needed. If you look at the GetDC patch I sent yesterday, it returns DDERR_DCALREADYCREATED to give ddraw more information about the error. This doesn't exist in d3d8 and d3d9, which should return D3DERR_INVALIDCALL in the same situation.
I have quickly created this function: inline HRESULT hr_ddraw_from_wined3d(HRESULT hr) { switch(hr) { case WINED3DERR_INVALIDCALL: return DDERR_INVALIDPARAMS; default: return hr; } } If you remove the first case clause the compiler should optimize it away with -O2, at least in my small assembler program it simply ignores the call, and doesn't inline any code. When this is in place, there's a easy place to add return code translation if needed. BTW, our d3d8 and d3d9 libraries know about the ddraw return codes because I've included the ddraw.h file.
participants (3)
-
H. Verbeet -
Ivan Gyurdiev -
Stefan Dösinger