[webkit-reviews] review granted: [Bug 225584] Use PixelBuffer rather than ImageData in platform/ code to fix layering violation : [Attachment 428201] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sam Weinig <sam at webkit.org>'s request
for review:
Bug 225584: Use PixelBuffer rather than ImageData in platform/ code to fix
layering violation
https://bugs.webkit.org/show_bug.cgi?id=225584

Attachment 428201: Patch

https://bugs.webkit.org/attachment.cgi?id=428201&action=review




--- Comment #12 from Darin Adler <darin at apple.com> ---
Comment on attachment 428201
  --> https://bugs.webkit.org/attachment.cgi?id=428201
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=428201&action=review

> Source/WebCore/platform/graphics/PixelBuffer.cpp:39
> +    static constexpr unsigned bytesPerPixel = 4;

No need for "static" here.

> Source/WebCore/platform/graphics/filters/FilterEffect.cpp:575
> +    ASSERT(m_unmultipliedImageResult == WTF::nullopt);

I might use "!" or "!x.hasValue()" instead.

> Source/WebCore/platform/graphics/filters/FilterEffect.cpp:593
> +    ASSERT(m_premultipliedImageResult == WTF::nullopt);

Ditto.

> Source/WebCore/platform/graphics/filters/FilterEffect.h:30
> +#include "PixelBuffer.h"

Don’t need to both include and forward-declare PixelBuffer.

> Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp:587
> +    case DisplayList::ItemType::GetPixelBuffer: {
>	   ASSERT_NOT_REACHED();
>	   break;
>      }

No need for these braces. Also seems like we want "return WTF::nullopt" or
maybe not even bother with "break".

> Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.h:54
> +    void putPixelBuffer(WebCore::AlphaPremultiplication inputFormat, const
WebCore::WebCore::PixelBuffer&, const WebCore::IntRect& srcRect, const
WebCore::IntPoint& destPoint, WebCore::AlphaPremultiplication destFormat)
override;

Does this compile? WebCore::WebCore::PixelBuffer