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

[Nikolay Sivov]

On 5/12/22 21:10, Dmitry Timoshkov wrote:
> Nikolay Sivov <nsivov at codeweavers.com> wrote:
>
>>>> 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.
> That's a good point, thanks.
>
>> 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.
> 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.

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.
>
>> 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.