Bug 34684 – [Haiku] Fix rect conversion and image rendering on Haiku

• Component: Platform
• Status: RESOLVED FIXED

[Stephan Aßmus]

The changes to the rect conversions are indeed correct. In Haiku (to stay
compatibly with BeOS), a BRect specifies the left/top and bottom/right pixel
*indices*, even though the values are floating point. So a rectangle covering
just one pixel would be specified as BRect(0, 0, 0, 0). In WebCore and other
frame works, such rectangles would be expressed as 0, 0, 1, 1. In WebCore, the
width and height of rectangles refer to the distance between pixels, while on
Haiku, a one pixel rect has indeed a width and height of 0, as confusing as
that may be.

The part of the patch that affects
WebCore/platform/graphics/haiku/ImageHaiku.cpp also implements the drawing
methods more correctly. Image observers are notified, and pattern drawing takes
the "phase" into account which makes scrolled backgrounds render correctly.
Transformations are still not supported, since the Haiku drawing backend itself
does not yet support them.

[Stephan Aßmus]

Created attachment 48296 [details]
Fixes to the rect conversions and image rendering on Haiku

Patch against r54275.

[WebKit Review Bot]

Attachment 48296 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/graphics/haiku/ImageHaiku.cpp:180:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1


If any of these errors are false positives, please file a bug against check-webkit-style.

[Stephan Aßmus]

Created attachment 48297 [details]
Fixes to the rect conversions and image rendering on Haiku

Revised patch to please the style police. (I found it nicer to read before...)

[Eric Seidel (no email)]

Comment on attachment 48297 [details]
Fixes to the rect conversions and image rendering on Haiku

 46 #include <stdio.h>
seems wrong.

This should be an OwnPtr:
     BBitmap* bmp = new BBitmap(BRect(0, 0, width() - 1, height() - 1), 0,
 38         B_RGBA32, bytesPerRow);
you can call .release() during return.

[David Levin]

Comment on attachment 48297 [details]
Fixes to the rect conversions and image rendering on Haiku

Eric's comment plus these:

> Index: WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp
>  NativeImagePtr RGBA32Buffer::asNewNativeImage() const
>  {
> +    int bytesPerRow = width() * sizeof(PixelData);
> +    BBitmap* bmp = new BBitmap(BRect(0, 0, width() - 1, height() - 1), 0,
> +        B_RGBA32, bytesPerRow);

You don't need to wrap this line, but if you're going to do this, I think folks typically align with the enclosing paren.

> +
> +    const uint8* src = reinterpret_cast<const uint8*>(m_bytes.data());
> +    uint8* dst = reinterpret_cast<uint8*>(bmp->Bits());

In general WebKit, likes to avoid abbreviations. s/src/source/ s/dst/destination/

> +    int h = height();
> +    for (int y = 0; y < h; y++) {
> +        memcpy(dst, src, bytesPerRow);
> +        dst += bytesPerRow;
> +        src += bytesPerRow;

Why doesn't this just do memcpy(dst, src, bytesPerRow * h); ?

[Stephan Aßmus]

Created attachment 48521 [details]
Fixes to the rect conversions and image rendering on Haiku

New patch fixes the issues pointed out, but my code evolved to fix drawing pre-multiplied bitmaps.

[Stephan Aßmus]

Created attachment 48524 [details]
Fixes to the rect conversions and image rendering on Haiku

Patch properly created with "svn-create-patch", which adds the "namespaces". Sorry about that!

[Stephan Aßmus]

Created attachment 48526 [details]
Fixes to the rect conversions and image rendering on Haiku

Fixed patch, the old one didn't apply anymore because of the changes to AffineTransform.

[WebKit Review Bot]

Attachment 48526 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:51:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:52:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:53:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:54:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:55:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:56:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:57:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:58:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:59:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:60:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:61:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:62:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:63:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 13


If any of these errors are false positives, please file a bug against check-webkit-style.

[Stephan Aßmus]

Created attachment 48527 [details]
Fixes to the rect conversions and image rendering on Haiku

Renamed bpr -> bytesPerRow.

[WebKit Review Bot]

Attachment 48527 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:51:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:52:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:53:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:54:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:55:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:56:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:57:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:58:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:59:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:60:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:61:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:62:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp:63:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 13


If any of these errors are false positives, please file a bug against check-webkit-style.

[Stephan Aßmus]

Created attachment 48528 [details]
Fixes to the rect conversions and image rendering on Haiku

Of course I forgot to replace tabs with spaces for the new code I added meanwhile. Really sorry for the noise!

[WebKit Commit Bot]

Comment on attachment 48528 [details]
Fixes to the rect conversions and image rendering on Haiku

Clearing flags on attachment: 48528

Committed r54637: <http://trac.webkit.org/changeset/54637>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.