[PATCH] d2d1: Make ID2D1Device::CreateImageBrush() accept only bitmap as a source image.
ID2D1Bitmap derives from ID2D1Image, which in turn derives from ID2D1Resource. That means that ID2D1Device::CreateImageBrush() can't be really passed anything but a ID2D1Bitmap* represented as a ID2D1Image*. I've added QueryInterface+FIXME just in case, probably it could be dropped. Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/d2d1/brush.c | 11 ++++++++++- dlls/d2d1/d2d1_private.h | 1 - 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/dlls/d2d1/brush.c b/dlls/d2d1/brush.c index b9a673fe54c..8d4715d429a 100644 --- a/dlls/d2d1/brush.c +++ b/dlls/d2d1/brush.c @@ -1324,10 +1324,19 @@ HRESULT d2d_image_brush_create(ID2D1Factory *factory, ID2D1Image *image, const D2D1_IMAGE_BRUSH_PROPERTIES *image_brush_desc, const D2D1_BRUSH_PROPERTIES *brush_desc, struct d2d_brush **brush) { + ID2D1Bitmap *bitmap; + if (!(*brush = heap_alloc_zero(sizeof(**brush)))) return E_OUTOFMEMORY; - d2d_brush_init(*brush, factory, D2D_BRUSH_TYPE_IMAGE, + if (FAILED(ID2D1Image_QueryInterface(image, &IID_ID2D1Bitmap, (void **)&bitmap))) + { + FIXME("ID2D1Image doesn't support ID2D1Bitmap interface.\n"); + return E_FAIL; + } + ID2D1Bitmap_Release(bitmap); + + d2d_brush_init(*brush, factory, D2D_BRUSH_TYPE_BITMAP, brush_desc, (ID2D1BrushVtbl *)&d2d_image_brush_vtbl); if (((*brush)->u.image.image = image)) ID2D1Image_AddRef((*brush)->u.image.image); diff --git a/dlls/d2d1/d2d1_private.h b/dlls/d2d1/d2d1_private.h index 29469f5142e..6183d9d4c99 100644 --- a/dlls/d2d1/d2d1_private.h +++ b/dlls/d2d1/d2d1_private.h @@ -42,7 +42,6 @@ enum d2d_brush_type D2D_BRUSH_TYPE_LINEAR, D2D_BRUSH_TYPE_RADIAL, D2D_BRUSH_TYPE_BITMAP, - D2D_BRUSH_TYPE_IMAGE, D2D_BRUSH_TYPE_COUNT, }; -- 2.36.1
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=115074 Your paranoid android. === debian11 (32 bit Chinese:China report) === d2d1: Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x6ac09fba). === debian11 (64 bit WoW report) === d2d1: Unhandled exception: page fault on read access to 0x0000000000000000 in 64-bit code (0x0000006aa08fae).
On 5/19/22 11:56, Dmitry Timoshkov wrote:
ID2D1Bitmap derives from ID2D1Image, which in turn derives from ID2D1Resource. That means that ID2D1Device::CreateImageBrush() can't be really passed anything but a ID2D1Bitmap* represented as a ID2D1Image*.
I've added QueryInterface+FIXME just in case, probably it could be dropped.
Command lists and effects outputs are also ID2D1Image, so it's not always a bitmap.
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 5/19/22 11:56, Dmitry Timoshkov wrote:
ID2D1Bitmap derives from ID2D1Image, which in turn derives from ID2D1Resource. That means that ID2D1Device::CreateImageBrush() can't be really passed anything but a ID2D1Bitmap* represented as a ID2D1Image*.
I've added QueryInterface+FIXME just in case, probably it could be dropped.
Command lists and effects outputs are also ID2D1Image, so it's not always a bitmap.
Thanks for pointing that out. Currently images that are not bitmaps can't work as brushes anyway, Henri already explaned that in response to original version of my patch. If anything besides bitmaps is supposed to be supported by ID2D1Device::CreateImageBrush() that needs quite a bit of work. For now accepting only bitmaps seems a fair limitation, and in fact that makes the app that I have here happy. -- Dmitry.
On 5/19/22 12:16, Dmitry Timoshkov wrote:
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 5/19/22 11:56, Dmitry Timoshkov wrote:
ID2D1Bitmap derives from ID2D1Image, which in turn derives from ID2D1Resource. That means that ID2D1Device::CreateImageBrush() can't be really passed anything but a ID2D1Bitmap* represented as a ID2D1Image*.
I've added QueryInterface+FIXME just in case, probably it could be dropped. Command lists and effects outputs are also ID2D1Image, so it's not always a bitmap. Thanks for pointing that out. Currently images that are not bitmaps can't work as brushes anyway, Henri already explaned that in response to original version of my patch. If anything besides bitmaps is supposed to be supported by ID2D1Device::CreateImageBrush() that needs quite a bit of work. For now accepting only bitmaps seems a fair limitation, and in fact that makes the app that I have here happy.
I'm not asking to make command lists and effects work before the bitmap image. But this change is backwards, if anything we could implement existing bitmap brush as an image brush with a bitmap. Checking image type on creation is likely insufficient, because you can do SetImage() later. Also extend and interpolation modes will be ignored.
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 5/19/22 12:16, Dmitry Timoshkov wrote:
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 5/19/22 11:56, Dmitry Timoshkov wrote:
ID2D1Bitmap derives from ID2D1Image, which in turn derives from ID2D1Resource. That means that ID2D1Device::CreateImageBrush() can't be really passed anything but a ID2D1Bitmap* represented as a ID2D1Image*.
I've added QueryInterface+FIXME just in case, probably it could be dropped. Command lists and effects outputs are also ID2D1Image, so it's not always a bitmap. Thanks for pointing that out. Currently images that are not bitmaps can't work as brushes anyway, Henri already explaned that in response to original version of my patch. If anything besides bitmaps is supposed to be supported by ID2D1Device::CreateImageBrush() that needs quite a bit of work. For now accepting only bitmaps seems a fair limitation, and in fact that makes the app that I have here happy.
I'm not asking to make command lists and effects work before the bitmap image. But this change is backwards, if anything we could implement existing bitmap brush as an image brush with a bitmap.
You are right. That's my fault, since I failed to consider initially simply adding a new brush vtable (that you pointed out in the first review) and using existing BITMAP_TYPE for the brush. Merging existing bitmap brush implementation with an image brush seems impossible due to different vtable layouts.
Checking image type on creation is likely insufficient, because you can do SetImage() later.
That's a good point, SetImage also needs to fixed in that regard.
Also extend and interpolation modes will be ignored.
That's also a fair point. Supporting only bitmaps from the very start would be more correct, again, that's my fault since I didn't know the consequences of introducing new brush type. -- Dmitry.
participants (3)
-
Dmitry Timoshkov -
Marvin -
Nikolay Sivov