[PATCH 1/5] d2d1: Implement d2d_d3d_render_target_CreateSolidColorBrush().

[Henri Verbeet]

On 16 June 2014 16:50, Jacek Caban <jacek at 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.