Bug 42071 – [WINCE] Add additonal methodes to BitmapInfo

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Patrick R. Gansterer]

see patch

[Patrick R. Gansterer]

Created attachment 61211 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 61211 [details]
Patch

> +        [WINCE] Add additonal methodes to BitmapInfo.

Typo: methodes

> -BitmapInfo bitmapInfoForSize(int width, int height)
> +BitmapInfo bitmapInfoForSize(int width, int height, WORD bitCount)
>  {
> +    ASSERT(bitCount == 16 || bitCount == 32);

ASSERT_ARG would be a bit better here:

ASSERT_ARG(bitCount, bitCount == 16 || bitCount == 32);

>      BitmapInfo bitmapInfo;
> -    bitmapInfo.bmiHeader.biSize          = sizeof(BITMAPINFOHEADER);

Perhaps the ChangeLog should mention that the BitmapInfo constructor does this for us. I was confused by this change at first.

> +#if !OS(WINCE) || PLATFORM(QT) || IMAGE_NO_ALPHA_USE_RGB555

Where/in what situations is IMAGE_NO_ALPHA_USE_RGB555 defined?

> +    if (bitCount == 16) {
> +        ((DWORD*)bitmapInfo.bmiColors)[0] = 0x0000F800;
> +        ((DWORD*)bitmapInfo.bmiColors)[1] = 0x000007E0;
> +        ((DWORD*)bitmapInfo.bmiColors)[2] = 0x0000001F;
> +    }
> +#endif

Where do these constants come from?

reinterpret_cast would be better here.

The changes look fine other than the cast, though I am interested in your answers to these questions.

[Patrick R. Gansterer]

Created attachment 62336 [details]
Patch

(In reply to comment #2)
> > +#if !OS(WINCE) || PLATFORM(QT) || IMAGE_NO_ALPHA_USE_RGB555
> 
> Where/in what situations is IMAGE_NO_ALPHA_USE_RGB555 defined?
I removed it by now.

[Adam Roben (:aroben)]

Comment on attachment 62336 [details]
Patch

> + * Copyright (C) 2007-2009 Torch Moible, Inc. All Rights Reserved.

Typo: Moible

[Patrick R. Gansterer]

Created attachment 62341 [details]
Patch

(In reply to comment #4)
> (From update of attachment 62336 [details])
> > + * Copyright (C) 2007-2009 Torch Moible, Inc. All Rights Reserved.
> 
> Typo: Moible
Wasn't too hard to fix. ;-)

[Adam Roben (:aroben)]

Comment on attachment 62341 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 62341 [details]
Patch

Clearing flags on attachment: 62341

Committed r63944: <http://trac.webkit.org/changeset/63944>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/63944 might have broken Qt Windows 32-bit Release and Qt Windows 32-bit Debug