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

[Lucian Poston]

On Tue, Nov 21, 2017 at 5:52 AM, Nikolay Sivov  wrote:
> 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?

Yes. Even prior to this patch, it could be initialized with a d3d
device and no surface. This patch series adds a code path to create a
bitmap with a surface and no device, namely
CreateBitmapFromDxgiSurface.
> > +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.

We have to convert from PROPERTIES to PROPERTIES1 at some point since
some input parameters to the older public APIs are PROPERTIES. Are you
saying the conversion should occur earlier, like immediately in the
older public APIs?
> > - 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?

The Texture2D implementation has code that is conditional on whether
it has an internal surface object, including its QueryInterface
implementation, so I wrote the bitmap code with that Texture2D
assumption. If that's an invalid assumption, I can change this.