Bug 43724 – Support all available biBitCount values in BitmapInfo

• Component: Platform
• Status: RESOLVED FIXED

[Patrick R. Gansterer]

see patch

[Patrick R. Gansterer]

Created attachment 63902 [details]
Patch

bitmapInfo.bmiHeader.biSizeImage     = 0;
bitmapInfo.bmiHeader.biXPelsPerMeter = 0;
bitmapInfo.bmiHeader.biYPelsPerMeter = 0;
bitmapInfo.bmiHeader.biClrUsed       = 0;
bitmapInfo.bmiHeader.biClrImportant  = 0;
Is alredy done via memset in constructor.

[Adam Roben (:aroben)]

Comment on attachment 63902 [details]
Patch

>  BitmapInfo bitmapInfoForSize(int width, int height, WORD bitCount)
>  {
> -    ASSERT_ARG(bitCount, bitCount == 16 || bitCount == 32);
> +    ASSERT_ARG(bitCount, bitCount == 1 || bitCount == 4 || bitCount == 8 || bitCount == 16 || bitCount == 24 || bitCount == 32);

Maybe an enum would be better for the bitCount parameter?

> @@ -43,11 +43,6 @@ BitmapInfo bitmapInfoForSize(int width, 
>      bitmapInfo.bmiHeader.biPlanes        = 1;
>      bitmapInfo.bmiHeader.biBitCount      = bitCount;
>      bitmapInfo.bmiHeader.biCompression   = BI_RGB;
> -    bitmapInfo.bmiHeader.biSizeImage     = 0;
> -    bitmapInfo.bmiHeader.biXPelsPerMeter = 0;
> -    bitmapInfo.bmiHeader.biYPelsPerMeter = 0;
> -    bitmapInfo.bmiHeader.biClrUsed       = 0;
> -    bitmapInfo.bmiHeader.biClrImportant  = 0;

Please explain this change in the ChangeLog just as you did in an earlier comment in this bug.
> @@ -44,10 +44,12 @@ struct BitmapInfo : public BITMAPINFO {
>      unsigned width() const { return abs(bmiHeader.biWidth); }
>      unsigned height() const { return abs(bmiHeader.biHeight); }
>      IntSize size() const { return IntSize(width(), height()); }
> -    unsigned paddedWidth() const { return is16bit() ? (width() + 1) & ~0x1 : width(); }
> +    unsigned bytesPerLine() const { return (width() * bmiHeader.biBitCount + 7) / 8; }
> +    unsigned paddedBytesPerLine() const { return (bytesPerLine() + 3) & ~0x3; }
> +    unsigned paddedWidth() const { return paddedBytesPerLine() * 8 / bmiHeader.biBitCount; }
>      unsigned numPixels() const { return paddedWidth() * height(); }
> -    unsigned paddedBytesPerLine() const { return is16bit() ? paddedWidth() * 2 : width() * 4; }
> -    unsigned bytesPerLine() const { return width() * bmiHeader.biBitCount / 8; }};

How will someone know whether they should call width() or paddedWidth()? (Ditto for the other functions.)

r=me

[Patrick R. Gansterer]

(In reply to comment #2)
> How will someone know whether they should call width() or paddedWidth()? (Ditto for the other functions.)
I think that you will know that if you do this "low level" graphic stuff on windows.
The padded*() function are only used when you set the colors byte-by-byte.

[Patrick R. Gansterer]

Created attachment 65138 [details]
Patch

The eol-style for BitmapInfo is wrong in the ChangeLog.

[Patrick R. Gansterer]

Created attachment 65139 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 65139 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 65139 [details]
Patch

Clearing flags on attachment: 65139

Committed r65857: <http://trac.webkit.org/changeset/65857>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/65857 might have broken GTK Linux 32-bit Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/65856
http://trac.webkit.org/changeset/65857
http://trac.webkit.org/changeset/65854
http://trac.webkit.org/changeset/65855