On Tue, 8 Jun 2021 at 10:57, Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
-HRESULT d2d_brush_get_ps_cb(struct d2d_brush *brush, struct d2d_brush *opacity_brush, - BOOL outline, BOOL is_arc, struct d2d_device_context *render_target, ID3D10Buffer **ps_cb) +HRESULT d2d_brush_init_ps_cb_data(struct d2d_brush *brush, struct d2d_brush *opacity_brush, + BOOL outline, BOOL is_arc, struct d2d_ps_cb *cb_data) { - D3D10_SUBRESOURCE_DATA buffer_data; - struct d2d_ps_cb cb_data = {0}; - D3D10_BUFFER_DESC buffer_desc; - HRESULT hr; - - cb_data.outline = outline; - cb_data.is_arc = is_arc; - if (!d2d_brush_fill_cb(brush, &cb_data.colour_brush)) + cb_data->outline = outline; + cb_data->is_arc = is_arc; + if (!d2d_brush_fill_cb(brush, &cb_data->colour_brush)) return E_NOTIMPL; - if (!d2d_brush_fill_cb(opacity_brush, &cb_data.opacity_brush)) + if (!d2d_brush_fill_cb(opacity_brush, &cb_data->opacity_brush)) return E_NOTIMPL;
- buffer_desc.ByteWidth = sizeof(cb_data); - buffer_desc.Usage = D3D10_USAGE_DEFAULT; - buffer_desc.BindFlags = D3D10_BIND_CONSTANT_BUFFER; - buffer_desc.CPUAccessFlags = 0; - buffer_desc.MiscFlags = 0; - - buffer_data.pSysMem = &cb_data; - buffer_data.SysMemPitch = 0; - buffer_data.SysMemSlicePitch = 0; - - if (FAILED(hr = ID3D10Device_CreateBuffer(render_target->d3d_device, &buffer_desc, &buffer_data, ps_cb))) - ERR("Failed to create constant buffer, hr %#x.\n", hr); - - return hr; + return S_OK; }
Arguably, what's left of the function could just be inlined in d2d_device_context_update_ps_cb() now.
@@ -266,6 +266,8 @@ static ULONG STDMETHODCALLTYPE d2d_device_context_inner_Release(IUnknown *iface) ID3D10Buffer_Release(context->vb); ID3D10Buffer_Release(context->ib); ID3D10PixelShader_Release(context->ps); + if (context->ps_cb) + ID3D10Buffer_Release(context->ps_cb); for (i = 0; i < D2D_SHAPE_TYPE_COUNT; ++i) { ID3D10VertexShader_Release(context->shape_resources[i].vs); If we created "context->ps_cb" in d2d_device_context_init(), this would become an unconditional release.
+static HRESULT d2d_device_context_update_ps_cb(struct d2d_device_context *context, + struct d2d_brush *brush, struct d2d_brush *opacity_brush, BOOL outline, BOOL is_arc) +{ + D3D10_SUBRESOURCE_DATA buffer_data; + struct d2d_ps_cb cb_data = {0}; + D3D10_BUFFER_DESC buffer_desc; + void *data; + HRESULT hr; + + if (FAILED(hr = d2d_brush_init_ps_cb_data(brush, opacity_brush, outline, is_arc, &cb_data))) + { + WARN("Failed to initialize brush constant buffer data, hr %#x.\n", hr); + return hr; + } + + if (context->ps_cb) + { + if (SUCCEEDED(hr = ID3D10Buffer_Map(context->ps_cb, D3D10_MAP_WRITE_DISCARD, 0, &data))) + { + memcpy(data, &cb_data, sizeof(cb_data)); + ID3D10Buffer_Unmap(context->ps_cb); + } + else + ERR("Failed to map constant buffer, hr %#x.\n", hr); + } We don't need the memcpy(). I.e., we can just do the following:
struct d2d_ps_cb *cb_data; [...] ID3D10Buffer_Map(..., &cb_data); d2d_brush_init_ps_cb_data(..., cb_data); ID3D10Buffer_Unmap(...);
+ else + { + buffer_desc.ByteWidth = sizeof(cb_data); + buffer_desc.Usage = D3D10_USAGE_DYNAMIC; + buffer_desc.BindFlags = D3D10_BIND_CONSTANT_BUFFER; + buffer_desc.CPUAccessFlags = D3D10_CPU_ACCESS_WRITE; + buffer_desc.MiscFlags = 0; + + buffer_data.pSysMem = &cb_data; + buffer_data.SysMemPitch = 0; + buffer_data.SysMemSlicePitch = 0; + + if (FAILED(hr = ID3D10Device_CreateBuffer(context->d3d_device, &buffer_desc, &buffer_data, + &context->ps_cb))) + { + ERR("Failed to create constant buffer, hr %#x.\n", hr); + } + } + + return hr; +} + And this could just happen during d2d_device_context_init(). (The "buffer_data" argument is optional; it can be convenient for initialising resources without explicitly mapping them, but I don't think it provides much of an advantage here.)
@@ -1676,34 +1707,19 @@ static void STDMETHODCALLTYPE d2d_device_context_Clear(ID2D1DeviceContext *iface return; }
- ps_cb_data.outline = FALSE; - ps_cb_data.colour_brush.type = D2D_BRUSH_TYPE_SOLID; - ps_cb_data.colour_brush.opacity = 1.0f; - c = &ps_cb_data.colour_brush.u.solid.colour; + memset(&brush, 0, sizeof(brush)); + brush.type = D2D_BRUSH_TYPE_SOLID; + brush.opacity = 1.0f; + c = &brush.u.solid.color; if (colour) *c = *colour; if (render_target->desc.pixelFormat.alphaMode == D2D1_ALPHA_MODE_IGNORE) c->a = 1.0f; - c->r *= c->a; - c->g *= c->a; - c->b *= c->a; - Constructing a brush like that is a little awkward, I think. To some extent that's also true of constructing the d2d_ps_cb structure in the original code, but brush objects are a bit more complex than the d2d_ps_cb structure. It would seem preferable to either directly map "ps_cb" here, and generate its contents in the existing way, or to create a proper brush object using d2d_solid_color_brush_create().