[Webkit-unassigned] [Bug 23526] clipToImageBuffer() for all platforms

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=23526





--- Comment #36 from Nikolas Zimmermann <zimmermann at kde.org>  2011-02-23 10:46:57 PST ---
(From update of attachment 83492)
View in context: https://bugs.webkit.org/attachment.cgi?id=83492&action=review

Looks great to me, I'll leave some notes, as I'm not yet certain whether it's r+, as I'm not too familiar with the Cairo port.

> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:240
> +    if (maskInformation.valid()) {

s/valid/isValid/

> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:255
> +    cairo_restore(m_data->cr);
> +    m_data->restore();

Why do you need, to do anything if maskInformation was not valid at this point? If no mask is used at all, you're now doing one cairo_restore/restore more, is that desired/needed?

> Source/WebCore/platform/graphics/cairo/GraphicsContextPlatformPrivateCairo.h:59
> +    bool valid() const { return m_maskSurface; }

s/valid/isValid/

> Source/WebCore/platform/graphics/cairo/GraphicsContextPlatformPrivateCairo.h:125
>      ContextShadow shadow;
>      Vector<ContextShadow> shadowStack;
> +    Vector<ImageMaskInformation> m_maskImageStack;

This is inconsistent, either make all of them private (I think this is out of scope for this bug), or just name it maskImageStack as well, and put a FIXME: Make these private. above the ContextShadow line.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.