[Webkit-unassigned] [Bug 79873] [BlackBerry] Upstream LayerCompositingThread.{h, cpp}

[bugzilla-daemon at webkit.org]

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


Rob Buis <rwlbuis at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #129659|review?                     |review-
               Flag|                            |




--- Comment #5 from Rob Buis <rwlbuis at gmail.com>  2012-03-05 07:16:36 PST ---
(From update of attachment 129659)
View in context: https://bugs.webkit.org/attachment.cgi?id=129659&action=review

Looks good, can be improved some more.

> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp:39
> +#include "HTMLCanvasElement.h"

Is this one actually used? Please check all includes in general.

> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp:152
> +    result.setP2(drawTransform.mapPoint(FloatPoint(x, y+h)));

Watch spacing "y + h"

> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp:153
> +    result.setP3(drawTransform.mapPoint(FloatPoint(x+w, y+h)));

Ditto.

> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp:162
> +static bool enableVideoClipping = false;

Can be moved into the function below, only used there.

> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp:240
> +            m_mediaPlayer->paint(0, IntRect((int)(x+0.5), (int)(y+0.5), m_bounds.width(), m_bounds.height()));

Take care of the spacing please,  x+0.5 -> x + 0.5 etc.

> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp:285
> +            // FIXME: is this going to happen?

Might require an ASSERT if we are quite sure it will not happen.

> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp:563
> +// FIXME: Copy pasted from WebCore::KeyframeAnimation!!!

I am seeing this kind of FIXME a lot. Maybe we need a PR to track this?

> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.h:57
> +class LayerMessage;

Not used?

> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.h:155
> +    }

Can be put on one line like above : size_t numSublayers() const { return m_sublayers.size(); }

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