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

[Dmitry Timoshkov]

Piotr Caban <piotr(a)codeweavers.com> 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?

-- 
Dmitry.

[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 <piotr(a)codeweavers.com> 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 <piotr.caban(a)gmail.com> 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.

-- 
Dmitry.

[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 <piotr.caban(a)gmail.com> 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