On 5/12/22 21:10, Dmitry Timoshkov wrote:
Nikolay Sivov nsivov@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.