LayoutTests/reflections should work in AC mode
Please follow the QtWebKit bug reporting guidelines when reporting bugs. See http://trac.webkit.org/wiki/QtWebKitBugs Specifically: - The 'QtWebKit' component should be used for bugs/features in the public QtWebKit API layer, not to signify that the bug is specific to the Qt port of WebKit http://trac.webkit.org/wiki/QtWebKitBugs#Component
*** Bug 34762 has been marked as a duplicate of this bug. ***
Created attachment 54200 [details] initial reflection support in GraphicsLayerQt
Comment on attachment 54200 [details] initial reflection support in GraphicsLayerQt WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:562 + replica->m_replicaEffect.data()->setOpacity(m_layer->opacity()); Out of curiousity: Isn't m_replicaEffect->setOpacity much easier to read than m_replicaEffect.data()->setOpacity? :)
hmmm... yes, for some reason I thought it was not supported, probably a brain lapse. Should I resubmit?
Created attachment 54866 [details] Revised documentation - all tests work with 4.7, though not all of them work with 4.6. btw I can't use m_replicaEffect-> instead of .data(). It just doesn't compile :)
Created attachment 54868 [details] Make sure the scene updates The previous patch only worked on FullViewportUpdateMode. Now fixed.
Created attachment 54869 [details] correct upload...
Comment on attachment 54869 [details] correct upload... Big patch, first comments. WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:56 + class ReplicaEffectQt : public QGraphicsEffect { Why that name? Why not ReflectionEffectQt or so? WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:61 + void renderMask(GraphicsLayer* layer, QPixmap& pixmap, const QRectF& boundingRect); you can leave out layer and pixmap, that is redundant, according to the style guide WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:62 + void draw(QPainter* painter); same here WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:66 + QRectF boundingRectFor(const QRectF& sourceRect) const; bounding rect for? WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:116 + bool isAncestorReplicated() const; hasReplicatedAncestor() ? WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:384 + if (scene()) This code here needs braces and its contents spans more than one non-logical line WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:404 + return qobject_cast<GraphicsLayerQtImpl*>(m_layer->parent()->platformLayer()->toGraphicsObject())->isAncestorReplicated(); maybe we need some accessors for this? WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:1528 + // FIXME: use a smaller pixmap. Maybe explain why
> + class ReplicaEffectQt : public QGraphicsEffect { > Why that name? Why not ReflectionEffectQt or so? because in GraphicsLayer.h it's called setReplicaLayer()... I don't mind changing it to reflection but I feel it doesn't really matter
Comment on attachment 54869 [details] correct upload... The current approach would be too slow on OpenGL due to the intermediate pixmaps. Researching other avenues.
Created attachment 56266 [details] Addressed Kenneth's comments and added some ChangeLog comments about OpenGL
Comment on attachment 56266 [details] Addressed Kenneth's comments and added some ChangeLog comments about OpenGL Need to revise the patch, because it conflicts with other already-accepted patches
Created attachment 57584 [details] Re-merged patch I tested the new patch with all the compositing layout tests, it doesn't break anything and makes reflections work.
Created attachment 57585 [details] Had an extra line-break
Comment on attachment 57585 [details] Had an extra line-break 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?
Assigning to Noam so he can land the patch with Kenneth's proposed changes.
Not adding new features to GraphicsLayerQt. TextureMapper would be a replacement.