Title: [105737] trunk/Source
Revision
105737
Author
noam.rosent...@nokia.com
Date
2012-01-24 07:17:11 -0800 (Tue, 24 Jan 2012)

Log Message

[Qt][WK2] Qt's cross-process AC copies images excessively when updating tiles.
https://bugs.webkit.org/show_bug.cgi?id=76877

Reviewed by Kenneth Rohde Christiansen.

Source/WebCore:

Add BitmapTexture::updateRawContents(), which allows uploading image data to a texture
without changing its format or swizzling RGB. The data has to be in the texture's native
format.

No new tests, this affects performance on all existing tests.

* platform/graphics/opengl/TextureMapperGL.cpp:
(WebCore::BitmapTextureGL::updateRawContents):
* platform/graphics/qt/TextureMapperQt.cpp:
* platform/graphics/texmap/TextureMapper.h:
(WebCore::BitmapTexture::updateRawContents):
* platform/graphics/texmap/TextureMapperNode.cpp:
(WebCore::TextureMapperNode::setContentsTileBackBuffer):
* platform/graphics/texmap/TextureMapperNode.h:

Source/WebKit2:

Instead of copying QImages, pass a reference to the ShareableBitmap that was originally
created by the web process. Also, swizzle the image's RGB in the web process, before it's
transferred to the UI process.

Data flow before change:
(Web Process) Render to image -> IPC -> (UI Process) Copy (render queue) -> Copy Swizzled RGB -> Upload

Data flow after change:
(Web Process) Render to image -> swizzle RGB -> IPC -> (UI Process) -> Upload

* Shared/ShareableBitmap.h:
* Shared/qt/ShareableBitmapQt.cpp:
(WebKit::ShareableBitmap::swizzleRGB):
* UIProcess/LayerTreeHostProxy.h:
* UIProcess/qt/LayerTreeHostProxyQt.cpp:
(WebKit::LayerTreeHostProxy::updateTile):
(WebKit::LayerTreeHostProxy::createImage):
(WebKit::LayerTreeHostProxy::syncRemoteContent):
(WebKit::LayerTreeHostProxy::updateTileForLayer):
(WebKit::LayerTreeHostProxy::createDirectlyCompositedImage):
(WebKit::LayerTreeHostProxy::purgeGLResources):
* WebProcess/WebPage/TiledBackingStoreRemoteTile.cpp:
(WebKit::TiledBackingStoreRemoteTile::updateBackBuffer):
* WebProcess/WebPage/qt/LayerTreeHostQt.cpp:
(WebKit::LayerTreeHostQt::adoptImageBackingStore):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (105736 => 105737)


--- trunk/Source/WebCore/ChangeLog	2012-01-24 14:53:31 UTC (rev 105736)
+++ trunk/Source/WebCore/ChangeLog	2012-01-24 15:17:11 UTC (rev 105737)
@@ -1,3 +1,25 @@
+2012-01-24  No'am Rosenthal  <noam.rosent...@nokia.com>
+
+        [Qt][WK2] Qt's cross-process AC copies images excessively when updating tiles.
+        https://bugs.webkit.org/show_bug.cgi?id=76877
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        Add BitmapTexture::updateRawContents(), which allows uploading image data to a texture
+        without changing its format or swizzling RGB. The data has to be in the texture's native
+        format.
+
+        No new tests, this affects performance on all existing tests.
+
+        * platform/graphics/opengl/TextureMapperGL.cpp:
+        (WebCore::BitmapTextureGL::updateRawContents):
+        * platform/graphics/qt/TextureMapperQt.cpp:
+        * platform/graphics/texmap/TextureMapper.h:
+        (WebCore::BitmapTexture::updateRawContents):
+        * platform/graphics/texmap/TextureMapperNode.cpp:
+        (WebCore::TextureMapperNode::setContentsTileBackBuffer):
+        * platform/graphics/texmap/TextureMapperNode.h:
+
 2012-01-24  Ilya Tikhonovsky  <loi...@chromium.org>
 
         Web Inspector: inspector/debugger/dom-breakpoints.html started to fail after r105642

Modified: trunk/Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp (105736 => 105737)


--- trunk/Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp	2012-01-24 14:53:31 UTC (rev 105736)
+++ trunk/Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp	2012-01-24 15:17:11 UTC (rev 105737)
@@ -347,6 +347,7 @@
     void setTextureMapper(TextureMapperGL* texmap) { m_textureMapper = texmap; }
 
     void updateContents(PixelFormat, const IntRect&, void*);
