Re: [PATCH 2/7] gdiplus: Initialize bitmap bits for reading in GdipBitmapLockBits when ImageFlagsReadOnly is specified

[Dmitry Timoshkov]

Piotr Caban  wrote:
>  /* Make sure we can convert to the requested format. */
> - if (flags & ImageLockModeRead)
> - if ((flags & ImageLockModeRead) || (bitmap->image.flags & ImageFlagsReadOnly))

Is there a test for this behaviour?

[PATCH 2/7] gdiplus: Initialize bitmap bits for reading in GdipBitmapLockBits when ImageFlagsReadOnly is specified

[Piotr Caban]

On 09/02/17 02:16, Dmitry Timoshkov wrote:
> Piotr Caban  wrote:
> >   /* Make sure we can convert to the requested format. */
> > - if (flags & ImageLockModeRead)
> > - if ((flags & ImageLockModeRead) || (bitmap->image.flags & ImageFlagsReadOnly))
>
> Is there a test for this behaviour?

It's not possible to test exactly this flag because we can't set them by 
hand (and none of the bitmap constructors sets only it). I'm testing an 
image with this flag in patch 7.

Thanks,
Piotr

[PATCH 2/7] gdiplus: Initialize bitmap bits for reading in GdipBitmapLockBits when ImageFlagsReadOnly is specified

[Dmitry Timoshkov]

Piotr Caban  wrote:
> > >   /* Make sure we can convert to the requested format. */
> > > - if (flags & ImageLockModeRead)
> > > - if ((flags & ImageLockModeRead) || (bitmap->image.flags & ImageFlagsReadOnly))
> >
> > Is there a test for this behaviour?
>
> It's not possible to test exactly this flag because we can't set them by 
> hand (and none of the bitmap constructors sets only it). I'm testing an 
> image with this flag in patch 7.

As far as I can see the test in patch 7 shows that GdipCreateHBITMAPFromBitmap()
should not use GdipBitmapLockBits() internally, and the patch 6 fixes this case.
Adding the check above is not justified neither by tests nor by an explanation.

[PATCH 2/7] gdiplus: Initialize bitmap bits for reading in GdipBitmapLockBits when ImageFlagsReadOnly is specified

[Piotr Caban]

Hi Dmitry,

On 10/02/17 02:06, Dmitry Timoshkov wrote:
> Piotr Caban  wrote:
> > > >    /* Make sure we can convert to the requested format. */
> > > > - if (flags & ImageLockModeRead)
> > > > - if ((flags & ImageLockModeRead) || (bitmap->image.flags & ImageFlagsReadOnly))
> > >
> > > Is there a test for this behaviour?
> >
> > It's not possible to test exactly this flag because we can't set them by
> > hand (and none of the bitmap constructors sets only it). I'm testing an
> > image with this flag in patch 7.
>
> As far as I can see the test in patch 7 shows that GdipCreateHBITMAPFromBitmap()
> should not use GdipBitmapLockBits() internally, and the patch 6 fixes this case.
> Adding the check above is not justified neither by tests nor by an explanation.

I'll add more tests for this case.

Thanks,
Piotr