[webkit-reviews] review granted: [Bug 232411] [GPU Process] Pattern should hold SourceImage which can be converted to a NativeImage only when needed : [Attachment 449994] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 232411: [GPU Process] Pattern should hold SourceImage which can be
converted to a NativeImage only when needed
https://bugs.webkit.org/show_bug.cgi?id=232411

Attachment 449994: Patch

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




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

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

> Source/WebCore/platform/graphics/SourceImage.cpp:108
> +	       return IntSize();

Maybe just use "{ }" instead of "IntSize()".

> Source/WebCore/platform/graphics/SourceImage.cpp:110
> +	       }
> +	   );

I would outdent these two lines by 4 spaces.

> Source/WebCore/platform/graphics/displaylists/DisplayListResourceHeap.h:45
> +    virtual std::optional<SourceImage>
getSourceImage(RenderingResourceIdentifier) const = 0;

WebKit coding style tells us to not use get for all these function names, but
it seems we have diverged from that tradition here in DisplayList.

> Source/WebCore/platform/graphics/displaylists/DisplayListResourceHeap.h:82
> +	       return SourceImage(*nativeImage);

I might have written:

    return { { *nativeImage } };

Or, if one of these works and helps with reference count churn (not sure either
works):

    return { { WTFMove(*nativeImage) } };
    return { { nativeImage.releaseNonNull() ) } };

> Source/WebCore/platform/graphics/displaylists/DisplayListResourceHeap.h:85
> +	       return SourceImage(*imageBuffer);

Ditto.

> Source/WebKit/GPUProcess/graphics/QualifiedResourceHeap.h:98
> +	       return WebCore::SourceImage(*nativeImage);

Ditto.

> Source/WebKit/GPUProcess/graphics/QualifiedResourceHeap.h:101
> +	       return WebCore::SourceImage(*imageBuffer);

Ditto.