[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-3' - test/TileCacheTests.cpp wsd/DocumentBroker.cpp wsd/TileCache.cpp wsd/TileCache.hpp
Libreoffice Gerrit user
logerrit at kemper.freedesktop.org
Mon Oct 1 10:28:09 UTC 2018
test/TileCacheTests.cpp | 58 ++++++++++++++++++++++++++++++++++++++++++++++++
wsd/DocumentBroker.cpp | 13 +++++++---
wsd/TileCache.cpp | 37 ++++++++++++++++++++++++++++++
wsd/TileCache.hpp | 8 ++++++
4 files changed, 112 insertions(+), 4 deletions(-)
New commits:
commit c2d800bd345a14c006697e4fd8a75fcadf36557c
Author: Tamás Zolnai <tamas.zolnai at collabora.com>
AuthorDate: Wed Sep 26 22:15:26 2018 +0200
Commit: Jan Holesovsky <kendy at collabora.com>
CommitDate: Mon Oct 1 12:27:49 2018 +0200
Avoid rendering / sending the same tile twice
When we're doing the prerendering we also need to register
a tileBeingRendered object, so we know that the given tile
will arrive soon. In earlier version of the code, rendering
and sending of the tile was not separated in time, but now it is,
so we need create a tileBeingRendered object even if we don't
subscribe the client to it yet.
(cherry picked from commit 5260eae05f89056d4ffc1bdfad1927c3c5f9706d)
Change-Id: I1374c22e5ffebe1d6fcc6e37261ddcedeb9066ec
Reviewed-on: https://gerrit.libreoffice.org/61124
Reviewed-by: Jan Holesovsky <kendy at collabora.com>
Tested-by: Jan Holesovsky <kendy at collabora.com>
diff --git a/test/TileCacheTests.cpp b/test/TileCacheTests.cpp
index ac670792c..2c8f241d8 100644
--- a/test/TileCacheTests.cpp
+++ b/test/TileCacheTests.cpp
@@ -79,6 +79,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
// temporarily disable
//CPPUNIT_TEST(testTileInvalidatePartCalc);
//CPPUNIT_TEST(testTileInvalidatePartImpress);
+ CPPUNIT_TEST(testTileBeingRenderedHandling);
CPPUNIT_TEST_SUITE_END();
@@ -102,6 +103,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
void testTileInvalidateCalc();
void testTileInvalidatePartCalc();
void testTileInvalidatePartImpress();
+ void testTileBeingRenderedHandling();
void checkTiles(std::shared_ptr<LOOLWebSocket>& socket,
const std::string& type,
@@ -1146,6 +1148,62 @@ void TileCacheTests::requestTiles(std::shared_ptr<LOOLWebSocket>& socket, const
}
}
+void TileCacheTests::testTileBeingRenderedHandling()
+{
+ // The issue here was that we requested the tile of the same tile twice
+ // and so sometimes we got the same tile message twice from wsd.
+ const char* testname = "testTileBeingRenderedHandling ";
+
+ std::string documentPath, documentURL;
+ getDocumentPathAndURL("empty.odt", documentPath, documentURL, testname);
+ std::shared_ptr<LOOLWebSocket> socket = loadDocAndGetSocket(_uri, documentURL, testname);
+
+ // Set the client visible area
+ sendTextFrame(socket, "clientvisiblearea x=-2662 y=0 width=16000 height=9875");
+ sendTextFrame(socket, "clientzoom tilepixelwidth=256 tilepixelheight=256 tiletwipwidth=3200 tiletwipheight=3200");
+
+ // Type one character to trigger invalidation
+ sendChar(socket, 'x', skNone, testname);
+
+ // First wsd forwards the invalidation
+ assertResponseString(socket, "invalidatetiles:", testname);
+
+ // For the first input wsd will send all invalidated tiles
+ int arrivedTiles = 0;
+ bool gotTile = false;
+ do
+ {
+ std::vector<char> tile = getResponseMessage(socket, "tile:", testname);
+ gotTile = !tile.empty();
+ if(gotTile)
+ ++arrivedTiles;
+ } while(gotTile);
+
+ CPPUNIT_ASSERT_MESSAGE("We expect two tiles at least!", arrivedTiles > 1);
+
+ // For the later inputs wsd will send one tile, since other ones are indentical
+ for(int i = 0; i < 10; ++i)
+ {
+ // Type an other character
+ sendChar(socket, 'x', skNone, testname);
+ assertResponseString(socket, "invalidatetiles:", testname);
+
+ arrivedTiles = 0;
+ gotTile = false;
+ do
+ {
+ std::vector<char> tile = getResponseMessage(socket, "tile:", testname, 1000);
+ gotTile = !tile.empty();
+ if(gotTile)
+ ++arrivedTiles;
+ } while(gotTile);
+
+ CPPUNIT_ASSERT_EQUAL(1, arrivedTiles);
+
+ sendTextFrame(socket, "tileprocessed tile=0:0:0:3200:3200");
+ }
+}
+
CPPUNIT_TEST_SUITE_REGISTRATION(TileCacheTests);
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index e01af9069..1d1adf746 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1321,15 +1321,16 @@ void DocumentBroker::handleTileCombinedRequest(TileCombined& tileCombined,
std::vector<TileDesc> tilesNeedsRendering;
for (auto& tile : tileCombined.getTiles())
{
+ tile.setVersion(++_tileVersion);
std::unique_ptr<std::fstream> cachedTile = _tileCache->lookupTile(tile);
if(cachedTile)
cachedTile->close();
else
{
// Not cached, needs rendering.
- tile.setVersion(++_tileVersion);
tilesNeedsRendering.push_back(tile);
_debugRenderedTileCount++;
+ tileCache().registerTileBeingRendered(tile);
}
}
@@ -1459,9 +1460,13 @@ void DocumentBroker::sendRequestedTiles(const std::shared_ptr<ClientSession>& se
else
{
// Not cached, needs rendering.
- tile.setVersion(++_tileVersion);
- tilesNeedsRendering.push_back(tile);
- _debugRenderedTileCount++;
+ if (!tileCache().hasTileBeingRendered(tile) || // There is no in progress rendering of the given tile
+ tileCache().getTileBeingRenderedVersion(tile) < tile.getVersion()) // We need a newer version
+ {
+ tile.setVersion(++_tileVersion);
+ tilesNeedsRendering.push_back(tile);
+ _debugRenderedTileCount++;
+ }
tileCache().subscribeToTileRendering(tile, session);
}
requestedTiles.pop_front();
diff --git a/wsd/TileCache.cpp b/wsd/TileCache.cpp
index 8e0da8cfe..a00987f3b 100644
--- a/wsd/TileCache.cpp
+++ b/wsd/TileCache.cpp
@@ -151,6 +151,20 @@ double TileCache::getTileBeingRenderedElapsedTimeMs(const std::string& tileCache
return iterator->second->getElapsedTimeMs();
}
+bool TileCache::hasTileBeingRendered(const TileDesc& tile)
+{
+ return (findTileBeingRendered(tile) != nullptr);
+}
+
+int TileCache::getTileBeingRenderedVersion(const TileDesc& tile)
+{
+ std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(tile);
+ if (tileBeingRendered)
+ return tileBeingRendered->getVersion();
+ else
+ return 0;
+}
+
std::unique_ptr<std::fstream> TileCache::lookupTile(const TileDesc& tile)
{
if (!_tileCachePersistent)
@@ -535,6 +549,29 @@ void TileCache::subscribeToTileRendering(const TileDesc& tile, const std::shared
}
}
+void TileCache::registerTileBeingRendered(const TileDesc& tile)
+{
+ std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(tile);
+ if (tileBeingRendered)
+ {
+ const auto duration = (std::chrono::steady_clock::now() - tileBeingRendered->getStartTime());
+ if (std::chrono::duration_cast<std::chrono::milliseconds>(duration).count() > COMMAND_TIMEOUT_MS)
+ {
+ // Tile painting has stalled. Reissue.
+ tileBeingRendered->setVersion(tile.getVersion());
+ }
+ }
+ else
+ {
+ const std::string cachedName = cacheFileName(tile);
+
+ assert(_tilesBeingRendered.find(cachedName) == _tilesBeingRendered.end());
+
+ tileBeingRendered = std::make_shared<TileBeingRendered>(cachedName, tile);
+ _tilesBeingRendered[cachedName] = tileBeingRendered;
+ }
+}
+
std::string TileCache::cancelTiles(const std::shared_ptr<ClientSession> &subscriber)
{
assert(subscriber && "cancelTiles expects valid subscriber");
diff --git a/wsd/TileCache.hpp b/wsd/TileCache.hpp
index 146e505ae..3d756fee0 100644
--- a/wsd/TileCache.hpp
+++ b/wsd/TileCache.hpp
@@ -46,6 +46,11 @@ public:
/// Otherwise returns 0 to signify a subscription exists.
void subscribeToTileRendering(const TileDesc& tile, const std::shared_ptr<ClientSession>& subscriber);
+ /// Create the TileBeingRendered object for the given tile indicating that the tile was sent to
+ /// the kit for rendering. Note: subscribeToTileRendering calls this internally, so you don't need
+ /// to call this method if you need also to subcribe for the rendered tile.
+ void registerTileBeingRendered(const TileDesc& tile);
+
/// Cancels all tile requests by the given subscriber.
std::string cancelTiles(const std::shared_ptr<ClientSession>& subscriber);
@@ -82,6 +87,9 @@ public:
void forgetTileBeingRendered(std::shared_ptr<TileCache::TileBeingRendered> tileBeingRendered, const TileDesc& tile);
double getTileBeingRenderedElapsedTimeMs(const std::string& tileCacheName) const;
+ bool hasTileBeingRendered(const TileDesc& tile);
+ int getTileBeingRenderedVersion(const TileDesc& tile);
+
void setThreadOwner(const std::thread::id &id) { _owner = id; }
void assertCorrectThread();
More information about the Libreoffice-commits
mailing list