[webkit-reviews] review granted: [Bug 35389] [Qt] GraphicsLayer: support reflections : [Attachment 57585] Had an extra line-break

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Noam Rosenthal
<noam.rosenthal at nokia.com>'s request for review:
Bug 35389: [Qt] GraphicsLayer: support reflections
https://bugs.webkit.org/show_bug.cgi?id=35389

Attachment 57585: Had an extra line-break
https://bugs.webkit.org/attachment.cgi?id=57585&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
r+ with the below changes.

 604		 if (m_layer->replicaLayer())
 605		     if (GraphicsLayerQtImpl* replica =
toGraphicsLayerQtImpl(m_layer->replicaLayer()->platformLayer()))
 606			 replica->m_reflectionEffect.data()->updateGeometry();

This would need braces.WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:1130
 +  
unnecessary newline

WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:1209
 +	    : QGraphicsEffect(parent)
too much indentation.

WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:1212
 +	    , m_opacity(1)
Shouldnt this be 1.0?

1224	 if (GraphicsLayer* maskLayer = layer->maskLayer())
 1225	      if (QGraphicsItem* maskLayerImpl = maskLayer->nativeLayer()) {
 1226		  QPainter maskPainter(&pixmap);
 1227		 
maskPainter.setCompositionMode(QPainter::CompositionMode_DestinationIn);
 1228		  QStyleOptionGraphicsItem option;
 1229		  option.exposedRect = option.rect = QRect(0, 0,
boundingRect.width(), boundingRect.height());
 1230		  maskLayerImpl->paint(&maskPainter, &option);
 1231	      }

The above is missing braces as well. Maybe invert the logic and add an early
return.


WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:1258
 +	QPixmap intermediatePixmap(painter->device()->width(),
painter->device()->height());
Isn't it better to get a handle of the device first, or is it implicitly
shared?