Re: GSoC-2011: Implement Missing Mesh Functions in Wine’s D3DX9

[Michael Mc Donnell]

On Sat, May 28, 2011 at 10:08 PM, Stefan Dösinger
<stefandoesinger at gmx.at> wrote:
> On Saturday 28 May 2011 19:55:55 you wrote:
>
>> It was implemented by setting the vertex declaration to null when the
>> declaration is invalid. That seems to match what happens on Windows,
>> because if a program has set an invalid declaration and then calls
>> GetDeclaration or DrawSubset it will fail with an access violation.
>> Should Wine also fail with an access violation or should it be
>> slightly more graceful and return E_FAIL instead (which my new patch
>> does)?
> Yes, I think returning E_FAIL is a good idea, but write a WARN if
> CreateVertexDeclaration fails or when you catch a NULL vdecl. A WARN will not
> be shown by default(unlike a FIXME), it is used when the app does something
> fishy but you think you handled it correctly.
>
>> +    if (FAILED(hr))
>> +        new_vertex_declaration = NULL;
>> +
>> +    /* Free old vertex declaration */
>> +    if (This->vertex_declaration)
>> +        IDirect3DVertexDeclaration9_Release(This->vertex_declaration);
> In this situation you can just release the old decl before calling
> CreateVertexDeclaration and pass &This->vertex_declaration to CreateVdecl.
> That simplified the code a bit. Don't forget to set This->vertex_declaration to
> NULL in case of an error though

Ok I've simplified the code. I've also inserted three WARNs. I've
skipped a WARN when freeing the mesh because that works fine on
Windows with an invalid declaration.

>> +    /* Set the position type to color instead of float3 (invalid
> declaration) */
>> +    hr = mesh->lpVtbl->UpdateSemantics(mesh, declaration_wrong_type_color);
>> +    todo_wine ok(hr == D3D_OK, "Test UpdateSematics wrong type color, "
>> +        "got %#x expected D3D_OK\n", hr);
> Is this really invalid? I don't know it on top of my head, but in general
> types and usage are independent, especially with shaders. I don't see a check
> in d3d9 or wined3d for this case.

You're right it isn't invalid. It doesn't crash, it just doesn't draw
anything. I've change that comment and re-ordered the tests.

> Beyond that I think the patches look OK. I'd recommend to send them to wine-
> devel one more time and then next week to wine-patches. Henri is on Vacation
> this week, and he's usually more picky than I am. I also hope some of the
> people who regularly contribute to d3dx9 will have a look and comment.

Ok, I've attached the newest version. I'll let the patches sit here
for a week, and start working on the other functions in the mean time.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-d3dx9-test-Added-UpdateSemanticsTest.patch
Type: text/x-patch
Size: 10971 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20110529/4456ebce/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-d3dx9-Implemented-UpdateSemantics-mesh-method.patch
Type: text/x-patch
Size: 7191 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20110529/4456ebce/attachment-0003.bin>