[PATCH v3] d2d1: Make ID2D1Device::CreateImageBrush() accept only bitmap as a source image.

[Nikolay Sivov]

On 6/7/22 12:12, Dmitry Timoshkov wrote:
> Nikolay Sivov <nsivov at codeweavers.com> wrote:
>
>>> +static void d2d_brush_bind_bitmap(struct d2d_brush *brush, struct
>>> d2d_device_context *context,
>>> +        unsigned int brush_idx)
>>> +{
>>> +    D2D1_IMAGE_BRUSH_PROPERTIES image_brush_desc;
>>> +
>>> +    image_brush_desc.sourceRectangle.left = 0.0f;
>>> +    image_brush_desc.sourceRectangle.top = 0.0f;
>>> +    image_brush_desc.sourceRectangle.right =
>>> brush->u.bitmap.bitmap->pixel_size.width;
>>> +    image_brush_desc.sourceRectangle.bottom =
>>> brush->u.bitmap.bitmap->pixel_size.height;
>> This needs a test, so we don't assume coordinate system here. We have
>> tests for filling with 4x4 bitmap brush, it will be a matter of creating
>> image brush with same bitmap, and rectangle as (0,0-4,4) vs (0,0-1,1).
>> If that shows no difference it means this is using normalized
>> coordinates, and we should initialize it here as (0,0-1,1).
> I've added the test in the attached version of the patch, is that what you
> had in mind? The test shows that (0,0-4,4) vs (0,0-1,1) source rectangles
> lead to different painting results under Windows. However, since source
> rectangle is completely ignored in current image brush implementation
> that doesn't really change anything, so I'm not sure what kind of result
> this test is supposed to have for the proposed patch.

It shows if we should be using normalized rectangle for regular bitmap 
brushes, or rectangle in pixel coordinates, like you did. It looks like 
pixel coordinates are used, after I tried some visual tests.

>
>>> @@ -1043,7 +1043,7 @@ static void STDMETHODCALLTYPE
>>> d2d_device_context_FillGeometry(ID2D1DeviceContext
>>>       if (FAILED(context->error.code))
>>>           return;
>>>
>>> -    if (opacity_brush && brush_impl->type != D2D_BRUSH_TYPE_BITMAP)
>>> +    if (opacity_brush && !(brush_impl->type == D2D_BRUSH_TYPE_BITMAP
>>> || brush_impl->type == D2D_BRUSH_TYPE_IMAGE))
>>>       {
>>>           d2d_device_context_set_error(context,
>>> D2DERR_INCOMPATIBLE_BRUSH_TYPES);
>>>           return;
>> Same here, we have a test for this case already, that needs to be
>> extended to verify this change.
> I guess you mean the tests in test_opacity_brush()? Anyway, it looks like
> omitting this part of the patch changes nothing in my application, moreover
> MSDN states that when the opacity brush is specified in FillGeometry() brush
> must be an ID2D1BitmapBrush. So, this part is clearly wrong, and if desired
> should be sent as a separate change.

Yes, I think it's fine to leave this for a separate change. I'd expect 
bitmap brush and (image brush with a bitmap) be treated the same way 
here. Documentation could as well be outdated, because image brush was 
introduced with later versions, device contexts were not used in 
initially released d2d API.

>
> Thanks again for the helpful comments.
>