v2: Support only D2D1_MAP_OPTIONS_READ and always use staging resource. v3: Handle texture creation failure, fix staging resource leak.
Signed-off-by: Dmitry Timoshkov dmitry@baikal.ru --- dlls/d2d1/bitmap.c | 53 ++++++++++++++++++++++++++++++++++++++-- dlls/d2d1/d2d1_private.h | 1 + 2 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/dlls/d2d1/bitmap.c b/dlls/d2d1/bitmap.c index 971e3c7ff6b..a75a9be62be 100644 --- a/dlls/d2d1/bitmap.c +++ b/dlls/d2d1/bitmap.c @@ -66,6 +66,8 @@ static ULONG STDMETHODCALLTYPE d2d_bitmap_Release(ID2D1Bitmap1 *iface)
if (!refcount) { + if (bitmap->staging_resource) + ID3D11Resource_Release(bitmap->staging_resource); if (bitmap->srv) ID3D11ShaderResourceView_Release(bitmap->srv); if (bitmap->rtv) @@ -229,9 +231,56 @@ static HRESULT STDMETHODCALLTYPE d2d_bitmap_GetSurface(ID2D1Bitmap1 *iface, IDXG 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); + + if (options != D2D1_MAP_OPTIONS_READ) + { + FIXME("Unhandled map options %#x.\n", options); + return E_INVALIDARG; + } + + if (!(bitmap->options & D2D1_BITMAP_OPTIONS_CPU_READ)) + return E_INVALIDARG; + + if (bitmap->staging_resource) + return D2DERR_WRONG_STATE; + + 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_READ; + if (SUCCEEDED(hr = 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_READ, 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; }
static HRESULT STDMETHODCALLTYPE d2d_bitmap_Unmap(ID2D1Bitmap1 *iface) diff --git a/dlls/d2d1/d2d1_private.h b/dlls/d2d1/d2d1_private.h index 8edfa030ee1..b886ac52dd9 100644 --- a/dlls/d2d1/d2d1_private.h +++ b/dlls/d2d1/d2d1_private.h @@ -395,6 +395,7 @@ struct d2d_bitmap ID3D11RenderTargetView *rtv; IDXGISurface *surface; ID3D11Resource *resource; + ID3D11Resource *staging_resource; D2D1_SIZE_U pixel_size; D2D1_PIXEL_FORMAT format; float dpi_x;
I started by extending tests from patch 3/3, and I'm curious now how does your application create such bitmap?
Is it just ID2D1DeviceContext::CreateBitmap(D2D1_BITMAP_OPTIONS_CANNOT_DRAW | D2D1_BITMAP_OPTIONS_CPU_READ), or something more complicated?
How is this bitmap used later? Is it set to an image brush, and then used for fills?
Nikolay Sivov nsivov@codeweavers.com wrote:
I started by extending tests from patch 3/3, and I'm curious now how does your application create such bitmap?
Is it just ID2D1DeviceContext::CreateBitmap(D2D1_BITMAP_OPTIONS_CANNOT_DRAW | D2D1_BITMAP_OPTIONS_CPU_READ), or something more complicated?
How is this bitmap used later? Is it set to an image brush, and then used for fills?
The application first creates a bitmap with D2D1_BITMAP_OPTIONS_TARGET, then creates an image brush from it. Then the app creates another bitmap with D2D1_BITMAP_OPTIONS_CANNOT_DRAW | D2D1_BITMAP_OPTIONS_CPU_READ, copies contents from original bitmap with ::CopyFromBitmap() and then ::Map()s it with D2D1_MAP_OPTIONS_READ.
On 5/29/22 12:03, Dmitry Timoshkov wrote:
Nikolay Sivov nsivov@codeweavers.com wrote:
I started by extending tests from patch 3/3, and I'm curious now how does your application create such bitmap?
Is it just ID2D1DeviceContext::CreateBitmap(D2D1_BITMAP_OPTIONS_CANNOT_DRAW | D2D1_BITMAP_OPTIONS_CPU_READ), or something more complicated?
How is this bitmap used later? Is it set to an image brush, and then used for fills?
The application first creates a bitmap with D2D1_BITMAP_OPTIONS_TARGET, then creates an image brush from it. Then the app creates another bitmap with D2D1_BITMAP_OPTIONS_CANNOT_DRAW | D2D1_BITMAP_OPTIONS_CPU_READ, copies contents from original bitmap with ::CopyFromBitmap() and then ::Map()s it with D2D1_MAP_OPTIONS_READ.
I see, makes sense. Please test with attached patches to see if this path still works. Turns out there is no need to create a temporary resource.
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=115724
Your paranoid android.
=== debian11 (64 bit WoW report) ===
d2d1: d2d1: Timeout
Nikolay Sivov nsivov@codeweavers.com wrote:
On 5/29/22 12:03, Dmitry Timoshkov wrote:
Nikolay Sivov nsivov@codeweavers.com wrote:
I started by extending tests from patch 3/3, and I'm curious now how does your application create such bitmap?
Is it just ID2D1DeviceContext::CreateBitmap(D2D1_BITMAP_OPTIONS_CANNOT_DRAW | D2D1_BITMAP_OPTIONS_CPU_READ), or something more complicated?
How is this bitmap used later? Is it set to an image brush, and then used for fills?
The application first creates a bitmap with D2D1_BITMAP_OPTIONS_TARGET, then creates an image brush from it. Then the app creates another bitmap with D2D1_BITMAP_OPTIONS_CANNOT_DRAW | D2D1_BITMAP_OPTIONS_CPU_READ, copies contents from original bitmap with ::CopyFromBitmap() and then ::Map()s it with D2D1_MAP_OPTIONS_READ.
I see, makes sense. Please test with attached patches to see if this path still works. Turns out there is no need to create a temporary resource.
Attached patches seem to work, Unmap() helper could be inlined though. Probably I should also note, that the patch to make an image brush work for bitmaps is still required for correct painting in my application.
Thanks for keeping working on this.
Dmitry Timoshkov dmitry@baikal.ru wrote:
Nikolay Sivov nsivov@codeweavers.com wrote:
On 5/29/22 12:03, Dmitry Timoshkov wrote:
Nikolay Sivov nsivov@codeweavers.com wrote:
I started by extending tests from patch 3/3, and I'm curious now how does your application create such bitmap?
Is it just ID2D1DeviceContext::CreateBitmap(D2D1_BITMAP_OPTIONS_CANNOT_DRAW | D2D1_BITMAP_OPTIONS_CPU_READ), or something more complicated?
How is this bitmap used later? Is it set to an image brush, and then used for fills?
The application first creates a bitmap with D2D1_BITMAP_OPTIONS_TARGET, then creates an image brush from it. Then the app creates another bitmap with D2D1_BITMAP_OPTIONS_CANNOT_DRAW | D2D1_BITMAP_OPTIONS_CPU_READ, copies contents from original bitmap with ::CopyFromBitmap() and then ::Map()s it with D2D1_MAP_OPTIONS_READ.
I see, makes sense. Please test with attached patches to see if this path still works. Turns out there is no need to create a temporary resource.
Attached patches seem to work, Unmap() helper could be inlined though. Probably I should also note, that the patch to make an image brush work for bitmaps is still required for correct painting in my application.
Unfortunately further testing shows that with today's winehq.git my application hangs on exit. Regression test points to
5e25a4546c812eb174d142a41c28cf83ef2b567e is the first bad commit commit 5e25a4546c812eb174d142a41c28cf83ef2b567e Author: Nikolay Sivov nsivov@codeweavers.com Date: Sun May 29 15:18:53 2022 +0300
d2d1: Implement bitmap mapping.
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
dlls/d2d1/bitmap.c | 72 +++++++++++++++++++++++++++++++++++++++++++++--- dlls/d2d1/d2d1_private.h | 1 + dlls/d2d1/tests/d2d1.c | 21 +------------- 3 files changed, 70 insertions(+), 24 deletions(-)
Do you have an idea what might be the reason of the hang?
On 5/31/22 15:03, Dmitry Timoshkov wrote:
Dmitry Timoshkov dmitry@baikal.ru wrote:
Nikolay Sivov nsivov@codeweavers.com wrote:
On 5/29/22 12:03, Dmitry Timoshkov wrote:
Nikolay Sivov nsivov@codeweavers.com wrote:
I started by extending tests from patch 3/3, and I'm curious now how does your application create such bitmap?
Is it just ID2D1DeviceContext::CreateBitmap(D2D1_BITMAP_OPTIONS_CANNOT_DRAW | D2D1_BITMAP_OPTIONS_CPU_READ), or something more complicated?
How is this bitmap used later? Is it set to an image brush, and then used for fills?
The application first creates a bitmap with D2D1_BITMAP_OPTIONS_TARGET, then creates an image brush from it. Then the app creates another bitmap with D2D1_BITMAP_OPTIONS_CANNOT_DRAW | D2D1_BITMAP_OPTIONS_CPU_READ, copies contents from original bitmap with ::CopyFromBitmap() and then ::Map()s it with D2D1_MAP_OPTIONS_READ.
I see, makes sense. Please test with attached patches to see if this path still works. Turns out there is no need to create a temporary resource.
Attached patches seem to work, Unmap() helper could be inlined though. Probably I should also note, that the patch to make an image brush work for bitmaps is still required for correct painting in my application.
Unfortunately further testing shows that with today's winehq.git my application hangs on exit. Regression test points to
5e25a4546c812eb174d142a41c28cf83ef2b567e is the first bad commit commit 5e25a4546c812eb174d142a41c28cf83ef2b567e Author: Nikolay Sivov nsivov@codeweavers.com Date: Sun May 29 15:18:53 2022 +0300
d2d1: Implement bitmap mapping. Signed-off-by: Nikolay Sivov <nsivov@codeweavers.com>
dlls/d2d1/bitmap.c | 72 +++++++++++++++++++++++++++++++++++++++++++++--- dlls/d2d1/d2d1_private.h | 1 + dlls/d2d1/tests/d2d1.c | 21 +------------- 3 files changed, 70 insertions(+), 24 deletions(-)
Do you have an idea what might be the reason of the hang?
Without any additional information, I don't have any.
Nikolay Sivov nsivov@codeweavers.com wrote:
On 5/31/22 15:03, Dmitry Timoshkov wrote:
Dmitry Timoshkov dmitry@baikal.ru wrote:
Nikolay Sivov nsivov@codeweavers.com wrote:
On 5/29/22 12:03, Dmitry Timoshkov wrote:
Nikolay Sivov nsivov@codeweavers.com wrote:
I started by extending tests from patch 3/3, and I'm curious now how does your application create such bitmap?
Is it just ID2D1DeviceContext::CreateBitmap(D2D1_BITMAP_OPTIONS_CANNOT_DRAW | D2D1_BITMAP_OPTIONS_CPU_READ), or something more complicated?
How is this bitmap used later? Is it set to an image brush, and then used for fills?
The application first creates a bitmap with D2D1_BITMAP_OPTIONS_TARGET, then creates an image brush from it. Then the app creates another bitmap with D2D1_BITMAP_OPTIONS_CANNOT_DRAW | D2D1_BITMAP_OPTIONS_CPU_READ, copies contents from original bitmap with ::CopyFromBitmap() and then ::Map()s it with D2D1_MAP_OPTIONS_READ.
I see, makes sense. Please test with attached patches to see if this path still works. Turns out there is no need to create a temporary resource.
Attached patches seem to work, Unmap() helper could be inlined though. Probably I should also note, that the patch to make an image brush work for bitmaps is still required for correct painting in my application.
Unfortunately further testing shows that with today's winehq.git my application hangs on exit. Regression test points to
5e25a4546c812eb174d142a41c28cf83ef2b567e is the first bad commit commit 5e25a4546c812eb174d142a41c28cf83ef2b567e Author: Nikolay Sivov nsivov@codeweavers.com Date: Sun May 29 15:18:53 2022 +0300
d2d1: Implement bitmap mapping. Signed-off-by: Nikolay Sivov <nsivov@codeweavers.com>
dlls/d2d1/bitmap.c | 72 +++++++++++++++++++++++++++++++++++++++++++++--- dlls/d2d1/d2d1_private.h | 1 + dlls/d2d1/tests/d2d1.c | 21 +------------- 3 files changed, 70 insertions(+), 24 deletions(-)
Do you have an idea what might be the reason of the hang?
Without any additional information, I don't have any.
It looks like that the hang is caused by new code path enabled by ::Map(), and using my original implementation on top of wine-7.9 shows exactly same behaviour.
Sorry for the false alarm.