[webkit-reviews] review denied: [Bug 34062] [Qt] Tuning and optimizations to GraphicsLayerQt : [Attachment 47963] Performance tuning to GraphicsLayerQt

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Noam Rosenthal
<noam.rosenthal at nokia.com>'s request for review:
Bug 34062: [Qt] Tuning and optimizations to GraphicsLayerQt
https://bugs.webkit.org/show_bug.cgi?id=34062

Attachment 47963: Performance tuning to GraphicsLayerQt
https://bugs.webkit.org/attachment.cgi?id=47963&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
Some minor comments

>  
>  class OpacityAnimationQt : public AnimationQt<qreal> {
>  public:
>      OpacityAnimationQt(GraphicsLayerQtImpl* layer, const KeyframeValueList&
values, const IntSize& boxSize, const Animation* anim, const QString & name)
> -	    : AnimationQt<qreal>(layer, values, boxSize, anim, name)
> +		   : AnimationQt<qreal>(layer, values, boxSize, anim, name)

Wrong indentation


> -    } else {
> +    } else
>	   for (QList<QWeakPointer<QAbstractAnimation> >::iterator it =
m_impl->m_animations.begin(); it != m_impl->m_animations.end(); ++it) {

The for spends multiply lines, so the else would need {! Please look at the
style guide, other examples include

correct:
if (true)
    doSomething

wrong:
if (true)
   // do something
   doSomething

correct:

if (true) {
   // do something
   doSomething
}


> @@ -129,6 +131,9 @@ public:
>      // compositor telling us to do so. We'll get that call from
ChromeClientQt

// compositor notifies us throught ChromeClientQt when we need to synchronize 
??? 

>      bool shouldSync;