[BlackBerry] Fix all flicker caused by empty/incomplete geometries.
authorjpetsovits@rim.com <jpetsovits@rim.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Feb 2013 16:58:17 +0000 (16:58 +0000)
committerjpetsovits@rim.com <jpetsovits@rim.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Feb 2013 16:58:17 +0000 (16:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=108951
RIM PR 286925

Reviewed by Rob Buis.
Internally reviewed by Arvid Nilsson.

The main idea for this patch is that whenever we adopt
a new backingstore geometry that doesn't contain any
rendered tiles, or VisibleZoom render jobs that need more
tiles to be rendered to be considered complete, we'll then
suspend blitting until there is valid content to show.

This main idea is codified as checks for empty buffers
in adoptAsFrontState(), and checks for the current state
of the render queue after rendering content in render().
However, as BackingStore objects with disabled surface pools
or pure use of accelerated compositing also swap geometries
in some circumstances, the use of suspend counters grows
increasingly fragile.

To make this patch more resilient against regressions,
the current suspend counter is complemented with several
explicit conditions for suspending screen updates,
and both subsequently combined into a single cached
boolean value telling the UI thread whether or not to
suspend. In the future, other suspend calls can be
migrated to this "state machine" design as well,
potentially phasing out the suspend counter altogether.

The immediate result is that there will be no flashing
of background color between page loads or after discarding
tiles on scale changes until the content has been rendered.

* Api/BackingStore.cpp:
(BlackBerry::WebKit::BackingStorePrivate::BackingStorePrivate):
(BlackBerry::WebKit::BackingStorePrivate::suspendBackingStoreUpdates):
(BlackBerry::WebKit::BackingStorePrivate::suspendScreenUpdates):
(BlackBerry::WebKit::BackingStorePrivate::resumeBackingStoreUpdates):
(BlackBerry::WebKit::BackingStorePrivate::resumeScreenUpdates):
(BlackBerry::WebKit::BackingStorePrivate::updateSuspendScreenUpdateState):
(WebKit):
(BlackBerry::WebKit::BackingStorePrivate::render):
(BlackBerry::WebKit::BackingStorePrivate::blitVisibleContents):
(BlackBerry::WebKit::BackingStorePrivate::adoptAsFrontState):
(BlackBerry::WebKit::BackingStorePrivate::setCurrentBackingStoreOwner):
(BlackBerry::WebKit::BackingStore::releaseOwnedBackingStoreMemory):
* Api/BackingStore_p.h:
(BackingStorePrivate):
* Api/WebPage.cpp:
(BlackBerry::WebKit::WebPagePrivate::setVisible):
(BlackBerry::WebKit::WebPagePrivate::setCompositorDrawsRootLayer):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@142137 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/blackberry/Api/BackingStore.cpp
Source/WebKit/blackberry/Api/BackingStore_p.h
Source/WebKit/blackberry/Api/WebPage.cpp
Source/WebKit/blackberry/ChangeLog

index 4f74e34..1fb11ff 100644 (file)
@@ -203,11 +203,14 @@ bool BackingStoreGeometry::isTileCorrespondingToBuffer(TileIndex index, TileBuff
 }
 
 BackingStorePrivate::BackingStorePrivate()
-    : m_suspendScreenUpdates(0)
+    : m_suspendScreenUpdateCounterWebKitThread(0)
     , m_suspendBackingStoreUpdates(0)
     , m_resumeOperation(BackingStore::None)
+    , m_suspendScreenUpdatesWebKitThread(true)
+    , m_suspendScreenUpdatesUserInterfaceThread(true)
     , m_suspendRenderJobs(false)
     , m_suspendRegularRenderJobs(false)
+    , m_tileMatrixContainsUsefulContent(false)
     , m_tileMatrixNeedsUpdate(false)
     , m_isScrollingOrZooming(false)
     , m_webPage(0)
@@ -284,6 +287,12 @@ bool BackingStorePrivate::isOpenGLCompositing() const
 
 void BackingStorePrivate::suspendBackingStoreUpdates()
 {
+    ASSERT(BlackBerry::Platform::webKitThreadMessageClient()->isCurrentThread());
+
+    // We still use atomic values here because the UI thread can use it in
+    // setScrollingOrZooming() (through shouldPerformRegularRenderJobs()).
+    // FIXME: Once that one respects thread boundaries properly, switch to
+    // regular increment/decrement operations.
     if (atomic_add_value(&m_suspendBackingStoreUpdates, 0)) {
         BBLOG(Platform::LogLevelInfo,
             "Backingstore already suspended, increasing suspend counter.");
@@ -294,7 +303,9 @@ void BackingStorePrivate::suspendBackingStoreUpdates()
 
 void BackingStorePrivate::suspendScreenUpdates()
 {
-    if (m_suspendScreenUpdates) {
+    ASSERT(BlackBerry::Platform::webKitThreadMessageClient()->isCurrentThread());
+
+    if (m_suspendScreenUpdateCounterWebKitThread) {
         BBLOG(Platform::LogLevelInfo,
             "Screen already suspended, increasing suspend counter.");
     }
@@ -302,13 +313,14 @@ void BackingStorePrivate::suspendScreenUpdates()
     // Make sure the user interface thread gets the message before we proceed
     // because blitVisibleContents() can be called from the user interface
     // thread and it must honor this flag.
-    ++m_suspendScreenUpdates;
-
-    BlackBerry::Platform::userInterfaceThreadMessageClient()->syncToCurrentMessage();
+    ++m_suspendScreenUpdateCounterWebKitThread;
+    updateSuspendScreenUpdateState();
 }
 
 void BackingStorePrivate::resumeBackingStoreUpdates()
 {
+    ASSERT(BlackBerry::Platform::webKitThreadMessageClient()->isCurrentThread());
+
     unsigned currentValue = atomic_add_value(&m_suspendBackingStoreUpdates, 0);
     ASSERT(currentValue >= 1);
     if (currentValue < 1) {
@@ -330,9 +342,10 @@ void BackingStorePrivate::resumeBackingStoreUpdates()
 
 void BackingStorePrivate::resumeScreenUpdates(BackingStore::ResumeUpdateOperation op)
 {
-    ASSERT(m_suspendScreenUpdates);
+    ASSERT(BlackBerry::Platform::webKitThreadMessageClient()->isCurrentThread());
+    ASSERT(m_suspendScreenUpdateCounterWebKitThread);
 
-    if (!m_suspendScreenUpdates) {
+    if (!m_suspendScreenUpdateCounterWebKitThread) {
         Platform::logAlways(Platform::LogLevelCritical,
             "Call mismatch: Screen hasn't been suspended, therefore won't resume!");
         return;
@@ -343,10 +356,10 @@ void BackingStorePrivate::resumeScreenUpdates(BackingStore::ResumeUpdateOperatio
         || (m_resumeOperation == BackingStore::None && op == BackingStore::Blit))
         m_resumeOperation = op;
 
-    if (m_suspendScreenUpdates >= 2) { // we're still suspended
+    if (m_suspendScreenUpdateCounterWebKitThread >= 2) { // we're still suspended
         BBLOG(Platform::LogLevelInfo,
             "Screen and backingstore still suspended, decreasing suspend counter.");
-        --m_suspendScreenUpdates;
+        --m_suspendScreenUpdateCounterWebKitThread;
         return;
     }
 
@@ -358,8 +371,8 @@ void BackingStorePrivate::resumeScreenUpdates(BackingStore::ResumeUpdateOperatio
         if (isOpenGLCompositing() && !isActive()) {
             m_webPage->d->setCompositorDrawsRootLayer(true);
             m_webPage->d->setNeedsOneShotDrawingSynchronization();
-            --m_suspendScreenUpdates;
-            BlackBerry::Platform::userInterfaceThreadMessageClient()->syncToCurrentMessage();
+            --m_suspendScreenUpdateCounterWebKitThread;
+            updateSuspendScreenUpdateState();
             return;
         }
 
@@ -396,8 +409,8 @@ void BackingStorePrivate::resumeScreenUpdates(BackingStore::ResumeUpdateOperatio
     // Make sure the user interface thread gets the message before we proceed
     // because blitVisibleContents() can be called from the user interface
     // thread and it must honor this flag.
-    --m_suspendScreenUpdates;
-    BlackBerry::Platform::userInterfaceThreadMessageClient()->syncToCurrentMessage();
+    --m_suspendScreenUpdateCounterWebKitThread;
+    updateSuspendScreenUpdateState();
 
     if (op == BackingStore::None)
         return;
@@ -415,6 +428,28 @@ void BackingStorePrivate::resumeScreenUpdates(BackingStore::ResumeUpdateOperatio
 #endif
 }
 
+void BackingStorePrivate::updateSuspendScreenUpdateState()
+{
+    ASSERT(BlackBerry::Platform::webKitThreadMessageClient()->isCurrentThread());
+
+    bool isBackingStoreUsable = isActive() && m_tileMatrixContainsUsefulContent
+        && (m_suspendBackingStoreUpdates || !m_renderQueue->hasCurrentVisibleZoomJob()); // Backingstore is not usable while we're waiting for an ("atomic") zoom job to finish.
+
+    bool shouldSuspend = m_suspendScreenUpdateCounterWebKitThread
+        || !m_webPage->isVisible()
+        || (!isBackingStoreUsable && !m_webPage->d->compositorDrawsRootLayer() && !shouldDirectRenderingToWindow());
+
+    if (m_suspendScreenUpdatesWebKitThread == shouldSuspend)
+        return;
+
+    m_suspendScreenUpdatesWebKitThread = shouldSuspend;
+
+    // FIXME: If we change the backingstore to dispatch geometries, this
+    //   assignment should be moved to a dispatched setter function instead.
+    m_suspendScreenUpdatesUserInterfaceThread = shouldSuspend;
+    BlackBerry::Platform::userInterfaceThreadMessageClient()->syncToCurrentMessage();
+}
+
 void BackingStorePrivate::repaint(const Platform::IntRect& windowRect,
                                   bool contentChanged, bool immediate)
 {
@@ -1135,12 +1170,15 @@ TileIndexList BackingStorePrivate::render(const TileIndexList& tileIndexList)
         newTileMap.set(index, backBuffer);
     }
 
-    newGeometry->setTileMap(newTileMap);
-    adoptAsFrontState(newGeometry);
-
     // Let the render queue know that the tile contents are up to date now.
     m_renderQueue->clear(renderedTiles, frontState(), RenderQueue::DontClearCompletedJobs);
 
+    // If we couldn't render all requested jobs, suspend blitting until we do.
+    updateSuspendScreenUpdateState();
+
+    newGeometry->setTileMap(newTileMap);
+    adoptAsFrontState(newGeometry);
+
 #if DEBUG_BACKINGSTORE
     Platform::logAlways(Platform::LogLevelInfo,
         "BackingStorePrivate::render done rendering %d tiles.",
@@ -1227,13 +1265,6 @@ void BackingStorePrivate::blitVisibleContents(bool force)
         return;
     }
 
-    if (!m_webPage->isVisible() || m_suspendScreenUpdates) {
-        // Avoid client going into busy loop while blit is impossible.
-        if (force)
-            m_hasBlitJobs = false;
-        return;
-    }
-
     if (!BlackBerry::Platform::userInterfaceThreadMessageClient()->isCurrentThread()) {
         BlackBerry::Platform::userInterfaceThreadMessageClient()->dispatchMessage(
             BlackBerry::Platform::createMethodCallMessage(
@@ -1241,6 +1272,13 @@ void BackingStorePrivate::blitVisibleContents(bool force)
         return;
     }
 
+    if (m_suspendScreenUpdatesUserInterfaceThread) {
+        // Avoid client going into busy loop while blit is impossible.
+        if (force)
+            m_hasBlitJobs = false;
+        return;
+    }
+
     if (!force) {
 #if USE(ACCELERATED_COMPOSITING)
         // If there's a WebPageCompositorClient, let it schedule the blit.
@@ -2226,13 +2264,22 @@ BackingStoreGeometry* BackingStorePrivate::frontState() const
 
 void BackingStorePrivate::adoptAsFrontState(BackingStoreGeometry* newFrontState)
 {
+    bool hasValidBuffers = false;
+
     // Remember the buffers we'll use in the new front state for comparison.
     WTF::Vector<TileBuffer*> newTileBuffers;
     TileMap newTileMap = newFrontState->tileMap();
     TileMap::const_iterator end = newTileMap.end();
     for (TileMap::const_iterator it = newTileMap.begin(); it != end; ++it) {
-        if (it->value)
+        if (it->value) {
+            hasValidBuffers = true;
             newTileBuffers.append(it->value);
+        }
+    }
+
+    if (!hasValidBuffers) {
+        m_tileMatrixContainsUsefulContent = false;
+        updateSuspendScreenUpdateState();
     }
 
     unsigned newFront = reinterpret_cast<unsigned>(newFrontState);
@@ -2241,8 +2288,13 @@ void BackingStorePrivate::adoptAsFrontState(BackingStoreGeometry* newFrontState)
     // Atomic change.
     _smp_xchg(&m_frontState, newFront);
 
-    // Wait until the user interface thread won't access the old front state anymore.
-    BlackBerry::Platform::userInterfaceThreadMessageClient()->syncToCurrentMessage();
+    if (hasValidBuffers) {
+        m_tileMatrixContainsUsefulContent = true;
+        updateSuspendScreenUpdateState(); // includes syncToCurrentMessage(), so we can use it instead
+    } else {
+        // Wait until the user interface thread won't access the old front state anymore.
+        BlackBerry::Platform::userInterfaceThreadMessageClient()->syncToCurrentMessage();
+    }
 
     // Reclaim unused old tile buffers as back buffers.
     TileMap oldTileMap = oldFrontState->tileMap();
@@ -2296,6 +2348,7 @@ void BackingStorePrivate::setCurrentBackingStoreOwner(WebPage* webPage)
         BackingStorePrivate::s_currentBackingStoreOwner->d->m_backingStore->d->resetTiles();
 
     BackingStorePrivate::s_currentBackingStoreOwner = webPage;
+    webPage->backingStore()->d->updateSuspendScreenUpdateState(); // depends on isActive()
 }
 
 bool BackingStorePrivate::isActive() const
@@ -2412,8 +2465,13 @@ void BackingStore::acquireBackingStoreMemory()
 
 void BackingStore::releaseOwnedBackingStoreMemory()
 {
-    if (BackingStorePrivate::s_currentBackingStoreOwner == d->m_webPage)
+    if (BackingStorePrivate::s_currentBackingStoreOwner == d->m_webPage) {
+        // Call resetTiles() (hopefully) after suspendScreenUpdates()
+        // so we will not cause checkerboard to be shown before suspending.
+        // This causes the tiles in use to be given back to the SurfacePool.
+        d->resetTiles();
         SurfacePool::globalSurfacePool()->releaseBuffers();
+    }
 }
 
 bool BackingStore::hasBlitJobs() const
index d6196e8..2f40f40 100644 (file)
@@ -129,8 +129,6 @@ public:
     // BlackBerry::Platform::Graphics::Window::GLES2Usage.
     bool isOpenGLCompositing() const;
 
-    bool isSuspended() const { return m_suspendBackingStoreUpdates; }
-
     // Suspends all backingstore updates so that rendering to the backingstore is disabled.
     void suspendBackingStoreUpdates();
 
@@ -143,6 +141,9 @@ public:
     // Resumes all screen updates so that 'blitVisibleContents' is enabled.
     void resumeScreenUpdates(BackingStore::ResumeUpdateOperation);
 
+    // Update m_suspendScreenUpdates*Thread based on a number of conditions.
+    void updateSuspendScreenUpdateState();
+
     // The functions repaint(), slowScroll(), scroll(), scrollingStartedHelper() are
     // called from outside WebKit and within WebKit via ChromeClientBlackBerry.
     void repaint(const Platform::IntRect& windowRect, bool contentChanged, bool immediate);
@@ -342,12 +343,15 @@ public:
 
     static WebPage* s_currentBackingStoreOwner;
 
-    unsigned m_suspendScreenUpdates;
+    unsigned m_suspendScreenUpdateCounterWebKitThread;
     unsigned m_suspendBackingStoreUpdates;
     BackingStore::ResumeUpdateOperation m_resumeOperation;
 
+    bool m_suspendScreenUpdatesWebKitThread;
+    bool m_suspendScreenUpdatesUserInterfaceThread;
     bool m_suspendRenderJobs;
     bool m_suspendRegularRenderJobs;
+    bool m_tileMatrixContainsUsefulContent;
     bool m_tileMatrixNeedsUpdate;
     bool m_isScrollingOrZooming;
     WebPage* m_webPage;
index b3f4c2f..51f4574 100644 (file)
@@ -3221,6 +3221,7 @@ void WebPagePrivate::setVisible(bool visible)
         }
 
         m_visible = visible;
+        m_backingStore->d->updateSuspendScreenUpdateState();
     }
 
 #if ENABLE(PAGE_VISIBILITY_API)
@@ -5343,6 +5344,7 @@ void WebPagePrivate::setCompositorDrawsRootLayer(bool compositorDrawsRootLayer)
     // When the BlackBerry port forces compositing mode, the root layer stops
     // painting to window and starts painting to layer instead.
     m_page->settings()->setForceCompositingMode(compositorDrawsRootLayer);
+    m_backingStore->d->updateSuspendScreenUpdateState();
 
     if (!m_mainFrame)
         return;
index 3e6512c..1248b9a 100644 (file)
@@ -1,3 +1,58 @@
+2013-02-07  Jakob Petsovits  <jpetsovits@rim.com>
+
+        [BlackBerry] Fix all flicker caused by empty/incomplete geometries.
+        https://bugs.webkit.org/show_bug.cgi?id=108951
+        RIM PR 286925
+
+        Reviewed by Rob Buis.
+        Internally reviewed by Arvid Nilsson.
+
+        The main idea for this patch is that whenever we adopt
+        a new backingstore geometry that doesn't contain any
+        rendered tiles, or VisibleZoom render jobs that need more
+        tiles to be rendered to be considered complete, we'll then
+        suspend blitting until there is valid content to show.
+
+        This main idea is codified as checks for empty buffers
+        in adoptAsFrontState(), and checks for the current state
+        of the render queue after rendering content in render().
+        However, as BackingStore objects with disabled surface pools
+        or pure use of accelerated compositing also swap geometries
+        in some circumstances, the use of suspend counters grows
+        increasingly fragile.
+
+        To make this patch more resilient against regressions,
+        the current suspend counter is complemented with several
+        explicit conditions for suspending screen updates,
+        and both subsequently combined into a single cached
+        boolean value telling the UI thread whether or not to
+        suspend. In the future, other suspend calls can be
+        migrated to this "state machine" design as well,
+        potentially phasing out the suspend counter altogether.
+
+        The immediate result is that there will be no flashing
+        of background color between page loads or after discarding
+        tiles on scale changes until the content has been rendered.
+
+        * Api/BackingStore.cpp:
+        (BlackBerry::WebKit::BackingStorePrivate::BackingStorePrivate):
+        (BlackBerry::WebKit::BackingStorePrivate::suspendBackingStoreUpdates):
+        (BlackBerry::WebKit::BackingStorePrivate::suspendScreenUpdates):
+        (BlackBerry::WebKit::BackingStorePrivate::resumeBackingStoreUpdates):
+        (BlackBerry::WebKit::BackingStorePrivate::resumeScreenUpdates):
+        (BlackBerry::WebKit::BackingStorePrivate::updateSuspendScreenUpdateState):
+        (WebKit):
+        (BlackBerry::WebKit::BackingStorePrivate::render):
+        (BlackBerry::WebKit::BackingStorePrivate::blitVisibleContents):
+        (BlackBerry::WebKit::BackingStorePrivate::adoptAsFrontState):
+        (BlackBerry::WebKit::BackingStorePrivate::setCurrentBackingStoreOwner):
+        (BlackBerry::WebKit::BackingStore::releaseOwnedBackingStoreMemory):
+        * Api/BackingStore_p.h:
+        (BackingStorePrivate):
+        * Api/WebPage.cpp:
+        (BlackBerry::WebKit::WebPagePrivate::setVisible):
+        (BlackBerry::WebKit::WebPagePrivate::setCompositorDrawsRootLayer):
+
 2013-02-07  Xiaobo Wang  <xbwang@torchmobile.com.cn>
 
         [BlackBerry] CHHW - Characters that are using 32 bits encoding get trunked to 16bits