Bug 42071

Summary: [WINCE] Add additonal methodes to BitmapInfo
Product: WebKit Reporter: Patrick R. Gansterer <paroga>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aroben, commit-queue, eric, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: Windows XP   
Attachments:
Description Flags
Patch
aroben: review-
Patch
aroben: review-, aroben: commit-queue-
Patch none

Description Patrick R. Gansterer 2010-07-12 05:21:47 PDT
see patch
Comment 1 Patrick R. Gansterer 2010-07-12 05:25:02 PDT
Created attachment 61211 [details]
Patch
Comment 2 Adam Roben (:aroben) 2010-07-22 13:41:45 PDT
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.
Comment 3 Patrick R. Gansterer 2010-07-22 14:06:38 PDT
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.
Comment 4 Adam Roben (:aroben) 2010-07-22 14:25:06 PDT
Comment on attachment 62336 [details]
Patch

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

Typo: Moible
Comment 5 Patrick R. Gansterer 2010-07-22 14:29:20 PDT
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. ;-)
Comment 6 Adam Roben (:aroben) 2010-07-22 15:03:51 PDT
Comment on attachment 62341 [details]
Patch

r=me
Comment 7 WebKit Commit Bot 2010-07-22 19:55:32 PDT
Comment on attachment 62341 [details]
Patch

Clearing flags on attachment: 62341

Committed r63944: <http://trac.webkit.org/changeset/63944>
Comment 8 WebKit Commit Bot 2010-07-22 19:55:36 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 WebKit Review Bot 2010-07-22 20:05:53 PDT
http://trac.webkit.org/changeset/63944 might have broken Qt Windows 32-bit Release and Qt Windows 32-bit Debug