+    void updateRawContents(const IntRect&, const void*);
     void pack()
     {
         // This is currently a stub.
@@ -737,6 +738,13 @@
     GL_CMD(glTexSubImage2D(GL_TEXTURE_2D, 0, rect.x(), rect.y(), rect.width(), rect.height(), glFormat, GL_UNSIGNED_BYTE, bits))
 }
 
+void BitmapTextureGL::updateRawContents(const IntRect& rect, const void* bits)
+{
+    GL_CMD(glBindTexture(GL_TEXTURE_2D, m_id))
+    GLuint glFormat = isOpaque() ? GL_RGB : GL_RGBA;
+    GL_CMD(glTexSubImage2D(GL_TEXTURE_2D, 0, rect.x(), rect.y(), rect.width(), rect.height(), glFormat, GL_UNSIGNED_BYTE, bits))
+}
+
 void BitmapTextureGL::setContentsToImage(Image* image)
 {
     ImageUID uid = image ? uidForImage(image) : 0;

Modified: trunk/Source/WebCore/platform/graphics/qt/TextureMapperQt.cpp (105736 => 105737)


--- trunk/Source/WebCore/platform/graphics/qt/TextureMapperQt.cpp	2012-01-24 14:53:31 UTC (rev 105736)
+++ trunk/Source/WebCore/platform/graphics/qt/TextureMapperQt.cpp	2012-01-24 15:17:11 UTC (rev 105737)
@@ -76,7 +76,6 @@
     m_painter.end();
 }
 
-
 bool BitmapTextureQt::save(const String& path)
 {
     return m_pixmap.save(path, "PNG");

Modified: trunk/Source/WebCore/platform/graphics/texmap/TextureMapper.h (105736 => 105737)


--- trunk/Source/WebCore/platform/graphics/texmap/TextureMapper.h	2012-01-24 14:53:31 UTC (rev 105736)
+++ trunk/Source/WebCore/platform/graphics/texmap/TextureMapper.h	2012-01-24 15:17:11 UTC (rev 105737)
@@ -71,6 +71,7 @@
     // For performance reasons, BitmapTexture might modify the bits directly (swizzle).
     // Thus, this method is only recommended for buffer update, such as used by WebKit2.
     virtual void updateContents(PixelFormat, const IntRect&, void* bits) = 0;
+    virtual void updateRawContents(const IntRect&, const void* bits) { }
     virtual PlatformGraphicsContext* beginPaintMedia()
     {
         return beginPaint(IntRect(0, 0, size().width(), size().height()));

Modified: trunk/Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp (105736 => 105737)


--- trunk/Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp	2012-01-24 14:53:31 UTC (rev 105736)
+++ trunk/Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp	2012-01-24 15:17:11 UTC (rev 105737)
@@ -481,7 +481,7 @@
     }
 }
 
-void TextureMapperNode::setContentsTileBackBuffer(int id, const IntRect& sourceRect, const IntRect& targetRect, void* bits, BitmapTexture::PixelFormat format)
+void TextureMapperNode::setContentsTileBackBuffer(int id, const IntRect& sourceRect, const IntRect& targetRect, const void* bits)
 {
     ASSERT(m_textureMapper);
 
@@ -498,7 +498,7 @@
     if (!tile.backBuffer.texture)
         tile.backBuffer.texture = m_textureMapper->createTexture();
     tile.backBuffer.texture->reset(sourceRect.size(), false);
-    tile.backBuffer.texture->updateContents(format, sourceRect, bits);
+    tile.backBuffer.texture->updateRawContents(sourceRect, bits);
     tile.isBackBufferUpdated = true;
 }
 

Modified: trunk/Source/WebCore/platform/graphics/texmap/TextureMapperNode.h (105736 => 105737)


--- trunk/Source/WebCore/platform/graphics/texmap/TextureMapperNode.h	2012-01-24 14:53:31 UTC (rev 105736)
+++ trunk/Source/WebCore/platform/graphics/texmap/TextureMapperNode.h	2012-01-24 15:17:11 UTC (rev 105737)
@@ -164,7 +164,7 @@
     void setTileOwnership(TileOwnership ownership) { m_state.tileOwnership = ownership; }
     int createContentsTile(float scale);
     void removeContentsTile(int id);
