Bug 42071 - [WINCE] Add additonal methodes to BitmapInfo
Summary: [WINCE] Add additonal methodes to BitmapInfo
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-12 05:21 PDT by Patrick R. Gansterer
Modified: 2010-07-22 20:05 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.74 KB, patch)
2010-07-12 05:25 PDT, Patrick R. Gansterer
aroben: review-
Details | Formatted Diff | Diff
Patch (5.51 KB, patch)
2010-07-22 14:06 PDT, Patrick R. Gansterer
aroben: review-
aroben: commit-queue-
Details | Formatted Diff | Diff
Patch (5.51 KB, patch)
2010-07-22 14:29 PDT, Patrick R. Gansterer
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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