[Qt] GraphicsLayer: preserves-3d and backface visibility

• Component: Layout and Rendering
• Status: CLOSED FIXED

[Noam Rosenthal]

Currently Qt AC only supports 3D transforms and animations, but doesn't support full 3D with backface-visibility and preserves-3d. Work on it is ongoing.

[Noam Rosenthal]

Created attachment 49338 [details]
WIP: preserves-3d and backface-visibility

This is not ready to review, as it's still a bit buggy. 
http://webkit.org/blog-files/3d-transforms/morphing-cubes.html works fine, but http://webkit.org/blog-files/3d-transforms/poster-circle.html doesn't, for example.

[Tor Arne Vestbø]

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

[Kenneth Rohde Christiansen]

Why was this bug closed? Is the patch landed? or not needed anymore? If it is not obvious why you close a bug, please add a comment.

[Noam Rosenthal]

hm... not sure who closed this, if it was me it was by mistake.

[Noam Rosenthal]

Created attachment 54355 [details]
better way to do preserves-3D

[Noam Rosenthal]

Created attachment 54462 [details]
preserves-3d works with Qt!

This patch makes all the preserves-3d examples work. It's a good solution until we have full 3D in Qt.

[WebKit Review Bot]

Attachment 54462 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:288:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:355:  Missing space before ( in if(  [whitespace/parens] [5]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qgraphicswebview.cpp"
Total errors found: 2 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Noam Rosenthal]

Comment on attachment 54462 [details]
preserves-3d works with Qt!

uploaded wrong patch

[Noam Rosenthal]

Created attachment 54488 [details]
uploading correct patch

[Kenneth Rohde Christiansen]

Comment on attachment 54488 [details]
uploading correct patch

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index dfd08fb..07cefb0 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,22 @@
> [phone]  Noam Rosenthal  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        [Qt] GraphicsLayer: preserves-3d and backface visibility
> +        https://bugs.webkit.org/show_bug.cgi?id=35312
> +
> +        Implement preserves-3d by maintaining the 3D transformation heirarchy inside GraphicsLayerQt, and extrapolating
> +        the relative QTransform. When the extrapolation fails (un-invertible matrix) we ignore the transformation change.
> +
> +        WebKitSite/blog-files/3d-transforms test now work with Qt.
> +
> +        * platform/graphics/qt/GraphicsLayerQt.cpp:
> +        (WebCore::GraphicsLayerQtImpl::updateTransform):
> +        (WebCore::GraphicsLayerQtImpl::opaqueArea):
> +        (WebCore::GraphicsLayerQtImpl::boundingRect):
> +        (WebCore::GraphicsLayerQtImpl::paint):
> +        (WebCore::GraphicsLayerQtImpl::flushChanges):
> +
>  2010-04-27  Adam Barth  <[e-mail]>
>  
>          Reviewed by Eric Seidel.
> diff --git a/WebCore/platform/graphics/qt/GraphicsLayerQt.cpp b/WebCore/platform/graphics/qt/GraphicsLayerQt.cpp
> index a31d045..1c4c275 100644
> --- a/WebCore/platform/graphics/qt/GraphicsLayerQt.cpp
> +++ b/WebCore/platform/graphics/qt/GraphicsLayerQt.cpp
> @@ -32,14 +32,11 @@
>  #include <QtCore/qmetaobject.h>
>  #include <QtCore/qset.h>
>  #include <QtCore/qtimer.h>
> -#include <QtGui/qbitmap.h>
>  #include <QtGui/qcolor.h>
>  #include <QtGui/qgraphicseffect.h>
>  #include <QtGui/qgraphicsitem.h>
>  #include <QtGui/qgraphicsscene.h>
> -#include <QtGui/qmatrix4x4.h>
>  #include <QtGui/qpainter.h>
> -#include <QtGui/qpalette.h>
>  #include <QtGui/qpixmap.h>
>  #include <QtGui/qpixmapcache.h>
>  #include <QtGui/qstyleoption.h>
> @@ -135,6 +132,8 @@ public:
>      // the compositor lets us special-case images and colors, so we try to do so
>      enum StaticContentType { HTMLContentType, PixmapContentType, ColorContentType, MediaContentType};
>  
> +    const GraphicsLayerQtImpl* rootLayer() const;
> +
>      GraphicsLayerQtImpl(GraphicsLayerQt* newLayer);
>      virtual ~GraphicsLayerQtImpl();
>  
> @@ -143,9 +142,8 @@ public:
>      virtual QRectF boundingRect() const;
>      virtual void paint(QPainter*, const QStyleOptionGraphicsItem*, QWidget*);
>  
> -    // we manage transforms ourselves because transform-origin acts differently in webkit and in Qt
> +    // We manage transforms ourselves because transform-origin acts differently in webkit and in Qt, and we need it as a fallback in case we encounter an un-invertible matrix
>      void setBaseTransform(const TransformationMatrix&);
> -    QTransform computeTransform(const TransformationMatrix& baseTransform) const;
>      void updateTransform();
>  
>      // let the compositor-API tell us which properties were changed
> @@ -163,10 +161,6 @@ public:
>      // or ChromeClientQt::scheduleCompositingLayerSync (meaning the sync will happen ASAP)
>      void flushChanges(bool recursive = true, bool forceTransformUpdate = false);
>  
> -    // optimization: returns true if this or an ancestor has a transform animation running.
> -    // this enables us to use ItemCoordinatesCache while the animation is running, otherwise we have to recache for every frame
> -    bool isTransformAnimationRunning() const;
> -
>  public slots:
>      // we need to notify the client (aka the layer compositor) when the animation actually starts
>      void notifyAnimationStarted();
> @@ -182,6 +176,7 @@ public:
>      GraphicsLayerQt* m_layer;
>  
>      TransformationMatrix m_baseTransform;
> +    TransformationMatrix m_transformRelativeToRootLayer;
>      bool m_transformAnimationRunning;
>      bool m_opacityAnimationRunning;
>      QWeakPointer<MaskEffectQt> m_maskEffect;
> @@ -288,6 +283,14 @@ GraphicsLayerQtImpl::~GraphicsLayerQtImpl()
>  #endif
>  }
>  
> +const GraphicsLayerQtImpl* GraphicsLayerQtImpl::rootLayer() const
> +{
> +    if (const GraphicsLayerQtImpl* parent = qobject_cast<const GraphicsLayerQtImpl*>(parentObject()))
> +        return parent->rootLayer();
> +    return this;
> +}
> +
> +
>  
>  QPixmap GraphicsLayerQtImpl::recache(const QRegion& regionToUpdate)
>  {
> @@ -299,7 +302,7 @@ QPixmap GraphicsLayerQtImpl::recache(const QRegion& regionToUpdate)
>  
>      // We might be drawing into an existing cache.
>      if (!QPixmapCache::find(m_backingStoreKey, &pixmap))
> -        region = QRegion(boundingRect().toAlignedRect());
> +        region = QRegion(QRect(0, 0, m_size.width(), m_size.height()));
>  
>      if (m_size != pixmap.size()) {
>          pixmap = QPixmap(m_size.toSize());
> @@ -325,63 +328,93 @@ QPixmap GraphicsLayerQtImpl::recache(const QRegion& regionToUpdate)
>  
>  void GraphicsLayerQtImpl::updateTransform()
>  {
> -    setBaseTransform(isTransformAnimationRunning() ? m_baseTransform : m_layer->transform());
> -}
> -
> -void GraphicsLayerQtImpl::setBaseTransform(const TransformationMatrix& baseTransform)
> -{
> -    m_baseTransform = baseTransform;
> -    setTransform(computeTransform(baseTransform));
> -}
> -
> -QTransform GraphicsLayerQtImpl::computeTransform(const TransformationMatrix& baseTransform) const
> -{
> -    if (!m_layer)
> -        return baseTransform;
> -
> -    TransformationMatrix computedTransform;
> +    if (!m_transformAnimationRunning)
> +        m_baseTransform = m_layer->transform();
>  
> -    // The origin for childrenTransform is always the center of the ancestor which contains the childrenTransform.
> -    // this has to do with how WebCore implements -webkit-perspective and -webkit-perspective-origin, which are the CSS
> -    // attribute that call setChildrenTransform
> -    QPointF offset = -pos() - boundingRect().bottomRight() / 2;
> +    TransformationMatrix localTransform;
>  
> -    for (const GraphicsLayerQtImpl* ancestor = this; (ancestor = qobject_cast<GraphicsLayerQtImpl*>(ancestor->parentObject())); ) {
> -        if (!ancestor->m_state.childrenTransform.isIdentity()) {
> -            const QPointF offset = mapFromItem(ancestor, QPointF(ancestor->m_size.width() / 2, ancestor->m_size.height() / 2));
> -            computedTransform
> -                .translate(offset.x(), offset.y())
> -                .multLeft(ancestor->m_state.childrenTransform)
> -                .translate(-offset.x(), -offset.y());
> -            break;
> -        }
> -    }
> +    GraphicsLayerQtImpl* parent = qobject_cast<GraphicsLayerQtImpl*>(parentObject());
>  
>      // webkit has relative-to-size originPoint, graphics-view has a pixel originPoint, here we convert
>      // we have to manage this ourselves because QGraphicsView's transformOrigin is incompatible
>      const qreal originX = m_state.anchorPoint.x() * m_size.width();
>      const qreal originY = m_state.anchorPoint.y() * m_size.height();
> -    computedTransform
> -            .translate3d(originX, originY, m_state.anchorPoint.z())
> -            .multLeft(baseTransform)
> +
> +    // We ignore QGraphicsItem::pos completely, and use only transforms - because we have to maintain that ourselves for 3D.
> +    localTransform
> +            .translate3d(originX + m_state.pos.x(), originY + m_state.pos.y(), m_state.anchorPoint.z())
> +            .multLeft(m_baseTransform)
>              .translate3d(-originX, -originY, -m_state.anchorPoint.z());
>  
> -    // now we project to 2D
> -    return QTransform(computedTransform);
> +    // This is the actual 3D transform of this item, with the ancestors' transform baked in.
> +    m_transformRelativeToRootLayer = TransformationMatrix(parent ? parent->m_transformRelativeToRootLayer : TransformationMatrix())
> +                                         .multLeft(localTransform);
> +
> +    // Now we have enough information to determine if the layer is facing backwards.
> +    if (!m_state.backfaceVisibility && m_transformRelativeToRootLayer.inverse().m33() < 0) {
> +        setVisible(false);
> +        // No point in making extra calculations for invisible elements.
> +        return;
> +    }
> +
> +    // Simplistic depth test - we stack the item behind its parent if its computed z is lower than the parent's computed z at the item's center point.
> +    if (parent) {
> +        const QPointF centerPointMappedToRoot = rootLayer()->mapFromItem(this, m_size.width() / 2, m_size.height() / 2);
> +        setFlag(ItemStacksBehindParent,
> +                m_transformRelativeToRootLayer.mapPoint(FloatPoint3D(centerPointMappedToRoot.x(), centerPointMappedToRoot.y(), 0)).z() <
> +                parent->m_transformRelativeToRootLayer.mapPoint(FloatPoint3D(centerPointMappedToRoot.x(), centerPointMappedToRoot.y(), 0)).z());
> +    }
> +
> +    // The item is front-facing or backface-visibility is on.
> +    setVisible(true);
> +
> +    // Flatten to 2D-space of this item if it doesn't preserve 3D.
> +    if (!m_state.preserves3D) {
> +        m_transformRelativeToRootLayer.setM13(0);
> +        m_transformRelativeToRootLayer.setM23(0);
> +        m_transformRelativeToRootLayer.setM31(0);
> +        m_transformRelativeToRootLayer.setM32(0);
> +        m_transformRelativeToRootLayer.setM33(1);
> +        m_transformRelativeToRootLayer.setM34(0);
> +        m_transformRelativeToRootLayer.setM43(0);
> +    }
> +
> +    // Apply perspective for the use of this item's children. Perspective is always applied from the item's center.
> +    if (!m_state.childrenTransform.isIdentity())
> +        m_transformRelativeToRootLayer
> +            .translate(m_size.width() / 2, m_size.height() /2)
> +            .multLeft(m_state.childrenTransform)
> +            .translate(-m_size.width() / 2, -m_size.height() /2);
> +
> +    bool inverseOk = true;
> +    // Use QTransform::inverse to extrapolate the relative transform of this item, based on the parent's transform relative to
> +    // the root layer and the desired transform for this item relative to the root layer.
> +    const QTransform parentTransform = parent ? parent->itemTransform(rootLayer()) : QTransform();
> +    const QTransform transform2D = QTransform(m_transformRelativeToRootLayer) * parentTransform.inverted(&inverseOk);
> +
> +    // In rare cases the transformation cannot be inversed - in that case we don't apply the transformation at all, otherwise we'd flicker.
> +    // FIXME: This should be amended when Qt moves to a real 3D scene-graph.
> +    if (!inverseOk)
> +        return;
> +
> +    setTransform(transform2D);
> +
> +    const QList<QGraphicsItem*> children = childItems();
> +    for (QList<QGraphicsItem*>::const_iterator it = children.begin(); it != children.end(); ++it)
> +        if (GraphicsLayerQtImpl* layer= qobject_cast<GraphicsLayerQtImpl*>((*it)->toGraphicsObject()))
> +            layer->updateTransform();
>  }
>  
> -bool GraphicsLayerQtImpl::isTransformAnimationRunning() const
> +void GraphicsLayerQtImpl::setBaseTransform(const TransformationMatrix& baseTransform)
>  {
> -    if (m_transformAnimationRunning)
> -        return true;
> -    if (GraphicsLayerQtImpl* parent = qobject_cast<GraphicsLayerQtImpl*>(parentObject()))
> -        return parent->isTransformAnimationRunning();
> -    return false;
> +    m_baseTransform = baseTransform;
> +    updateTransform();
>  }
>  
>  QPainterPath GraphicsLayerQtImpl::opaqueArea() const
>  {
>      QPainterPath painterPath;
> +
>      // we try out best to return the opaque area, maybe it will help graphics-view render less items
>      if (m_currentContent.backgroundColor.isValid() && m_currentContent.backgroundColor.alpha() == 0xff)
>          painterPath.addRect(boundingRect());
> @@ -411,10 +444,9 @@ void GraphicsLayerQtImpl::paint(QPainter* painter, const QStyleOptionGraphicsIte
>      case HTMLContentType:
>          if (m_state.drawsContent) {
>              QPixmap backingStore;
> -            // We might need to recache, in case we try to paint and the cache
> -            // was purged (e.g. if it was full).
> +            // We might need to recache, in case we try to paint and the cache was purged (e.g. if it was full).
>              if (!QPixmapCache::find(m_backingStoreKey, &backingStore) || backingStore.size() != m_size.toSize())
> -                backingStore = recache(QRegion(boundingRect().toAlignedRect()));
> +                backingStore = recache(QRegion(m_state.contentsRect));
>              painter->drawPixmap(0, 0, backingStore);
>          }
>          break;
> @@ -500,9 +532,6 @@ void GraphicsLayerQtImpl::flushChanges(bool recursive, bool forceUpdateTransform
>          }
>      }
>  
> -    if ((m_changeMask & PositionChange) && (m_layer->position() != m_state.pos))
> -        setPos(m_layer->position().x(), m_layer->position().y());
> -
>      if (m_changeMask & SizeChange) {
>          if (m_layer->size() != m_state.size) {
>              prepareGeometryChange();
> @@ -515,7 +544,7 @@ void GraphicsLayerQtImpl::flushChanges(bool recursive, bool forceUpdateTransform
>          if (scene())
>              scene()->update();
>  
> -    if (m_changeMask & (ChildrenTransformChange | Preserves3DChange | TransformChange | AnchorPointChange | SizeChange)) {
> +    if (m_changeMask & (ChildrenTransformChange | Preserves3DChange | TransformChange | AnchorPointChange | SizeChange | BackfaceVisibilityChange | PositionChange)) {
>          // due to the differences between the way WebCore handles transforms and the way Qt handles transforms,
>          // all these elements affect the transforms of all the descendants.
>          forceUpdateTransform = true;
> @@ -587,9 +616,6 @@ void GraphicsLayerQtImpl::flushChanges(bool recursive, bool forceUpdateTransform
>      if ((m_changeMask & BackgroundColorChange) && (m_pendingContent.backgroundColor != m_currentContent.backgroundColor))
>          update();
>  
> -    // FIXME: the following flags are currently not handled, as they don't have a clear test or are in low priority
> -    // GeometryOrientationChange, ContentsOrientationChange, BackfaceVisibilityChange, ChildrenTransformChange, Preserves3DChange
> -
>      m_state.maskLayer = m_layer->maskLayer();
>      m_state.pos = m_layer->position();
>      m_state.anchorPoint = m_layer->anchorPoint();
WebCore/ChangeLog:9
 +          the relative QTransform. When the extrapolation fails (un-invertible matrix) we ignore the transformation change.
Is this the same behaviour as other platforms? (mac)

[Kenneth Rohde Christiansen]

Oh sorry, new review system :-)

[Noam Rosenthal]

*** Bug 38074 has been marked as a duplicate of this bug. ***

[WebKit Commit Bot]

Comment on attachment 54488 [details]
uploading correct patch

Clearing flags on attachment: 54488

Committed r58408: <http://trac.webkit.org/changeset/58408>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/58408 might have broken SnowLeopard Intel Release (Tests)

[Simon Hausmann]

Revision r58408 cherry-picked into qtwebkit-2.0 with commit d310f9fc02e82ab25aa251c480ac5b000ef6368b