Re: [PATCH 1/2] d2d1: Implement ID2D1Bitmap1::Map().

[Nikolay Sivov]

On 5/12/22 19:49, Dmitry Timoshkov wrote:
> Nikolay Sivov  wrote:
> > On 5/12/22 16:23, Dmitry Timoshkov wrote:
> > > static HRESULT STDMETHODCALLTYPE d2d_bitmap_Map(ID2D1Bitmap1 *iface, D2D1_MAP_OPTIONS options,
> > >            D2D1_MAPPED_RECT *mapped_rect)
> > >    {
> > > - FIXME("iface %p, options %#x, mapped_rect %p stub!\n", iface, options, mapped_rect);
> > > - struct d2d_bitmap *bitmap = impl_from_ID2D1Bitmap1(iface);
> > > - ID3D11Device *device;
> > > - ID3D11DeviceContext *context;
> > > - D3D11_TEXTURE2D_DESC texture_desc;
> > > - ID3D11Texture2D *staging_texture;
> > > - D3D11_MAPPED_SUBRESOURCE mapped_subresource;
> > > - HRESULT hr;
> > > - return E_NOTIMPL;
> > > - TRACE("iface %p, options %#x, mapped_rect %p.\n", iface, options, mapped_rect);
> > > -
> > > - ID3D11Resource_GetDevice(bitmap->resource, &device);
> > > - ID3D11Device_GetImmediateContext(device, &context);
> > > - ID3D11Texture2D_GetDesc((ID3D11Texture2D *)bitmap->resource, &texture_desc);
> > > -
> > > - texture_desc.Usage = D3D11_USAGE_STAGING;
> > > - texture_desc.BindFlags = 0;
> > > - texture_desc.CPUAccessFlags = d3d11_cpu_access_from_d2d1_map_options(options);
> > > - ID3D11Device_CreateTexture2D(device, &texture_desc, NULL, &staging_texture);
> > > - ID3D11DeviceContext_CopyResource(context, (ID3D11Resource *)staging_texture, bitmap->resource);
> > > - if (SUCCEEDED(hr = ID3D11DeviceContext_Map(context, (ID3D11Resource *)staging_texture, 0,
> > > -
> > >        d3d11_map_from_d2d1_map_options(options), 0, &mapped_subresource)))
> > > - {
> > > -
> > >    mapped_rect->pitch = mapped_subresource.RowPitch;
> > > -
> > >    mapped_rect->bits = mapped_subresource.pData;
> > > -
> > >    bitmap->staging_resource = (ID3D11Resource *)staging_texture;
> > > - }
> > > - else
> > > - {
> > > -
> > >    WARN("Failed to map resource, hr %#lx.\n", hr);
> > > -
> > >    ID3D11Texture2D_Release(staging_texture);
> > > - }
> > > -
> > > - ID3D11DeviceContext_Release(context);
> > > - ID3D11Device_Release(device);
> > > -
> > > - return hr;
> > > }
> >
> > According to docs, Map() only works for D2D1_BITMAP_OPTIONS_CPU_READ,
> > that potentially simplifies things.
>
> Thanks for the review. I guess that you suggest to drop the from_d2d1_map_options()
> helpers and always use READ access, is that correct, or there are other things to
> get into account for simplification?

If it has CPU_READ I'd think you can map it directly, without a staging 
resource.

Another implication is that bitmap resource might have to be made 
inaccessible after Map(), e.g. it's possible you can't use it in drawing 
commands, right away, or it will fail on EndDraw(), who knows.
> > Also Unmap() in patch 2 will not
> > write anything back.
>
> Should it? I don't see that mentioned in MSDN, and implying that only READ access
> is supposed to be supported by ::Map() it means that bitmap memory can't be modified
> by user.

It also says that it's supported in effects for DISCARD | WRITE, and 
there is a movement in this area on patch list, so might not be entirely 
theoretical.

But anyway, it's impossible to guess, this needs some tests for 
different options, maybe CreateBitmap() fails for anything than READ or 
NONE. In that case you don't need to handle anything else, and if Map 
only works for bitmaps with CPU_READ, you probably can map resource 
without a copy. Another question is what happens with 
CreateBitmapFromDxgiSurface if you give it unmappable surface, but 
CPU_READ option.