[webkit-reviews] review granted: [Bug 33808] Animated scaling of background-image is too slow : [Attachment 47549] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Oliver Hunt
<oliver at apple.com>'s request for review:
Bug 33808: Animated scaling of background-image is too slow
https://bugs.webkit.org/show_bug.cgi?id=33808

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>

> +    bool isIdentityOrTranslation() const
> +    {
> +	   return m_matrix[0][0] == 1 && m_matrix[0][1] == 0 && m_matrix[0][2]
== 0 && m_matrix[0][3] == 0 &&
> +	   m_matrix[1][0] == 0 && m_matrix[1][1] == 1 && m_matrix[1][2] == 0 &&
m_matrix[1][3] == 0 &&
> +	   m_matrix[2][0] == 0 && m_matrix[2][1] == 0 && m_matrix[2][2] == 1 &&
m_matrix[2][3] == 0 &&
> +	   m_matrix[3][3] == 1;

Since you moved this you should fix the style (indentation on the following
lines, and && at the stat of the line).

> diff --git a/WebCore/rendering/RenderBoxModelObject.cpp
b/WebCore/rendering/RenderBoxModelObject.cpp

> +    const IntSize& size() const { return m_size; }
> +    double time() const { return m_time; }

What is "time"? Would be good to rename to make it more understandable. Yes,
you're just moving this but I think it's a good opportunity to improve it.

I prefer getters and setters next to eachother.

> +    static void highQualityRepaintTimerFired(RenderBoxModelObject* object)
> +    {
> +	   RenderBoxModelScaleObserver::boxModelObjectDestroyed(object);
> +	   object->repaint();

Can this repaint be optimized to only repaint the part of the box covered by
the image?

r=me with those changes.