Re: [v2 PATCH] d2d1: Support shared bitmaps created from dxgi surface

[Nikolay Sivov]

On 10.10.2016 14:15, Henri Verbeet wrote:
> On 8 October 2016 at 19:18, Nikolay Sivov  wrote:
> > -HRESULT d2d_bitmap_create_shared(ID2D1Factory *factory, ID3D10Device *target_device,
> > +HRESULT d2d_bitmap_create_shared(ID2D1RenderTarget *render_target, ID3D10Device *target_device,
> >          REFIID iid, void *data, const D2D1_BITMAP_PROPERTIES *desc, struct d2d_bitmap **bitmap)
> >  {
> > - D2D1_BITMAP_PROPERTIES d;
> > - ID2D1Factory *factory;
> > -
> > - ID2D1RenderTarget_GetFactory(render_target, &factory);
> > - ID2D1Factory_Release(factory);
>
> This is a little improper. It does save a few lines, but I'd rather
> just release it after we're actually done with it.

Okay.
> > - else if (IsEqualGUID(iid, &IID_IDXGISurface) || IsEqualGUID(iid, &IID_IDXGISurface1))
>
> "else" is redundant here, although that's minor.

Sure.
> > - {
> > -
> >    ID3D10ShaderResourceView *view;
> > -
> >    DXGI_SURFACE_DESC surface_desc;
> > -
> >    IDXGISurface *surface = data;
> > -
> >    ID3D10Resource *resource;
> > -
> >    D2D1_SIZE_U pixel_size;
> > -
> >    HRESULT hr;
> > -
> > -
> >    if (FAILED(IDXGISurface_QueryInterface(surface, &IID_ID3D10Resource, (void **)&resource)))
> > -
> >    {
> > -
> >        WARN("Failed to get d3d resource from dxgi surface.\n");
> > -
> >        return E_FAIL;
> > -
> >    }
> > -
> > -
> >    hr = ID3D10Device_CreateShaderResourceView(target_device, resource, NULL, &view);
>
> I think we should validate the dxgi surface's device matches the
> target device first, like in the IID_ID2D1Bitmap case.

Will do.
> It's in fact possible to share dxgi resources between different
> devices and supposedly even different processes, but doing that would
> require using IDXGIResource::GetSharedHandle(),
> ID3D10Device::OpenSharedResource(), and D3D10_RESOURCE_MISC_SHARED,
> all of which are currently unimplemented in Wine. I'm not sure if d2d1
> allows it.

This sounds like too much, with nothing that depends on this yet.
I guess different device case/same process is easy to test,
but if it's a general sharing rule of dxgi/d3d10/11 to restrict resource
to device it was created from, tests should go to dxgi/d3d10/11 instead.
> > - /* Shared DXGI surface. */
> > - desc.type = D2D1_RENDER_TARGET_TYPE_DEFAULT;
> > - desc.pixelFormat.format = DXGI_FORMAT_B8G8R8A8_UNORM;
> > - desc.pixelFormat.alphaMode = D2D1_ALPHA_MODE_PREMULTIPLIED;
> > - desc.dpiX = 0.0f;
> > - desc.dpiY = 0.0f;
> > - desc.usage = D2D1_RENDER_TARGET_USAGE_NONE;
> > - desc.minLevel = D2D1_FEATURE_LEVEL_DEFAULT;
> > -
> > - hr = ID2D1Factory_CreateDxgiSurfaceRenderTarget(factory1, surface2, &desc, &rt2);
> > - ok(SUCCEEDED(hr), "Failed to create render target, hr %#x.\n", hr);
> > -
> > - bitmap_desc.pixelFormat.format = DXGI_FORMAT_B8G8R8A8_UNORM;
> > - bitmap_desc.pixelFormat.alphaMode = D2D1_ALPHA_MODE_PREMULTIPLIED;
> > - bitmap_desc.dpiX = 0.0f;
> > - bitmap_desc.dpiY = 0.0f;
> > -
> > - hr = ID2D1RenderTarget_CreateSharedBitmap(rt2, &IID_IDXGISurface, surface2, &bitmap_desc, &bitmap2);
> > - ok(SUCCEEDED(hr) || broken(hr == E_INVALIDARG) /* vista */, "Failed to create bitmap, hr %#x.\n", hr);
>
> It's interesting that this works. I'd expect actually drawing with
> that bitmap on that render target to be undefined though.

Undefined because same surface is used for render target and bitmap? Do
you want me to change that?