Skip to content
Permalink
Browse files
2010-04-23 No'am Rosenthal <noam.rosenthal@nokia.com>
        Reviewed by Kenneth Rohde Christiansen.

        [Qt] [Performance] GraphicsLayer: constructing the layers takes too long
        https://bugs.webkit.org/show_bug.cgi?id=36365

        The issue came from using QGraphicsView's cache as is. The problem is that
        several code-paths require re-rendering of the item, but not re-rendering
        of the web content into the cache.

        The way to solve it is by having GraphicsLayerQt manage the cache directly
        via QPixmapCache, instead of using QGraphicsItem cache modes.

        FPS measurement shows significant improvement (20FPS before, 40FPS after)
        on several use-cases, including blog-files/leaves on a desktop environment.

        * platform/graphics/qt/GraphicsLayerQt.cpp:
        (WebCore::GraphicsLayerQtImpl::GraphicsLayerQtImpl):
        (WebCore::GraphicsLayerQtImpl::recache):
        (WebCore::GraphicsLayerQtImpl::paint):
        (WebCore::GraphicsLayerQtImpl::flushChanges):

Canonical link: https://commits.webkit.org/49470@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@58190 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
eseidel committed Apr 23, 2010
1 parent 09728a8 commit ae0fa7025df8c9e97d6681a23e98efe54b8ba9f8
Showing 2 changed files with 78 additions and 19 deletions.
@@ -1,3 +1,26 @@
2010-04-23 No'am Rosenthal <noam.rosenthal@nokia.com>

Reviewed by Kenneth Rohde Christiansen.

[Qt] [Performance] GraphicsLayer: constructing the layers takes too long
https://bugs.webkit.org/show_bug.cgi?id=36365

The issue came from using QGraphicsView's cache as is. The problem is that
several code-paths require re-rendering of the item, but not re-rendering
of the web content into the cache.

The way to solve it is by having GraphicsLayerQt manage the cache directly
via QPixmapCache, instead of using QGraphicsItem cache modes.

FPS measurement shows significant improvement (20FPS before, 40FPS after)
on several use-cases, including blog-files/leaves on a desktop environment.

* platform/graphics/qt/GraphicsLayerQt.cpp:
(WebCore::GraphicsLayerQtImpl::GraphicsLayerQtImpl):
(WebCore::GraphicsLayerQtImpl::recache):
(WebCore::GraphicsLayerQtImpl::paint):
(WebCore::GraphicsLayerQtImpl::flushChanges):

2010-04-23 James Robinson <jamesr@chromium.org>

Reviewed by Dimitri Glazkov.
@@ -41,6 +41,7 @@
#include <QtGui/qpainter.h>
#include <QtGui/qpalette.h>
#include <QtGui/qpixmap.h>
#include <QtGui/qpixmapcache.h>
#include <QtGui/qstyleoption.h>

namespace WebCore {
@@ -150,6 +151,12 @@ class GraphicsLayerQtImpl : public QGraphicsObject {
// let the compositor-API tell us which properties were changed
void notifyChange(ChangeMask);

// actual rendering of the web-content into a QPixmap
// We prefer to use our own caching because it gives us a higher level of granularity than QGraphicsItem cache modes -
// sometimes we need to cache the contents even `though the item needs to be updated, e.g. when the background-color is changed.
// TODO: investigate if QGraphicsItem caching can be improved to support that out of the box.
QPixmap recache(const QRegion&);

// called when the compositor is ready for us to show the changes on screen
// this is called indirectly from ChromeClientQt::setNeedsOneShotDrawingSynchronization
// (meaning the sync would happen together with the next draw)
@@ -203,6 +210,7 @@ public slots:
int m_changeMask;

QSizeF m_size;
QPixmapCache::Key m_backingStoreKey;
#ifndef QT_NO_ANIMATION
QList<QWeakPointer<QAbstractAnimation> > m_animations;
#endif
@@ -255,9 +263,6 @@ GraphicsLayerQtImpl::GraphicsLayerQtImpl(GraphicsLayerQt* newLayer)
// they are ignored and passed to the item below.
setEnabled(true);

// we'll set the cache when we know what's going on
setCacheMode(ItemCoordinateCache);

connect(this, SIGNAL(notifyAnimationStartedAsync()), this, SLOT(notifyAnimationStarted()), Qt::QueuedConnection);
}

@@ -283,6 +288,41 @@ GraphicsLayerQtImpl::~GraphicsLayerQtImpl()
#endif
}


