[2/2] d3dx9: Implement UpdateSemantics mesh method. (try 2)

[Michael Mc Donnell]

On Fri, Jun 10, 2011 at 8:08 AM, Dylan Smith <dylan.ah.smith at gmail.com> wrote:
> Your patches have dos line endings (i.e. "\r\n") which causes git apply to
> fail, although the patch command only gives a warning.

Yeah I think this is because of git-imap was changed to convert the
line endings on the fly. I only get the dos line endings if I
downloaded the patch from my imap folder. Does anyone know how to make
git-imap stop converting the line endings?

>
>> @@ -208,6 +221,12 @@ static DWORD WINAPI
>> ID3DXMeshImpl_GetNumBytesPerVertex(ID3DXMesh *iface)
>>     TRACE("iface (%p)\n", This);
>> +    if (!This->vertex_declaration)
>> +    {
>> +        WARN("Can't get number of bytes per vertex of an invalid vertex
>> declaration.\n");
>> +        return E_FAIL;
>> +    }
>
> GetNumBytesPerVertex returns a DWORD not an HRESULT, so I wouldn't expect
> E_FAIL to ever be returned? Unusual behaviour like this should be covered by
> a test.

Thanks for catching that one. You're right I added the E_FAIL for
GetNumBytesPerVertex too quickly with out testing. I'll write a test
case for that.

>
>> +    /* An application can pass an invalid declaration to UpdateSemantics
>> and
>> +     * still expect D3D_OK (see tests). If the declaration is invalid,
>> then
>> +     * subsequent calls to GetDeclaration, GetNumBytesPerVertex, and
>> DrawSubset,
>> +     * which use the declaration, will fail.
>> +     */
>
> D3DXCreateMesh fails when the declaration contains a non-zero Stream value.
> I would expect UpdateSemantics to be as strict as D3DXCreateMesh, otherwise
> a test could validate the different behaviour.

Ok, I'll write a test for that too.

> You tested a declaration with larger fields, but not with the vertex size
> being smaller or greater. (E.g. Try removing the last field, or appending a
> field.)

Yeah I see I only test for larger overlapping fields, but not vertex
declaration sizes being smaller or greater. I'll fix that.

> I did some testing and couldn't get GetDeclaration or GetNumBytesPerVertex
> to fail on windows.  DrawSubset did seem to fail after UpdateSemantics is
> called with a declaration that causes CreateVertexDeclaration to fail. This
> suggests that the native implementation probably just stores the
> D3DVERTEXELEMENT9 array and calls CreateVertexDeclaration on DrawSubset. You
> can probably cache the created vertex declaration and release it on
> UpdateSemantics for performance.

You're right again. GetDeclaration returns the invalid declaration,
which means that it is probably cached like you're describing. I'll
add checks for that too.