-    void setContentsTileBackBuffer(int id, const IntRect& sourceRect, const IntRect& targetRect, void* bits, BitmapTexture::PixelFormat);
+    void setContentsTileBackBuffer(int id, const IntRect& sourceRect, const IntRect& targetRect, const void* bits);
     void setTileBackBufferTextureForDirectlyCompositedImage(int id, const IntRect& sourceRect, const FloatRect& targetRect, BitmapTexture*);
     void clearAllDirectlyCompositedImageTiles();
     void purgeNodeTexturesRecursive();

Modified: trunk/Source/WebKit2/ChangeLog (105736 => 105737)


--- trunk/Source/WebKit2/ChangeLog	2012-01-24 14:53:31 UTC (rev 105736)
+++ trunk/Source/WebKit2/ChangeLog	2012-01-24 15:17:11 UTC (rev 105737)
@@ -1,3 +1,36 @@
+2012-01-24  No'am Rosenthal  <noam.rosent...@nokia.com>
+
+        [Qt][WK2] Qt's cross-process AC copies images excessively when updating tiles.
+        https://bugs.webkit.org/show_bug.cgi?id=76877
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        Instead of copying QImages, pass a reference to the ShareableBitmap that was originally
+        created by the web process. Also, swizzle the image's RGB in the web process, before it's
+        transferred to the UI process.
+
+        Data flow before change:
+        (Web Process) Render to image -> IPC -> (UI Process) Copy (render queue) -> Copy Swizzled RGB -> Upload
+
+        Data flow after change:
+        (Web Process) Render to image -> swizzle RGB -> IPC -> (UI Process) -> Upload
+
+        * Shared/ShareableBitmap.h:
+        * Shared/qt/ShareableBitmapQt.cpp:
+        (WebKit::ShareableBitmap::swizzleRGB):
+        * UIProcess/LayerTreeHostProxy.h:
+        * UIProcess/qt/LayerTreeHostProxyQt.cpp:
+        (WebKit::LayerTreeHostProxy::updateTile):
+        (WebKit::LayerTreeHostProxy::createImage):
+        (WebKit::LayerTreeHostProxy::syncRemoteContent):
+        (WebKit::LayerTreeHostProxy::updateTileForLayer):
+        (WebKit::LayerTreeHostProxy::createDirectlyCompositedImage):
+        (WebKit::LayerTreeHostProxy::purgeGLResources):
+        * WebProcess/WebPage/TiledBackingStoreRemoteTile.cpp:
+        (WebKit::TiledBackingStoreRemoteTile::updateBackBuffer):
+        * WebProcess/WebPage/qt/LayerTreeHostQt.cpp:
+        (WebKit::LayerTreeHostQt::adoptImageBackingStore):
+
 2012-01-24  Rafael Brandao  <rafael.l...@openbossa.org>
 
         [Qt][WK2] QtWebIconDatabaseClient leaves a dangling pointer on WebIconDatabase after its destruction

Modified: trunk/Source/WebKit2/Shared/ShareableBitmap.h (105736 => 105737)


--- trunk/Source/WebKit2/Shared/ShareableBitmap.h	2012-01-24 14:53:31 UTC (rev 105736)
+++ trunk/Source/WebKit2/Shared/ShareableBitmap.h	2012-01-24 15:17:11 UTC (rev 105737)
@@ -135,6 +135,7 @@
     // This creates a QImage that directly references the shared bitmap data.
     // This is only safe to use when we know that the contents of the shareable bitmap won't change.
     QImage createQImage();
+    void swizzleRGB();
 #endif
 
 private:

Modified: trunk/Source/WebKit2/Shared/qt/ShareableBitmapQt.cpp (105736 => 105737)


--- trunk/Source/WebKit2/Shared/qt/ShareableBitmapQt.cpp	2012-01-24 14:53:31 UTC (rev 105736)
+++ trunk/Source/WebKit2/Shared/qt/ShareableBitmapQt.cpp	2012-01-24 15:17:11 UTC (rev 105737)
@@ -70,4 +70,17 @@
     notImplemented();
 }
 
-} // namespace WebKit
+void ShareableBitmap::swizzleRGB()
+{
+    uint32_t* data = ""
+    int width = size().width();
+    int height = size().height();
+    for (int y = 0; y < height; ++y) {
+        uint32_t* p = data + y * width;
+        for (int x = 0; x < width; ++x)
+            p[x] = ((p[x] << 16) & 0xff0000) | ((p[x] >> 16) & 0xff) | (p[x] & 0xff00ff00);
+    }
+}
+
+}
+// namespace WebKit

