On 16 June 2014 16:50, Jacek Caban jacek@codeweavers.com wrote:
struct d2d_brush *b = unsafe_impl_from_ID2DBrush(brush);
Since you have many different vtbls for ID2DBrush implementation, you can't use the usual vtbl trick for checking if passed pointer is really our implementation. One way this can be solved is to abuse
Other places that do this kind of thing just check against multiple vtbls.
QueryInterface, so that you could call:
ID2DBrush_QueryInterface(brush, &IID_brush_struct, (void**)&brush_struct);
QueryInterface could just return the struct directly for that special IID. And yeah, returning struct instead of an interface is not pretty, but that's probably fine here. You could even skip AddRef in this case.
That works, although personally I'd say it's a little more complex than just having the casts, and not really any prettier. I don't care too strongly though, so as far as I'm concerned it comes down to whatever Alexandre prefers.
... ID3D10Device_PSSetShader(device, b->shader); ID3D10Device_PSSetConstantBuffers(device, 0, 1, &b->cb); ... ID3D10Device_DrawIndexed(device, ...);
I see, this could really use common code.
Sure. Eventually we'll probably have something like "d2d_brush_apply(brush, device);", and we'll probably want to queue up and merge draw operations until ID2D1RenderTarget::EndDraw(), but that's the basic idea.
I think the two casts, while not very pretty, are worth it in comparison.
This, again, depends on how this will look when the implementation is more complete. Such casts are usually a sign of a design problem, but the final decision is up to you. *If* such casts will be only in brush creation code, that's not too bad.
Yeah, it should only be needed in the creation / initialization code, everything else should just operate on struct d2d_brush.
You could make them even more local (eg. by replacing d2d_solid_color_brush_init by a function that returns ID2D1SolidBrush interface pointer, so that caller doesn't need to do any more cast and the problem does not spread across the code).
Yeah, perhaps. Effectively that just means moving most of the body of d2d_d3d_render_target_CreateSolidColorBrush() to brush.c, and it's another one of those things that I'm not really opposed to, but don't really think are worth it either.