Re: [PATCH 9/9] d2d1: Partially implement ID2D1Bitmap1

[Nikolay Sivov]

On 21.11.2017 11:41, Lucian Poston wrote:
> @@ -69,6 +93,10 @@ static ULONG STDMETHODCALLTYPE d2d_bitmap_Release(ID2D1Bitmap1 *iface)

...
> -
>        ID2D1ColorContext_Release(bitmap->color_context);
> -
>    if (bitmap->surface)
> -
>        IDXGISurface_Release(bitmap->surface);

...
> -
> - if (This->surface)
> - {
> -
>    IDXGISurface_AddRef(This->surface);
> - }
> -

Is it really possible that bitmap has no surface?
> +static D2D1_BITMAP_PROPERTIES1 bitmap_properties_to_properties1(
> -
>    const D2D1_BITMAP_PROPERTIES *desc)
>
> +{
> - D2D1_BITMAP_PROPERTIES1 d;
> - d.bitmapOptions = D2D1_BITMAP_OPTIONS_NONE;
> - d.colorContext = NULL;
> - if (desc == NULL)
> - {
> -
>    d.pixelFormat.format = DXGI_FORMAT_UNKNOWN;
> -
>    d.pixelFormat.alphaMode = D2D1_ALPHA_MODE_UNKNOWN;
> -
>    d.dpiX = 96.0f;
> -
>    d.dpiY = 96.0f;
> - }
> - else
> - {
> -
>    d.pixelFormat.format = desc->pixelFormat.format;
> -
>    d.pixelFormat.alphaMode = desc->pixelFormat.alphaMode;
> -
>    d.dpiX = desc->dpiX;
> -
>    d.dpiY = desc->dpiY;
> - }
> -
> - return d;
>
> +}
> -

Maybe it's better to switch to PROPERTIES1 internally instead of
converting down like that.
> static void d2d_bitmap_init(struct d2d_bitmap *bitmap, ID2D1Factory *factory,
> -
>    ID3D10ShaderResourceView *view, D2D1_SIZE_U size, const D2D1_BITMAP_PROPERTIES *desc)
> -
>    ID3D10ShaderResourceView *view, D2D1_SIZE_U size, const D2D1_BITMAP_PROPERTIES1 *desc,
> -
>    IDXGISurface *surface)

...
> +HRESULT d2d_bitmap_create_from_dxgi(ID2D1Factory *factory, IDXGISurface *surface,
> -
>    D2D1_SIZE_U size, const void *src_data, UINT32 pitch,
> -
>    const D2D1_BITMAP_PROPERTIES1 *desc, struct d2d_bitmap **bitmap)

You can create a bitmap from:

- externally surface, so it's just referenced;
- from memory buffer, surface is created internally using this data;
- from WIC bitmap, basically same as memory buffer case;
- shared bitmap, same as first case.

Patch duplicates from shared bitmap case does already in case of surface
input, maybe that should be factored out somehow.
> - if (FAILED(hr = ID3D10Texture2D_QueryInterface(texture, &IID_IDXGISurface, (void **)&surface)))
> - {
> -
>    surface = NULL;
> -
>    WARN("Texture2D had no underlying DXGISurface");
> - }
> -

When does this fail? I mean it probably makes sense to have a warning,
but continue without a surface?