Modified: trunk/Source/WebKit2/UIProcess/LayerTreeHostProxy.h (105736 => 105737)


--- trunk/Source/WebKit2/UIProcess/LayerTreeHostProxy.h	2012-01-24 14:53:31 UTC (rev 105736)
+++ trunk/Source/WebKit2/UIProcess/LayerTreeHostProxy.h	2012-01-24 15:17:11 UTC (rev 105737)
@@ -110,8 +110,8 @@
     void syncLayerParameters(const WebLayerInfo&);
     void createTile(WebLayerID, int, float scale);
     void removeTile(WebLayerID, int);
-    void updateTile(WebLayerID, int, const WebCore::IntRect&, const WebCore::IntRect&, const QImage&);
-    void createImage(int64_t, const QImage&);
+    void updateTile(WebLayerID, int, const WebCore::IntRect&, const WebCore::IntRect&, ShareableBitmap*);
+    void createImage(int64_t, ShareableBitmap*);
     void destroyImage(int64_t);
     void assignImageToLayer(WebCore::GraphicsLayer*, int64_t imageID);
     void flushLayerChanges();

Modified: trunk/Source/WebKit2/UIProcess/qt/LayerTreeHostProxyQt.cpp (105736 => 105737)


--- trunk/Source/WebKit2/UIProcess/qt/LayerTreeHostProxyQt.cpp	2012-01-24 14:53:31 UTC (rev 105736)
+++ trunk/Source/WebKit2/UIProcess/qt/LayerTreeHostProxyQt.cpp	2012-01-24 15:17:11 UTC (rev 105737)
@@ -96,7 +96,7 @@
     int remoteTileID;
     IntRect sourceRect;
     IntRect targetRect;
-    QImage image;
+    RefPtr<ShareableBitmap> bitmap;
 };
 
 struct RemoveTileMessageData {
@@ -106,7 +106,7 @@
 
 struct CreateImageMessageData {
     int64_t imageID;
-    QImage image;
+    RefPtr<ShareableBitmap> bitmap;
 };
 
 struct DestroyImageMessageData {
@@ -350,7 +350,7 @@
     m_tileToNodeTile.remove(tileID);
 }
 
-void LayerTreeHostProxy::updateTile(WebLayerID layerID, int tileID, const IntRect& sourceRect, const IntRect& targetRect, const QImage& image)
+void LayerTreeHostProxy::updateTile(WebLayerID layerID, int tileID, const IntRect& sourceRect, const IntRect& targetRect, ShareableBitmap* bitmap)
 {
     ensureLayer(layerID);
     TextureMapperNode* node = toTextureMapperNode(layerByID(layerID));
@@ -361,15 +361,16 @@
     if (!nodeTileID)
         return;
 
-    QImage imageRef(image);
     node->setTextureMapper(m_textureMapper.get());
-    node->setContentsTileBackBuffer(nodeTileID, sourceRect, targetRect, imageRef.bits(), BitmapTexture::BGRAFormat);
+    QImage image = bitmap->createQImage();
+    node->setContentsTileBackBuffer(nodeTileID, sourceRect, targetRect, image.constBits());
 }
 
-void LayerTreeHostProxy::createImage(int64_t imageID, const QImage& image)
+void LayerTreeHostProxy::createImage(int64_t imageID, ShareableBitmap* bitmap)
 {
     TiledImage tiledImage;
     static const int TileDimension = 1024;
+    QImage image = bitmap->createQImage();
     bool imageHasAlpha = image.hasAlphaChannel();
     IntRect imageRect(0, 0, image.width(), image.height());
     for (int y = 0; y < image.height(); y += TileDimension) {
@@ -383,13 +384,12 @@
                 subImage = image.copy(rect);
             RefPtr<BitmapTexture> texture = m_textureMapper->createTexture();
             texture->reset(rect.size(), !imageHasAlpha);
-            texture->updateContents(imageHasAlpha ? BitmapTexture::BGRAFormat : BitmapTexture::BGRFormat, IntRect(IntPoint::zero(), rect.size()), subImage.bits());
+            texture->updateRawContents(IntRect(IntPoint::zero(), rect.size()), subImage.constBits());
             tiledImage.add(rect.location(), texture);
         }
     }
 
-    m_directlyCompositedImages.remove(imageID);
-    m_directlyCompositedImages.add(imageID, tiledImage);
+    m_directlyCompositedImages.set(imageID, tiledImage);
 }
 
 void LayerTreeHostProxy::destroyImage(int64_t imageID)
