[webkit-reviews] review granted: [Bug 180008] Use more Uint8ClampedArray& and constness in filter and image buffer code : [Attachment 327572] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 180008: Use more Uint8ClampedArray& and constness in filter and image
buffer code
https://bugs.webkit.org/show_bug.cgi?id=180008

Attachment 327572: Patch

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




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

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

I probably would have used auto in  quite a few places instead of writing out
types

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:485
> +	       RetainPtr<CGContextRef> sourceContext =
adoptCF(CGBitmapContextCreate(reinterpret_cast<void*>(const_cast<uint8_t*>(srcR
ows)), width.unsafeGet(), height.unsafeGet(), 8, srcBytesPerRow, colorSpace,
kCGImageAlphaPremultipliedLast));

The reinterpret_cast should not be needed here: the const_cast alone should be
sufficient, because we can pass a uint8_t* as a void* without a cast.

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:494
> +	       srcRows = const_cast<const uint8_t*>(destRows);

A const_cast should not be needed to add const, only to remove it. We should be
able to assign to a variable of type const uint8_t* with uint8_t* without a
cast.

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:542
> +	   src.data = reinterpret_cast<void*>(const_cast<uint8_t*>(srcRows));

The reinterpret_cast should not be needed here: the const_cast alone should be
sufficient, because we can pass assign to a void* with a uint8_t* without a
cast.

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:571
> +	       srcRows = const_cast<const uint8_t*>(destRows);

A const_cast should not be needed to add const, only to remove it. We should be
able to assign to a variable of type const uint8_t* with uint8_t* without a
cast.

>
Source/WebCore/platform/graphics/cpu/arm/filters/FECompositeArithmeticNEON.h:47
> +    const uint32_t* sourcePixel = reinterpret_cast<uint32_t*>(source);

I believe this won’t compile unless we change the type inside reinterpret_cast
to "const uint32_t*", not just the type of the local.