From 808dc42f1e6a574778bc9e8bd41beb7bd9aef06f Mon Sep 17 00:00:00 2001 From: Chris Craik Date: Mon, 12 Sep 2011 15:49:36 -0700 Subject: Fix layer tile allocation to be more consistent, less disco bug:5290559 Two major things: * Avoid swapping front/back textures before the textures have been blitted to avoid race condition where blit fails because it doesn't see a back texture * Don't allow a tile to acquire its front texture to paint into, as the acquisition logic doesn't handle this. Change-Id: I84b59396ca9faaf3ddc7c75b6f66e4736bf4c3cf --- .../WebCore/platform/graphics/android/BaseTile.cpp | 19 +++++++++++-------- .../WebCore/platform/graphics/android/BaseTile.h | 9 ++++++++- .../platform/graphics/android/BaseTileTexture.cpp | 1 - .../platform/graphics/android/TilesManager.cpp | 22 +++++++++++++++------- 4 files changed, 34 insertions(+), 17 deletions(-) (limited to 'Source/WebCore') diff --git a/Source/WebCore/platform/graphics/android/BaseTile.cpp b/Source/WebCore/platform/graphics/android/BaseTile.cpp index 8a4c2d5..dc17a21 100644 --- a/Source/WebCore/platform/graphics/android/BaseTile.cpp +++ b/Source/WebCore/platform/graphics/android/BaseTile.cpp @@ -70,7 +70,7 @@ BaseTile::BaseTile(bool isLayerTile) , m_lastDirtyPicture(0) , m_isTexturePainted(false) , m_isLayerTile(isLayerTile) - , m_isSwapNeeded(false) + , m_swapDrawCount(0) , m_drawCount(0) { #ifdef DEBUG_COUNT @@ -133,7 +133,7 @@ void BaseTile::reserveTexture() android::AutoMutex lock(m_atomicSync); if (texture && m_backTexture != texture) { - m_isSwapNeeded = false; // no longer ready to swap + m_swapDrawCount = 0; // no longer ready to swap m_backTexture = texture; // this is to catch when the front texture is stolen from beneath us. We @@ -240,7 +240,7 @@ bool BaseTile::isTileReady() { // Return true if the tile's most recently drawn texture is up to date android::AutoMutex lock(m_atomicSync); - BaseTileTexture * texture = m_isSwapNeeded ? m_backTexture : m_frontTexture; + BaseTileTexture * texture = m_swapDrawCount ? m_backTexture : m_frontTexture; if (!texture) return false; @@ -435,10 +435,13 @@ void BaseTile::paintBitmap() if (!m_dirtyArea[m_currentDirtyAreaIndex].isEmpty()) m_dirty = true; - XLOG("painted tile %p (%d, %d), dirty=%d", this, x, y, m_dirty); + XLOG("painted tile %p (%d, %d), texture %p, dirty=%d", this, x, y, texture, m_dirty); - if (!m_dirty) - m_isSwapNeeded = true; + if (!m_dirty) { + // swap textures, but WAIT until the next draw call (since we need + // to let GLWebViewState blit them at the beginning of drawGL) + m_swapDrawCount = TilesManager::instance()->getDrawGLCount() + 1; + } } m_atomicSync.unlock(); @@ -459,7 +462,7 @@ void BaseTile::discardTextures() { bool BaseTile::swapTexturesIfNeeded() { android::AutoMutex lock(m_atomicSync); - if (m_isSwapNeeded) { + if (m_swapDrawCount && TilesManager::instance()->getDrawGLCount() >= m_swapDrawCount) { // discard old texture and swap the new one in its place if (m_frontTexture) m_frontTexture->release(this); @@ -467,7 +470,7 @@ bool BaseTile::swapTexturesIfNeeded() { XLOG("%p's frontTexture was %p, now becoming %p", this, m_frontTexture, m_backTexture); m_frontTexture = m_backTexture; m_backTexture = 0; - m_isSwapNeeded = false; + m_swapDrawCount = 0; XLOG("display texture for %d, %d front is now %p, texture is %p", m_x, m_y, m_frontTexture, m_backTexture); return true; diff --git a/Source/WebCore/platform/graphics/android/BaseTile.h b/Source/WebCore/platform/graphics/android/BaseTile.h index 4c9650f..734a3c8 100644 --- a/Source/WebCore/platform/graphics/android/BaseTile.h +++ b/Source/WebCore/platform/graphics/android/BaseTile.h @@ -148,7 +148,14 @@ private: BaseRenderer* m_renderer; bool m_isLayerTile; - bool m_isSwapNeeded; + + // this is set when the back texture is finished painting and should be + // swapped to the front. it is set with the NEXT drawGL call (see + // TilesManager::m_drawGLCount) so that the textures may be blitted at the + // beginning of GLWebViewState::drawGL before they are swapped + + // 4 steps for texture: paint -> blit -> swap -> draw + unsigned long long m_swapDrawCount; // the most recent GL draw before this tile was prepared. used for // prioritization and caching. tiles with old drawcounts and textures they diff --git a/Source/WebCore/platform/graphics/android/BaseTileTexture.cpp b/Source/WebCore/platform/graphics/android/BaseTileTexture.cpp index 8cc67b9..f049b6f 100644 --- a/Source/WebCore/platform/graphics/android/BaseTileTexture.cpp +++ b/Source/WebCore/platform/graphics/android/BaseTileTexture.cpp @@ -229,7 +229,6 @@ bool BaseTileTexture::release(TextureOwner* owner) // force readyFor to return false next call (even if texture reaquired by same tile) m_ownTextureTileInfo.m_x = -1; m_ownTextureTileInfo.m_y = -1; - m_ownTextureTileInfo.m_scale = 0; if (!m_busy) { m_owner = 0; } else { diff --git a/Source/WebCore/platform/graphics/android/TilesManager.cpp b/Source/WebCore/platform/graphics/android/TilesManager.cpp index bb8feb9..ee35ce2 100644 --- a/Source/WebCore/platform/graphics/android/TilesManager.cpp +++ b/Source/WebCore/platform/graphics/android/TilesManager.cpp @@ -100,7 +100,7 @@ TilesManager::TilesManager() , m_showVisualIndicator(false) , m_invertedScreen(false) , m_invertedScreenSwitch(false) - , m_drawGLCount(0) + , m_drawGLCount(1) { XLOG("TilesManager ctor"); m_textures.reserveCapacity(MAX_TEXTURE_ALLOCATION); @@ -268,9 +268,10 @@ BaseTileTexture* TilesManager::getAvailableTexture(BaseTile* owner) // The heuristic for selecting a texture is as follows: // 1. If a tile isn't owned, break with that one - // 2. If we find a tile in the same page with a different scale, + // 2. Don't let tiles acquire their front textures + // 3. If we find a tile in the same page with a different scale, // it's old and not visible. Break with that one - // 3. Otherwise, use the least recently prepared tile, but ignoring tiles + // 4. Otherwise, use the least recently prepared tile, but ignoring tiles // drawn in the last frame to avoid flickering BaseTileTexture* farthestTexture = 0; @@ -284,6 +285,11 @@ BaseTileTexture* TilesManager::getAvailableTexture(BaseTile* owner) break; } + // Don't let a tile acquire its own front texture, as the acquisition + // logic doesn't handle that + if (currentOwner == owner) + continue; + if (currentOwner->page() == owner->page() && texture->scale() != owner->scale()) { // if we render the back page with one scale, then another while // still zooming, we recycle the tiles with the old scale instead of @@ -300,12 +306,13 @@ BaseTileTexture* TilesManager::getAvailableTexture(BaseTile* owner) } if (farthestTexture) { - TextureOwner* previousOwner = farthestTexture->owner(); + BaseTile* previousOwner = static_cast(farthestTexture->owner()); if (farthestTexture->acquire(owner)) { if (previousOwner) { - XLOG("%s texture %p stolen from tile %d, %d, drawCount was %llu", + XLOG("%s texture %p stolen from tile %d, %d, drawCount was %llu (current is %llu)", owner->isLayerTile() ? "LAYER" : "BASE", - farthestTexture, owner->x(), owner->y(), oldestDrawCount); + farthestTexture, previousOwner->x(), previousOwner->y(), + oldestDrawCount, getDrawGLCount()); } availableTexturePool->remove(availableTexturePool->find(farthestTexture)); @@ -313,7 +320,8 @@ BaseTileTexture* TilesManager::getAvailableTexture(BaseTile* owner) } } - XLOG("Couldn't find an available texture for tile %x (%d, %d) out of %d available!!!", + XLOG("Couldn't find an available texture for %s tile %x (%d, %d) out of %d available", + owner->isLayerTile() ? "LAYER" : "BASE", owner, owner->x(), owner->y(), max); #ifdef DEBUG printTextures(); -- cgit v1.1