@@ -490,18 +490,18 @@
 
         case LayerTreeMessageToRenderer::UpdateTile: {
             const UpdateTileMessageData& data = ""
-            updateTile(data.layerID, data.remoteTileID, data.sourceRect, data.targetRect, data.image);
+            updateTile(data.layerID, data.remoteTileID, data.sourceRect, data.targetRect, data.bitmap.get());
             break;
         }
 
         case LayerTreeMessageToRenderer::CreateImage: {
             const CreateImageMessageData& data = ""
-            createImage(data.imageID, data.image);
+            createImage(data.imageID, data.bitmap.get());
             break;
         }
 
         case LayerTreeMessageToRenderer::DestroyImage: {
-            const CreateImageMessageData& data = ""
+            const DestroyImageMessageData& data = ""
             destroyImage(data.imageID);
             break;
         }
@@ -534,8 +534,7 @@
     UpdateTileMessageData data;
     data.layerID = layerID;
     data.remoteTileID = tileID;
-    RefPtr<ShareableBitmap> bitmap = ShareableBitmap::create(updateInfo.bitmapHandle);
-    data.image = bitmap->createQImage().copy();
+    data.bitmap = ShareableBitmap::create(updateInfo.bitmapHandle);
     data.sourceRect = IntRect(IntPoint::zero(), updateInfo.updateRectBounds.size());
     data.targetRect = updateInfo.updateRectBounds;
     pushUpdateToQueue(UpdateTileMessage::create(data));
@@ -580,9 +579,8 @@
 void LayerTreeHostProxy::createDirectlyCompositedImage(int64_t key, const WebKit::ShareableBitmap::Handle& handle)
 {
     CreateImageMessageData data;
-    RefPtr<ShareableBitmap> bitmap = ShareableBitmap::create(handle);
     data.imageID = key;
-    data.image = bitmap->createQImage().copy();
+    data.bitmap = ShareableBitmap::create(handle);
     pushUpdateToQueue(CreateImageMessage::create(data));
 }
 

Modified: trunk/Source/WebKit2/WebProcess/WebPage/TiledBackingStoreRemoteTile.cpp (105736 => 105737)


--- trunk/Source/WebKit2/WebProcess/WebPage/TiledBackingStoreRemoteTile.cpp	2012-01-24 14:53:31 UTC (rev 105736)
+++ trunk/Source/WebKit2/WebProcess/WebPage/TiledBackingStoreRemoteTile.cpp	2012-01-24 15:17:11 UTC (rev 105737)
@@ -88,6 +88,11 @@
     OwnPtr<GraphicsContext> graphicsContext(bitmap->createGraphicsContext());
     graphicsContext->drawImageBuffer(m_localBuffer.get(), ColorSpaceDeviceRGB, IntPoint(0, 0));
 
+#if PLATFORM(QT)
+    // Qt uses BGRA interally, we swizzle to RGBA for OpenGL.
+    bitmap->swizzleRGB();
+#endif
+
     UpdateInfo updateInfo;
     updateInfo.updateRectBounds = m_rect;
     updateInfo.updateScaleFactor = m_tiledBackingStore->contentsScale();

Modified: trunk/Source/WebKit2/WebProcess/WebPage/qt/LayerTreeHostQt.cpp (105736 => 105737)


--- trunk/Source/WebKit2/WebProcess/WebPage/qt/LayerTreeHostQt.cpp	2012-01-24 14:53:31 UTC (rev 105736)
+++ trunk/Source/WebKit2/WebProcess/WebPage/qt/LayerTreeHostQt.cpp	2012-01-24 15:17:11 UTC (rev 105737)
@@ -321,6 +321,8 @@
         graphicsContext->drawImage(image, ColorSpaceDeviceRGB, IntPoint::zero());
     }
 
+    // Qt uses BGRA internally, we swizzle to RGBA for OpenGL.
+    bitmap->swizzleRGB();
     ShareableBitmap::Handle handle;
     bitmap->createHandle(handle);
     m_webPage->send(Messages::LayerTreeHostProxy::CreateDirectlyCompositedImage(key, handle));
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to