[Libreoffice-commits] online.git: 7 commits - common/Common.hpp kit/Kit.cpp loleaflet/src test/TileCacheTests.cpp wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/TestStubs.cpp wsd/TileCache.cpp wsd/TileCache.hpp
Libreoffice Gerrit user
logerrit at kemper.freedesktop.org
Sat Sep 29 07:42:00 UTC 2018
common/Common.hpp | 2
kit/Kit.cpp | 4
loleaflet/src/layer/tile/GridLayer.js | 3
test/TileCacheTests.cpp | 218 ++++++++++++++++++++++++++++++++--
wsd/ClientSession.cpp | 65 +++++++++-
wsd/ClientSession.hpp | 23 ---
wsd/DocumentBroker.cpp | 54 ++++++--
wsd/TestStubs.cpp | 2
wsd/TileCache.cpp | 48 +++++--
wsd/TileCache.hpp | 8 +
10 files changed, 364 insertions(+), 63 deletions(-)
New commits:
commit ac2cd92d25815cd476008c6cd8035e989d0c8826
Author: Tamás Zolnai <tamas.zolnai at collabora.com>
AuthorDate: Fri Sep 28 19:30:57 2018 +0200
Commit: Tamás Zolnai <tamas.zolnai at collabora.com>
CommitDate: Sat Sep 29 09:27:06 2018 +0200
Upper limit of sent out versions of the same tile
We try to decrease the network usage with avoiding sending out
to much tiles to the client. When we already sent out two versions
of the same tile without having the tileprocessed message from the
client we delay sending out the next version to avoid spamming tiles
on the network.
Change-Id: Ia47cd7c0d3fb829f6777f0c3265970433591df19
diff --git a/common/Common.hpp b/common/Common.hpp
index 42e8dd450..c7e246b88 100644
--- a/common/Common.hpp
+++ b/common/Common.hpp
@@ -20,6 +20,8 @@ constexpr int CHILD_REBALANCE_INTERVAL_MS = CHILD_TIMEOUT_MS / 10;
constexpr int POLL_TIMEOUT_MS = COMMAND_TIMEOUT_MS / 10;
constexpr int WS_SEND_TIMEOUT_MS = 1000;
+constexpr int TILE_ROUNDTRIP_TIMEOUT_MS = 5000;
+
/// Pipe and Socket read buffer size.
/// Should be large enough for ethernet packets
/// which can be 1500 bytes long.
diff --git a/test/TileCacheTests.cpp b/test/TileCacheTests.cpp
index da9a5e1a0..f95a5e0ae 100644
--- a/test/TileCacheTests.cpp
+++ b/test/TileCacheTests.cpp
@@ -86,6 +86,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST(testTileInvalidatedOutside);
CPPUNIT_TEST(testTileBeingRenderedHandling);
CPPUNIT_TEST(testWireIDFilteringOnWSDSide);
+ CPPUNIT_TEST(testLimitTileVersionsOnFly);
CPPUNIT_TEST_SUITE_END();
@@ -116,6 +117,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
void testTileInvalidatedOutside();
void testTileBeingRenderedHandling();
void testWireIDFilteringOnWSDSide();
+ void testLimitTileVersionsOnFly();
void checkTiles(std::shared_ptr<LOOLWebSocket>& socket,
const std::string& type,
@@ -1480,6 +1482,66 @@ void TileCacheTests::testWireIDFilteringOnWSDSide()
CPPUNIT_ASSERT_MESSAGE("Not expected tile message arrived!", tile.empty());
}
+void TileCacheTests::testLimitTileVersionsOnFly()
+{
+ // We have an upper limit (2) for the versions of the same tile wsd send out
+ // without getting the tileprocessed message for the first tile message.
+ const char* testname = "testLimitTileVersionsOnFly ";
+
+ 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 sending tiles
+ sendChar(socket, 'x', skNone, testname);
+
+ // Handle all tiles send by wsd
+ bool getTileResp = false;
+ do
+ {
+ std::string tile = getResponseString(socket, "tile:", testname, 1000);
+ getTileResp = !tile.empty();
+ } while(getTileResp);
+
+ // Type an other character to trigger sending tiles
+ sendChar(socket, 'x', skNone, testname);
+
+ // Handle all tiles sent by wsd
+ getTileResp = false;
+ do
+ {
+ std::string tile = getResponseString(socket, "tile:", testname, 1000);
+ getTileResp = !tile.empty();
+ } while(getTileResp);
+
+ // For the third invalidation wsd does not send the new tile since
+ // two versions of the same tile were already sent.
+ sendChar(socket, 'x', skNone, testname);
+
+ std::vector<char> tile1 = getResponseMessage(socket, "tile:", testname, 1000);
+ CPPUNIT_ASSERT_MESSAGE("Not expected tile message arrived!", tile1.empty());
+
+ // When the next tileprocessed message arrive with correct tileID
+ // wsd sends the delayed tile
+ sendTextFrame(socket, "tileprocessed tile=0:0:0:3200:3200");
+
+ int arrivedTiles = 0;
+ bool 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);
+}
+
CPPUNIT_TEST_SUITE_REGISTRATION(TileCacheTests);
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index a64141893..61b20714b 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -1107,7 +1107,7 @@ void ClientSession::removeOutdatedTilesOnFly()
{
auto tileIter = _tilesOnFly.begin();
double elapsedTimeMs = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - tileIter->second).count();
- if(elapsedTimeMs > 5000.0)
+ if(elapsedTimeMs > TILE_ROUNDTRIP_TIMEOUT_MS)
{
LOG_WRN("Tracker tileID was dropped because of time out. Tileprocessed message did not arrive");
_tilesOnFly.erase(tileIter);
@@ -1117,6 +1117,18 @@ void ClientSession::removeOutdatedTilesOnFly()
}
}
+size_t ClientSession::countIdenticalTilesOnFly(const TileDesc& tile) const
+{
+ size_t count = 0;
+ std::string tileID = generateTileID(tile);
+ for(auto& tileItem : _tilesOnFly)
+ {
+ if(tileItem.first == tileID)
+ ++count;
+ }
+ return count;
+}
+
Util::Rectangle ClientSession::getNormalizedVisibleArea() const
{
Util::Rectangle normalizedVisArea;
@@ -1249,7 +1261,7 @@ void ClientSession::handleTileInvalidation(const std::string& message,
Util::Rectangle tileRect (j * _tileWidthTwips, i * _tileHeightTwips, _tileWidthTwips, _tileHeightTwips);
if(invalidateRect.intersects(tileRect))
{
- invalidTiles.emplace_back(TileDesc(part, _tileWidthPixel, _tileHeightPixel, j * _tileWidthTwips, i * _tileHeightTwips, _tileWidthTwips, _tileHeightTwips, -1, 0, -1, false));
+ invalidTiles.emplace_back(part, _tileWidthPixel, _tileHeightPixel, j * _tileWidthTwips, i * _tileHeightTwips, _tileWidthTwips, _tileHeightTwips, -1, 0, -1, false);
TileWireId oldWireId = 0;
auto iter = _oldWireIds.find(generateTileID(invalidTiles.back()));
@@ -1336,7 +1348,7 @@ void ClientSession::clearTileSubscription()
_tilesBeingRendered.clear();
}
-std::string ClientSession::generateTileID(const TileDesc& tile)
+std::string ClientSession::generateTileID(const TileDesc& tile) const
{
std::ostringstream tileID;
tileID << tile.getPart() << ":" << tile.getTilePosX() << ":" << tile.getTilePosY() << ":"
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 45a254609..33c7ca7c2 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -110,6 +110,7 @@ public:
void clearTilesOnFly();
size_t getTilesOnFlyCount() const { return _tilesOnFly.size(); }
void removeOutdatedTilesOnFly();
+ size_t countIdenticalTilesOnFly(const TileDesc& tile) const;
Util::Rectangle getVisibleArea() const { return _clientVisibleArea; }
/// Visible area can have negative value as position, but we have tiles only in the positive range
@@ -176,7 +177,7 @@ private:
const std::shared_ptr<DocumentBroker>& docBroker);
/// Generate a unique id for a tile
- std::string generateTileID(const TileDesc& tile);
+ std::string generateTileID(const TileDesc& tile) const;
private:
std::weak_ptr<DocumentBroker> _docBroker;
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index dc6fd4743..d3fcf7986 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1426,12 +1426,26 @@ void DocumentBroker::sendRequestedTiles(const std::shared_ptr<ClientSession>& se
std::deque<TileDesc>& requestedTiles = session->getRequestedTiles();
if (!requestedTiles.empty())
{
+ size_t delayedTiles = 0;
std::vector<TileDesc> tilesNeedsRendering;
while(session->getTilesOnFlyCount() + session->getTilesBeingRenderedCount() < tilesOnFlyUpperLimit &&
- !requestedTiles.empty())
+ !requestedTiles.empty() &&
+ // If we delayed all tiles we don't send any tile (we will when next tileprocessed message arrives)
+ delayedTiles < requestedTiles.size())
{
TileDesc& tile = *(requestedTiles.begin());
+ // We already sent out two versions of the same tile, let's not send the third one
+ // until we get a tileprocessed message for this specific tile.
+ if (session->countIdenticalTilesOnFly(tile) >= 2)
+ {
+ requestedTiles.push_back(requestedTiles.front());
+ requestedTiles.pop_front();
+ delayedTiles += 1;
+ LOG_INF("Requested tile was delayed!");
+ continue;
+ }
+
// Satisfy as many tiles from the cache.
std::unique_ptr<std::fstream> cachedTile = _tileCache->lookupTile(tile);
if (cachedTile)
commit a1e2acd5effb1c805f06c78040b71d30fb821be4
Author: Tamás Zolnai <tamas.zolnai at collabora.com>
AuthorDate: Wed Sep 26 22:15:41 2018 +0200
Commit: Tamás Zolnai <tamas.zolnai at collabora.com>
CommitDate: Sat Sep 29 09:27:06 2018 +0200
Better to have a smaller tiles-on-fly limit
It's good if this limit big enough to send all the tiles of the
current visible area at once, so the tiles arrive at the same
time to the client by zoom or scroll. Now this value is set to a bit
bigger so if we have a small amount of tiles before zoom / scroll we'll
still see the same on the client side.
Change-Id: I8e28dbf6197fe2f683fe9528e9a32c15a191b20e
diff --git a/test/TileCacheTests.cpp b/test/TileCacheTests.cpp
index 6da19f3be..da9a5e1a0 100644
--- a/test/TileCacheTests.cpp
+++ b/test/TileCacheTests.cpp
@@ -1189,10 +1189,10 @@ void TileCacheTests::testTileRequestByZoom()
sendTextFrame(socket, "clientzoom tilepixelwidth=256 tilepixelheight=256 tiletwipwidth=3200 tiletwipheight=3200");
// Request all tile of the visible area (it happens by zoom)
- sendTextFrame(socket, "tilecombine part=0 width=256 height=256 tileposx=0,3200,6400,9600,12800,0,3200,6400,9600,12800,0,3200,6400,9600,12800,0,3200,6400,9600,12800,0,3200,6400,9600,12800 tileposy=0,0,0,0,0,3200,3200,3200,3200,3200,6400,6400,6400,6400,6400,9600,9600,9600,9600,9600,12800,12800,12800,12800,12800 tilewidth=3200 tileheight=3200");
+ sendTextFrame(socket, "tilecombine part=0 width=256 height=256 tileposx=0,3200,6400,9600,12800,0,3200,6400,9600,12800,0,3200,6400,9600,12800,0,3200,6400,9600,12800 tileposy=0,0,0,0,0,3200,3200,3200,3200,3200,6400,6400,6400,6400,6400,9600,9600,9600,9600,9600 tilewidth=3200 tileheight=3200");
// Check that we get all the tiles without we send back the tileprocessed message
- for (int i = 0; i < 25; ++i)
+ for (int i = 0; i < 20; ++i)
{
std::vector<char> tile = getResponseMessage(socket, "tile:", testname);
CPPUNIT_ASSERT_MESSAGE("Did not get tile as expected!", !tile.empty());
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 5f2ac353e..dc6fd4743 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1408,7 +1408,7 @@ void DocumentBroker::sendRequestedTiles(const std::shared_ptr<ClientSession>& se
std::ceil(normalizedVisArea._y1 / session->getTileHeightInTwips()) + 1;
const int tilesInVisArea = tilesFitOnWidth * tilesFitOnHeight;
- tilesOnFlyUpperLimit = std::max(TILES_ON_FLY_MIN_UPPER_LIMIT, tilesInVisArea * 1.5f);
+ tilesOnFlyUpperLimit = std::max(TILES_ON_FLY_MIN_UPPER_LIMIT, tilesInVisArea * 1.1f);
}
else
{
commit 9683f53a9c81dfcb9624705cbeeed48ec426ca3b
Author: Tamás Zolnai <tamas.zolnai at collabora.com>
AuthorDate: Wed Sep 26 22:15:37 2018 +0200
Commit: Tamás Zolnai <tamas.zolnai at collabora.com>
CommitDate: Sat Sep 29 09:27:06 2018 +0200
Filter out tiles by wired on wsd side too
Some times tiles with the same wireID survives the wireID
filtering in kit, so we should do that in wsd too.
The issue is with the tilesBeingRendered construction. First when
one tile is filtered out on kit side the client remains subcribed
to the tile, since wsd does not know filtering happened.
Second via the tilesBeingRendered object more clients can be subcribed
to the same tile and so when one client request a new version of this
tile (with an old wireID) the rendered tile is sent to all subscribed
clients even if the other clients has up-to-date tiles.
Change-Id: I4ca6b7a83a5d6979a9f924d766a71aba5e5362c7
diff --git a/test/TileCacheTests.cpp b/test/TileCacheTests.cpp
index 9019cebd8..6da19f3be 100644
--- a/test/TileCacheTests.cpp
+++ b/test/TileCacheTests.cpp
@@ -85,6 +85,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST(testTileProcessed);
CPPUNIT_TEST(testTileInvalidatedOutside);
CPPUNIT_TEST(testTileBeingRenderedHandling);
+ CPPUNIT_TEST(testWireIDFilteringOnWSDSide);
CPPUNIT_TEST_SUITE_END();
@@ -114,6 +115,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
void testTileProcessed();
void testTileInvalidatedOutside();
void testTileBeingRenderedHandling();
+ void testWireIDFilteringOnWSDSide();
void checkTiles(std::shared_ptr<LOOLWebSocket>& socket,
const std::string& type,
@@ -1402,6 +1404,82 @@ void TileCacheTests::testTileBeingRenderedHandling()
}
}
+void TileCacheTests::testWireIDFilteringOnWSDSide()
+{
+ const char* testname = "testWireIDFilteringOnWSDSide ";
+
+ std::string documentPath, documentURL;
+ getDocumentPathAndURL("empty.odt", documentPath, documentURL, testname);
+ std::shared_ptr<LOOLWebSocket> socket1 = loadDocAndGetSocket(_uri, documentURL, testname);
+ // Set the client visible area
+ sendTextFrame(socket1, "clientvisiblearea x=-4005 y=0 width=50490 height=72300");
+ sendTextFrame(socket1, "clientzoom tilepixelwidth=256 tilepixelheight=256 tiletwipwidth=3840 tiletwipheight=3840");
+
+ std::shared_ptr<LOOLWebSocket> socket2 = loadDocAndGetSocket(_uri, documentURL, testname, true);
+ // Set the client visible area
+ sendTextFrame(socket1, "clientvisiblearea x=-4005 y=0 width=50490 height=72300");
+ sendTextFrame(socket1, "clientzoom tilepixelwidth=256 tilepixelheight=256 tiletwipwidth=3840 tiletwipheight=3840");
+
+ //1. First make the first client to trigger the kit to filter out tiles based on identical wireIDs
+
+ // Type one character to trigger invalidation
+ sendChar(socket1, 'x', skNone, testname);
+
+ // First wsd forwards the invalidation
+ assertResponseString(socket1, "invalidatetiles:", testname);
+
+ // For the first input wsd will send all invalidated tiles
+ int arrivedTiles = 0;
+ bool gotTile = false;
+ do
+ {
+ std::vector<char> tile = getResponseMessage(socket1, "tile:", testname);
+ gotTile = !tile.empty();
+ if(gotTile)
+ ++arrivedTiles;
+ } while(gotTile);
+
+ CPPUNIT_ASSERT_MESSAGE("We expect two tiles at least!", arrivedTiles > 1);
+
+ // Type an other character
+ sendChar(socket1, 'x', skNone, testname);
+ assertResponseString(socket1, "invalidatetiles:", testname);
+
+ // For the second input wsd will send one tile, since other tiles are indentical
+ arrivedTiles = 0;
+ gotTile = false;
+ do
+ {
+ std::vector<char> tile = getResponseMessage(socket1, "tile:", testname);
+ gotTile = !tile.empty();
+ if(gotTile)
+ ++arrivedTiles;
+ } while(gotTile);
+
+ CPPUNIT_ASSERT_EQUAL(1, arrivedTiles);
+
+ //2. Now request the same tiles by the other client (e.g. scroll to the same view)
+
+ sendTextFrame(socket2, "tilecombine part=0 width=256 height=256 tileposx=0,3840,7680 tileposy=0,0,0 tilewidth=3840 tileheight=3840");
+
+ // We expect three tiles sent to the second client
+ arrivedTiles = 0;
+ gotTile = false;
+ do
+ {
+ std::vector<char> tile = getResponseMessage(socket2, "tile:", testname);
+ gotTile = !tile.empty();
+ if(gotTile)
+ ++arrivedTiles;
+ } while(gotTile);
+
+ CPPUNIT_ASSERT_EQUAL(3, arrivedTiles);
+
+ // wsd should not send tiles messages for the first client
+ std::vector<char> tile = getResponseMessage(socket1, "tile:", testname);
+ CPPUNIT_ASSERT_MESSAGE("Not expected tile message arrived!", tile.empty());
+}
+
CPPUNIT_TEST_SUITE_REGISTRATION(TileCacheTests);
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 9a17858da..a64141893 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -1026,6 +1026,41 @@ bool ClientSession::forwardToClient(const std::shared_ptr<Message>& payload)
return true;
}
+void ClientSession::enqueueSendMessage(const std::shared_ptr<Message>& data)
+{
+ const std::shared_ptr<DocumentBroker> docBroker = _docBroker.lock();
+ // If in the correct thread - no need for wakeups.
+ if (docBroker)
+ docBroker->assertCorrectThread();
+
+ const std::string command = data->firstToken();
+ if (command == "tile:")
+ {
+ // Avoid sending tile if it has the same wireID as the previously sent tile
+ const TileDesc tile = TileDesc::parse(data->firstLine());
+ const std::string tileID = generateTileID(tile);
+ auto iter = _oldWireIds.find(tileID);
+ if(iter != _oldWireIds.end() && tile.getWireId() != 0 && tile.getWireId() == iter->second)
+ {
+ LOG_INF("WSD filters out a tile with the same wireID: " << tile.serialize("tile:"));
+ return;
+ }
+ }
+
+ LOG_TRC(getName() << " enqueueing client message " << data->id());
+ size_t sizeBefore = _senderQueue.size();
+ size_t newSize = _senderQueue.enqueue(data);
+
+ // Track sent tile
+ if (command == "tile:")
+ {
+ const TileDesc tile = TileDesc::parse(data->firstLine());
+ traceTileBySend(tile, sizeBefore == newSize);
+ if (sizeBefore != newSize)
+ LOG_INF("Sending new tile to client: " << tile.serialize("tile:"));
+ }
+}
+
Authorization ClientSession::getAuthorization() const
{
Poco::URI::QueryParameters queryParams = _uriPublic.getQueryParameters();
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 326ff50c0..45a254609 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -78,25 +78,7 @@ public:
return true;
}
- void enqueueSendMessage(const std::shared_ptr<Message>& data)
- {
- const std::shared_ptr<DocumentBroker> docBroker = _docBroker.lock();
- // If in the correct thread - no need for wakeups.
- if (docBroker)
- docBroker->assertCorrectThread();
-
- LOG_TRC(getName() << " enqueueing client message " << data->id());
- size_t sizeBefore = _senderQueue.size();
- size_t newSize = _senderQueue.enqueue(data);
-
- // Track sent tile
- const std::string command = data->firstToken();
- if (command == "tile:")
- {
- const TileDesc tile = TileDesc::parse(data->firstLine());
- traceTileBySend(tile, sizeBefore == newSize);
- }
- }
+ void enqueueSendMessage(const std::shared_ptr<Message>& data);
/// Set the save-as socket which is used to send convert-to results.
void setSaveAsSocket(const std::shared_ptr<StreamSocket>& socket)
diff --git a/wsd/TestStubs.cpp b/wsd/TestStubs.cpp
index af4231e5d..405b7a6c4 100644
--- a/wsd/TestStubs.cpp
+++ b/wsd/TestStubs.cpp
@@ -28,4 +28,6 @@ void ClientSession::traceUnSubscribeToTile(const std::string& /*tileCacheName*/)
void ClientSession::clearTileSubscription() {};
+void ClientSession::enqueueSendMessage(const std::shared_ptr<Message>& /*data*/) {};
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
commit a1a0bf3718f5dff3996a3ace98a6a9cb0f10f55b
Author: Tamás Zolnai <tamas.zolnai at collabora.com>
AuthorDate: Wed Sep 26 22:16:11 2018 +0200
Commit: Tamás Zolnai <tamas.zolnai at collabora.com>
CommitDate: Sat Sep 29 09:27:06 2018 +0200
Don't send tiles which was changed outside of the visible area
Since this commit:
9473908d45a884827356b504c5f768e2f29cc46b
We can avoid that, because the tiles will be invalidated
on the client side and when the client visible area changes
the invalidated tiles are requested anyway.
Change-Id: I272e3b4b87380ae574c16a2b480dbc8caabf4b32
diff --git a/test/TileCacheTests.cpp b/test/TileCacheTests.cpp
index 619ccb5a4..9019cebd8 100644
--- a/test/TileCacheTests.cpp
+++ b/test/TileCacheTests.cpp
@@ -1339,8 +1339,11 @@ void TileCacheTests::testTileInvalidatedOutside()
// First wsd forwards the invalidation
assertResponseString(socket, "invalidatetiles:", testname);
- // Then sends the new tile which was invalidated inside the visible area
- assertResponseString(socket, "tile:", testname);
+ // Since the invalidation rectangle is outside the visible area
+ // wsd does not send a new tile even if some of the invalidated tiles
+ // are partly visible.
+ std::vector<char> tile = getResponseMessage(socket, "tile:", testname);
+ CPPUNIT_ASSERT_MESSAGE("Not expected tile message arrived!", tile.empty());
}
void TileCacheTests::testTileBeingRenderedHandling()
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index ba7c7d123..9a17858da 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -1192,9 +1192,14 @@ void ClientSession::handleTileInvalidation(const std::string& message,
int part = result.first;
Util::Rectangle& invalidateRect = result.second;
+ // We can ignore the invalidation if it's outside of the visible area
+ if(!normalizedVisArea.intersects(invalidateRect))
+ return;
+
if( part == -1 ) // If no part is specifed we use the part used by the client
part = _clientSelectedPart;
+
std::vector<TileDesc> invalidTiles;
if(part == _clientSelectedPart || _isTextDocument)
{
commit 07e99ad336c87f3d7e211abaaf18639be27c2929
Author: Tamás Zolnai <tamas.zolnai at collabora.com>
AuthorDate: Wed Sep 26 22:15:30 2018 +0200
Commit: Tamás Zolnai <tamas.zolnai at collabora.com>
CommitDate: Sat Sep 29 09:27:06 2018 +0200
Fix tilesBeingRendered tracking by client
I changed the code in this commit:
c2a5f6acb0f1e93f19104b761661c852d930fb9e
To make kit send a tilecombine message even if it does not
send actual tile data so we can track that the rendering was
done and so we can update the clients' _tilesBeingRendered
list. The issue is that tileBeingRendered object
belongs to not only one client, but more and so we don't
know which client gets the actual empty tile response.
So revert this method and rather use a smaller timeout for "waiting" the
arrival of the rendered tile.
Change-Id: I2dbbab1a62b81cbbb5314f2f37fdbc3415c69130
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index d8cce0218..826cdd226 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -1127,9 +1127,7 @@ public:
// The tile content is identical to what the client already has, so skip it
LOG_TRC("Match for tile #" << tileIndex << " at (" << positionX << "," <<
positionY << ") oldhash==hash (" << hash << "), wireId: " << wireId << " skipping");
- tiles[tileIndex].setWireId(wireId);
- tiles[tileIndex].setImgSize(0);
- tileIndex++;
+ tiles.erase(tiles.begin() + tileIndex);
continue;
}
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 317024d8b..ba7c7d123 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -51,8 +51,7 @@ ClientSession::ClientSession(const std::string& id,
_tileHeightPixel(0),
_tileWidthTwips(0),
_tileHeightTwips(0),
- _isTextDocument(false),
- _tilesBeingRendered(0)
+ _isTextDocument(false)
{
const size_t curConnections = ++LOOLWSD::NumConnections;
LOG_INF("ClientSession ctor [" << getName() << "], current number of connections: " << curConnections);
@@ -1282,9 +1281,9 @@ void ClientSession::removeOutdatedTileSubscriptions()
while(iterator != _tilesBeingRendered.end())
{
double elapsedTime = docBroker->tileCache().getTileBeingRenderedElapsedTimeMs(*iterator);
- if(elapsedTime < 0.0 && elapsedTime > 5000.0)
+ if(elapsedTime < 0.0 && elapsedTime > 200.0)
{
- LOG_WRN("Tracked TileBeingRendered was dropped because of time out.");
+ LOG_INF("Tracked TileBeingRendered was dropped because of time out.");
_tilesBeingRendered.erase(iterator);
}
else
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 3ea0e8916..5f2ac353e 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1545,16 +1545,25 @@ void DocumentBroker::handleTileCombinedResponse(const std::vector<char>& payload
try
{
- const TileCombined tileCombined = TileCombined::parse(firstLine);
- const char* buffer = payload.data();
- size_t offset = firstLine.size() + 1;
+ const size_t length = payload.size();
+ if (firstLine.size() < static_cast<std::string::size_type>(length) - 1)
+ {
+ const TileCombined tileCombined = TileCombined::parse(firstLine);
+ const char* buffer = payload.data();
+ size_t offset = firstLine.size() + 1;
- std::unique_lock<std::mutex> lock(_mutex);
+ std::unique_lock<std::mutex> lock(_mutex);
- for (const auto& tile : tileCombined.getTiles())
+ for (const auto& tile : tileCombined.getTiles())
+ {
+ tileCache().saveTileAndNotify(tile, buffer + offset, tile.getImgSize());
+ offset += tile.getImgSize();
+ }
+ }
+ else
{
- tileCache().saveTileAndNotify(tile, buffer + offset, tile.getImgSize());
- offset += tile.getImgSize();
+ LOG_WRN("Dropping empty tilecombine response: " << firstLine);
+ // They will get re-issued if we don't forget them.
}
}
catch (const std::exception& exc)
diff --git a/wsd/TileCache.cpp b/wsd/TileCache.cpp
index 2a45cbd01..3ee3647ea 100644
--- a/wsd/TileCache.cpp
+++ b/wsd/TileCache.cpp
@@ -183,17 +183,6 @@ void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const
std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(tile);
- // Kit did not send image data, because tile has the same wireID as the previously sent tile
- // We need to remove only the subscriptions
- if(size == 0)
- {
- if(tileBeingRendered && tileBeingRendered->getVersion() <= tile.getVersion())
- {
- forgetTileBeingRendered(tileBeingRendered, tile);
- }
- return;
- }
-
// Save to disk.
const std::string cachedName = (tileBeingRendered ? tileBeingRendered->getCacheName()
: cacheFileName(tile));
commit 5260eae05f89056d4ffc1bdfad1927c3c5f9706d
Author: Tamás Zolnai <tamas.zolnai at collabora.com>
AuthorDate: Wed Sep 26 22:15:26 2018 +0200
Commit: Tamás Zolnai <tamas.zolnai at collabora.com>
CommitDate: Sat Sep 29 09:27:06 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.
Change-Id: I1374c22e5ffebe1d6fcc6e37261ddcedeb9066ec
diff --git a/test/TileCacheTests.cpp b/test/TileCacheTests.cpp
index 32a47169d..619ccb5a4 100644
--- a/test/TileCacheTests.cpp
+++ b/test/TileCacheTests.cpp
@@ -84,6 +84,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST(testTileWireIDHandling);
CPPUNIT_TEST(testTileProcessed);
CPPUNIT_TEST(testTileInvalidatedOutside);
+ CPPUNIT_TEST(testTileBeingRenderedHandling);
CPPUNIT_TEST_SUITE_END();
@@ -112,6 +113,7 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
void testTileWireIDHandling();
void testTileProcessed();
void testTileInvalidatedOutside();
+ void testTileBeingRenderedHandling();
void checkTiles(std::shared_ptr<LOOLWebSocket>& socket,
const std::string& type,
@@ -1230,19 +1232,18 @@ void TileCacheTests::testTileWireIDHandling()
sendChar(socket, 'x', skNone, testname);
assertResponseString(socket, "invalidatetiles:", testname);
- // For the second input wsd will send less tiles, since some of them are indentical
- int arrivedTiles2 = 0;
+ // For the second input wsd will send one tile, since some of them are indentical
+ arrivedTiles = 0;
gotTile = false;
do
{
std::vector<char> tile = getResponseMessage(socket, "tile:", testname);
gotTile = !tile.empty();
if(gotTile)
- ++arrivedTiles2;
+ ++arrivedTiles;
} while(gotTile);
- CPPUNIT_ASSERT_MESSAGE("We expect one tile at least!", arrivedTiles2 >= 1);
- CPPUNIT_ASSERT_MESSAGE("We expect less tile for the second input (wireID)!", arrivedTiles > arrivedTiles2);
+ CPPUNIT_ASSERT_EQUAL(1, arrivedTiles);
}
void TileCacheTests::testTileProcessed()
@@ -1342,6 +1343,62 @@ void TileCacheTests::testTileInvalidatedOutside()
assertResponseString(socket, "tile:", testname);
}
+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 7fe5646ee..3ea0e8916 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1322,15 +1322,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);
}
}
@@ -1461,9 +1462,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 52d923ffd..2a45cbd01 100644
--- a/wsd/TileCache.cpp
+++ b/wsd/TileCache.cpp
@@ -145,6 +145,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)
{
const std::string fileName = _cacheDir + "/" + cacheFileName(tile);
@@ -526,6 +540,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 dbee88304..52fb104ff 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(const 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();
commit b7f76d24fb49433c1d6d3b540a33530a482e6df3
Author: Tamás Zolnai <tamas.zolnai at collabora.com>
AuthorDate: Wed Sep 26 22:15:18 2018 +0200
Commit: Tamás Zolnai <tamas.zolnai at collabora.com>
CommitDate: Sat Sep 29 09:27:06 2018 +0200
loleaflet: Reset invalid counter when request new tiles by view change
This invalidCount does not work perfectly, since we don't always
send tile after every invalidation message (e.g. tile deduplication
on severs side, wireID handling). So this invalidCount usage should
be reconsidered. Now just avoid to have a tile still invalid after
we requested (and get) the newest tile when we are changing the view.
Change-Id: I249be63611767af02b04e142bb1c2afcb3a8eebb
diff --git a/loleaflet/src/layer/tile/GridLayer.js b/loleaflet/src/layer/tile/GridLayer.js
index ac4ec1190..679f5caa3 100644
--- a/loleaflet/src/layer/tile/GridLayer.js
+++ b/loleaflet/src/layer/tile/GridLayer.js
@@ -544,6 +544,9 @@ L.GridLayer = L.Layer.extend({
if (tile && tile.loaded && !invalid) {
tile.current = true;
newView = false;
+ } else if (invalid) {
+ tile._invalidCount = 1;
+ queue.push(coords);
} else {
queue.push(coords);
}
More information about the Libreoffice-commits
mailing list