QPixmap GraphicsLayerQtImpl::recache(const QRegion& regionToUpdate)
{
if (!m_layer->drawsContent())
return QPixmap();

QRegion region = regionToUpdate;
QPixmap pixmap;

// We might be drawing into an existing cache.
if (!QPixmapCache::find(m_backingStoreKey, &pixmap))
region = QRegion(boundingRect().toAlignedRect());

if (m_size != pixmap.size()) {
pixmap = QPixmap(m_size.toSize());
if (!m_layer->contentsOpaque())
pixmap.fill(Qt::transparent);
m_pendingContent.regionToUpdate = QRegion(QRect(QPoint(0, 0), m_size.toSize()));
}

QPainter painter(&pixmap);
GraphicsContext gc(&painter);

// Clear the area in cache that we're drawing into
painter.setCompositionMode(QPainter::CompositionMode_Clear);
painter.fillRect(region.boundingRect(), Qt::transparent);

// Render the actual contents into the cache
painter.setCompositionMode(QPainter::CompositionMode_SourceOver);
m_layer->paintGraphicsLayerContents(gc, region.boundingRect());

m_backingStoreKey = QPixmapCache::insert(pixmap);
return pixmap;
}

void GraphicsLayerQtImpl::updateTransform()
{
setBaseTransform(isTransformAnimationRunning() ? m_baseTransform : m_layer->transform());
@@ -370,9 +410,12 @@ void GraphicsLayerQtImpl::paint(QPainter* painter, const QStyleOptionGraphicsIte
switch (m_currentContent.contentType) {
case HTMLContentType:
if (m_state.drawsContent) {
// this is the expensive bit. we try to minimize calls to this area by proper caching
GraphicsContext gc(painter);
m_layer->paintGraphicsLayerContents(gc, option->rect);
QPixmap backingStore;
// 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()));
painter->drawPixmap(0, 0, backingStore);
}
break;
case PixmapContentType:
@@ -484,19 +527,13 @@ void GraphicsLayerQtImpl::flushChanges(bool recursive, bool forceUpdateTransform
update();
setFlag(ItemHasNoContents, false);

// no point in caching a directly composited pixmap into another pixmap
setCacheMode(NoCache);

break;
case MediaContentType:
setFlag(ItemHasNoContents, true);
setCacheMode(NoCache);
m_pendingContent.mediaLayer.data()->setParentItem(this);
break;

case ColorContentType:
// no point in caching a solid-color rectangle
setCacheMode(NoCache);
if (m_pendingContent.contentType != m_currentContent.contentType || m_pendingContent.contentsBackgroundColor != m_currentContent.contentsBackgroundColor)
update();
m_state.drawsContent = false;
@@ -509,12 +546,8 @@ void GraphicsLayerQtImpl::flushChanges(bool recursive, bool forceUpdateTransform
case HTMLContentType:
if (m_pendingContent.contentType != m_currentContent.contentType)
update();
if (!m_state.drawsContent && m_layer->drawsContent()) {
if (!m_state.drawsContent && m_layer->drawsContent())
update();
if (m_layer->drawsContent() && !m_maskEffect)
setCacheMode(ItemCoordinateCache);
} else if (!m_layer->drawsContent())
setCacheMode(NoCache);

setFlag(ItemHasNoContents, !m_layer->drawsContent());
break;
@@ -544,8 +577,12 @@ void GraphicsLayerQtImpl::flushChanges(bool recursive, bool forceUpdateTransform

if (m_maskEffect)
m_maskEffect.data()->update();
else if (m_changeMask & DisplayChange)
else if (m_changeMask & DisplayChange) {
// Recache now: all the content is ready and we don't want to wait until the paint event.
recache(m_pendingContent.regionToUpdate);
update(m_pendingContent.regionToUpdate.boundingRect());
m_pendingContent.regionToUpdate = QRegion();
}

if ((m_changeMask & BackgroundColorChange) && (m_pendingContent.backgroundColor != m_currentContent.backgroundColor))
update();
@@ -572,7 +609,6 @@ void GraphicsLayerQtImpl::flushChanges(bool recursive, bool forceUpdateTransform
m_currentContent.contentType = m_pendingContent.contentType;
m_currentContent.mediaLayer = m_pendingContent.mediaLayer;
m_currentContent.backgroundColor = m_pendingContent.backgroundColor;
m_currentContent.regionToUpdate |= m_pendingContent.regionToUpdate;
m_currentContent.contentsBackgroundColor = m_pendingContent.contentsBackgroundColor;
m_pendingContent.regionToUpdate = QRegion();
m_changeMask = NoChanges;

0 comments on commit ae0fa70

Please sign in to comment.