[webkit-reviews] review granted: [Bug 77582] [Chromium] Use the current clip when marking paints as opaque : [Attachment 125272] Patch

[bugzilla-daemon at webkit.org]

Stephen White <senorblanco at chromium.org> has granted Dana Jansens
<danakj at chromium.org>'s request for review:
Bug 77582: [Chromium] Use the current clip when marking paints as opaque
https://bugs.webkit.org/show_bug.cgi?id=77582

Attachment 125272: Patch
https://bugs.webkit.org/attachment.cgi?id=125272&action=review

------- Additional Comments from Stephen White <senorblanco at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=125272&action=review


Looks good as-is; the FYI is optional.	r=me

> Source/WebCore/platform/graphics/skia/OpaqueRegionSkia.cpp:260
> +    SkIRect deviceClip;
> +    context->canvas()->getClipDeviceBounds(&deviceClip);
> +    SkRect rect = paintRect;
> +    if (!rect.intersect(SkIntToScalar(deviceClip.fLeft),
SkIntToScalar(deviceClip.fTop), SkIntToScalar(deviceClip.fRight),
SkIntToScalar(deviceClip.fBottom)))

FYI, you could also do something like this:

SkIRect deviceIClip;
context->canvas()->getClipDeviceBounds(&deviceIClip);
SkRect deviceClip;
deviceClip.set(deviceIClip);
if (!rect.intersect(deviceClip))

...

Not shorter in line count, but it does save you doing all those SkIntToScalars
yourself.