Summary: | Support all available biBitCount values in BitmapInfo | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Patrick R. Gansterer <paroga> | ||||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, aroben, commit-queue, eric, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Windows XP | ||||||||||
Attachments: |
|
Description
Patrick R. Gansterer
2010-08-09 08:55:42 PDT
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.
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 (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. Created attachment 65138 [details]
Patch
The eol-style for BitmapInfo is wrong in the ChangeLog.
Created attachment 65139 [details]
Patch
Comment on attachment 65139 [details]
Patch
r=me
Comment on attachment 65139 [details] Patch Clearing flags on attachment: 65139 Committed r65857: <http://trac.webkit.org/changeset/65857> All reviewed patches have been landed. Closing bug. 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 |