Re: [PATCH v4] d2d1: Implement ID2D1DeviceContext::CreateImageBrush().

[Henri Verbeet]

On Wed, 11 May 2022 at 16:22, Dmitry Timoshkov  wrote:
> +HRESULT d2d_image_brush_create(ID2D1Factory *factory, ID2D1Image *image,
> -
>    const D2D1_IMAGE_BRUSH_PROPERTIES *image_brush_desc, const D2D1_BRUSH_PROPERTIES *brush_desc,
> -
>    struct d2d_brush **brush)
>
> +{
> - if (!(*brush = heap_alloc_zero(sizeof(**brush))))
> -
>    return E_OUTOFMEMORY;
> -
> - d2d_brush_init(*brush, factory, D2D_BRUSH_TYPE_BITMAP,
> -
>        brush_desc, (ID2D1BrushVtbl *)&d2d_image_brush_vtbl);

I think you intended to use D2D_BRUSH_TYPE_IMAGE above.
> - if (((*brush)->u.image.image = image))
> -
>    ID2D1Image_AddRef((*brush)->u.image.image);
> - if (image_brush_desc)
> - {
> -
>    (*brush)->u.image.extend_mode_x = image_brush_desc->extendModeX;
> -
>    (*brush)->u.image.extend_mode_y = image_brush_desc->extendModeY;
> -
>    (*brush)->u.image.interpolation_mode = image_brush_desc->interpolationMode;
> - }

This ignores "image_brush_desc->sourceRectangle". That's fine to some
extent, but might be easy to overlook when implementing
d2d_image_brush_SetSourceRectangle()/d2d_image_brush_GetSourceRectangle()
later.
> +static void d2d_brush_bind_image(struct d2d_brush *brush, struct d2d_device_context *context,
> -
>    unsigned int brush_idx)
>
> +{
> - ID3D11SamplerState **sampler_state;
> - ID3D11DeviceContext *d3d_context;
> - ID2D1Bitmap *bitmap;
> - struct d2d_bitmap *src_impl;
> - HRESULT hr;
> -
> - hr = ID2D1Image_QueryInterface(brush->u.image.image, &IID_ID2D1Bitmap, (void **)&bitmap);
> - if (FAILED(hr))
> - {
> -
>    FIXME("ID2D1Image doesn't support ID2D1Bitmap interface.\n");
> -
>    return;
> - }
> -
> - src_impl = unsafe_impl_from_ID2D1Bitmap(bitmap);
> -
> - ID3D11Device1_GetImmediateContext(context->d3d_device, &d3d_context);
> - ID3D11DeviceContext_PSSetShaderResources(d3d_context, brush_idx, 1, &src_impl->srv);
> -
> - sampler_state = &context->sampler_states
> -
>        [brush->u.image.interpolation_mode % D2D_SAMPLER_INTERPOLATION_MODE_COUNT]
> -
>        [brush->u.image.extend_mode_x % D2D_SAMPLER_EXTEND_MODE_COUNT]
> -
>        [brush->u.image.extend_mode_y % D2D_SAMPLER_EXTEND_MODE_COUNT];
> -
> - if (!*sampler_state)
> - {
> -
>    D3D11_SAMPLER_DESC sampler_desc;
> -
> -
>    if (brush->u.image.interpolation_mode == D2D1_INTERPOLATION_MODE_NEAREST_NEIGHBOR)
> -
>        sampler_desc.Filter = D3D11_FILTER_MIN_MAG_MIP_POINT;
> -
>    else
> -
>        sampler_desc.Filter = D3D11_FILTER_MIN_MAG_MIP_LINEAR;
> -
>    sampler_desc.AddressU = texture_address_mode_from_extend_mode(brush->u.image.extend_mode_x);
> -
>    sampler_desc.AddressV = texture_address_mode_from_extend_mode(brush->u.image.extend_mode_y);
> -
>    sampler_desc.AddressW = D3D11_TEXTURE_ADDRESS_CLAMP;
> -
>    sampler_desc.MipLODBias = 0.0f;
> -
>    sampler_desc.MaxAnisotropy = 0;
> -
>    sampler_desc.ComparisonFunc = D3D11_COMPARISON_NEVER;
> -
>    sampler_desc.BorderColor[0] = 0.0f;
> -
>    sampler_desc.BorderColor[1] = 0.0f;
> -
>    sampler_desc.BorderColor[2] = 0.0f;
> -
>    sampler_desc.BorderColor[3] = 0.0f;
> -
>    sampler_desc.MinLOD = 0.0f;
> -
>    sampler_desc.MaxLOD = 0.0f;
> -
> -
>    if (FAILED(hr = ID3D11Device1_CreateSamplerState(context->d3d_device, &sampler_desc, sampler_state)))
> -
>        ERR("Failed to create sampler state, hr %#lx.\n", hr);
> - }
> -
> - ID3D11DeviceContext_PSSetSamplers(d3d_context, brush_idx, 1, sampler_state);
> - ID3D11DeviceContext_Release(d3d_context);
> - ID2D1Bitmap_Release(bitmap);
>
> +}

This doesn't seem wrong, although we could probably introduce a common
helper for this and d2d_brush_bind_bitmap(). At the same time though,
it's not going to do much as long as the pixel shader (i.e., ps_code[]
in d2d_device_context_init()) doesn't handle image brushes. (And in
it's current form, we never get here because we set the brush type to
D2D_BRUSH_TYPE_BITMAP in d2d_image_brush_create().) It might be best
to just go with the FIXME in d2d_brush_bind_resources(), unless we
need the brush to actually work.