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

[Nikolay Sivov]

On 5/12/22 22:47, Dmitry Timoshkov wrote:
> Nikolay Sivov <nsivov at codeweavers.com> wrote:
>
>>>> 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.
>>> The application I have here passes a bitmap without CPU_READ flag, so
>>> ::Map() fails, and that's the reason for creating a staging texture.
>>> Initially I had the code that calls ::Map() for the bitmap->resource,
>>> and if that fails then creates a staging texture. What approach would
>>> be better: check that bitmap was created with CPU_READ flag, or always
>>> try to map bitmap->resource and fallback to a staging texture if that
>>> fails?
>> Bitmap always references same resource, and resource parameters can't
>> change, so in principle you'll know which path will work when bitmap is
>> created.
> So, to sum it up, ::Map() should have dedicated code paths for bitmaps with
> and without CPU_READ flag.

Not just the flag, bitmap could be created from existing resource.

I suggest for now limit this to READ, with a staging resource, that we 
can later optimize.

Please also add a test for bitmap configuration that your application 
uses. Also what happens on a Map() on already mapped bitmap is also 
interesting, right now you'll leak a texture.
>
>> It's probably fine to have support only for READ map, and have a fixme,
>> with error return value otherwise.
>>
>> For CPU_READ flag, we probably should have a fixme too.
>>
>> Note that you don't release staging texture on unmap, and
>> d3d11_map_from_d2d1_map_options() seems wrong because D3D11_MAP_* is not
>> a mask.
> Thanks for noticing these bugs, good catch.
>
>>>> Another question is what happens with
>>>> CreateBitmapFromDxgiSurface if you give it unmappable surface, but
>>>> CPU_READ option.
>>> Is that accepatable to leave this question aside for the initial implementation?
>>>
>> I quickly tested this with existing tests that are using swapchain
>> buffer. It fails to map on windows, but with this patch it works. So
>> there is already a discrepancy.
> Probably having a fixme or a comment mentioning that scenario could also
> be acceptable?
>
Ideally in a form of a todo test.