[Libreoffice-commits] online.git: loolwsd/MessageQueue.cpp loolwsd/MessageQueue.hpp loolwsd/test
Jan Holesovsky
kendy at collabora.com
Tue Oct 4 12:26:50 UTC 2016
loolwsd/MessageQueue.cpp | 35 +++++++++++++++-
loolwsd/MessageQueue.hpp | 4 +
loolwsd/test/TileQueueTests.cpp | 83 +++++++++++++++++++++++++++++++++++++---
3 files changed, 113 insertions(+), 9 deletions(-)
New commits:
commit a1e9c1eb42c711bbb2ab8293979daeb18c1b6094
Author: Jan Holesovsky <kendy at collabora.com>
Date: Tue Oct 4 14:23:42 2016 +0200
Handle the Impress slide previews with low priority + unit test.
After processing a tile request for a slide preview, move the rest of them to
the end of the queue, so that other events are processed before we render the
next preview.
Change-Id: I82aa411406d6731b1c0bf3b849780220b5e68110
diff --git a/loolwsd/MessageQueue.cpp b/loolwsd/MessageQueue.cpp
index f06f670..ffa5f79 100644
--- a/loolwsd/MessageQueue.cpp
+++ b/loolwsd/MessageQueue.cpp
@@ -143,6 +143,9 @@ void TileQueue::removeDuplicate(const std::string& tileMsg)
// input from clients, but strings we have created ourselves here in C++ code, so probably we
// can be sure that the "ver" parameter is always in such a location that this does what we
// mean.
+ // FIXME: also the ver=... is only for debugging from what I can see, so
+ // double-check if we can actually avoid the 'ver' everywhere in the non-debug
+ // builds
const std::string newMsg = tileMsg.substr(0, tileMsg.find(" ver"));
for (size_t i = 0; i < _queue.size(); ++i)
@@ -173,19 +176,43 @@ int TileQueue::priority(const std::string& tileMsg)
return -1;
}
+void TileQueue::deprioritizePreviews()
+{
+ for (size_t i = 0; i < _queue.size(); ++i)
+ {
+ const auto front = _queue.front();
+ const std::string message(front.data(), front.size());
+
+ // stop at the first non-tile or non-'id' (preview) message
+ std::string id;
+ if (LOOLProtocol::getFirstToken(message) != "tile" || !LOOLProtocol::getTokenStringFromMessage(message, "id", id))
+ break;
+
+ _queue.pop_front();
+ _queue.push_back(front);
+ }
+}
+
MessageQueue::Payload TileQueue::get_impl()
{
- std::vector<TileDesc> tiles;
const auto front = _queue.front();
auto msg = std::string(front.data(), front.size());
std::string id;
- if (LOOLProtocol::getFirstToken(msg) != "tile" || LOOLProtocol::getTokenStringFromMessage(msg, "id", id))
+ bool isTile = (LOOLProtocol::getFirstToken(msg) == "tile");
+ bool isPreview = isTile && LOOLProtocol::getTokenStringFromMessage(msg, "id", id);
+ if (!isTile || isPreview)
{
// Don't combine non-tiles or tiles with id.
Log::trace() << "MessageQueue res: " << msg << Log::end;
_queue.pop_front();
+
+ // de-prioritize the other tiles with id - usually the previews in
+ // Impress
+ if (isPreview)
+ deprioritizePreviews();
+
return front;
}
@@ -201,7 +228,7 @@ MessageQueue::Payload TileQueue::get_impl()
// avoid starving - stop the search when we reach a non-tile,
// otherwise we may keep growing the queue of unhandled stuff (both
// tiles and non-tiles)
- if (LOOLProtocol::getFirstToken(prio) != "tile")
+ if (LOOLProtocol::getFirstToken(prio) != "tile" || LOOLProtocol::getTokenStringFromMessage(prio, "id", id))
break;
int p = priority(prio);
@@ -220,6 +247,7 @@ MessageQueue::Payload TileQueue::get_impl()
_queue.erase(_queue.begin() + prioritized);
+ std::vector<TileDesc> tiles;
tiles.emplace_back(TileDesc::parse(msg));
// Combine as many tiles as possible with the top one.
@@ -254,6 +282,7 @@ MessageQueue::Payload TileQueue::get_impl()
if (tiles.size() == 1)
{
msg = tiles[0].serialize("tile");
+ Log::trace() << "MessageQueue res: " << msg << Log::end;
return Payload(msg.data(), msg.data() + msg.size());
}
diff --git a/loolwsd/MessageQueue.hpp b/loolwsd/MessageQueue.hpp
index 2a13484..8ce2287 100644
--- a/loolwsd/MessageQueue.hpp
+++ b/loolwsd/MessageQueue.hpp
@@ -131,6 +131,10 @@ private:
/// Search the queue for a duplicate tile and remove it (if present).
void removeDuplicate(const std::string& tileMsg);
+ /// De-prioritize the previews (tiles with 'id') - move them to the end of
+ /// the queue.
+ void deprioritizePreviews();
+
/// Priority of the given tile message.
/// -1 means the lowest prio (the tile does not intersect any of the cursors),
/// the higher the number, the bigger is priority [up to _viewOrder.size()-1].
diff --git a/loolwsd/test/TileQueueTests.cpp b/loolwsd/test/TileQueueTests.cpp
index efe13cf..964df48 100644
--- a/loolwsd/test/TileQueueTests.cpp
+++ b/loolwsd/test/TileQueueTests.cpp
@@ -43,6 +43,7 @@ class TileQueueTests : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST(testTileCombinedRendering);
CPPUNIT_TEST(testTileRecombining);
CPPUNIT_TEST(testViewOrder);
+ CPPUNIT_TEST(testPreviewsDeprioritization);
CPPUNIT_TEST_SUITE_END();
@@ -50,6 +51,7 @@ class TileQueueTests : public CPPUNIT_NS::TestFixture
void testTileCombinedRendering();
void testTileRecombining();
void testViewOrder();
+ void testPreviewsDeprioritization();
};
void TileQueueTests::testTileQueuePriority()
@@ -131,6 +133,15 @@ void TileQueueTests::testTileCombinedRendering()
CPPUNIT_ASSERT_EQUAL(payloadFull, queue.get());
}
+namespace {
+
+std::string payloadAsString(const MessageQueue::Payload& payload)
+{
+ return std::string(payload.data(), payload.size());
+}
+
+}
+
void TileQueueTests::testTileRecombining()
{
TileQueue queue;
@@ -142,8 +153,7 @@ void TileQueueTests::testTileRecombining()
CPPUNIT_ASSERT_EQUAL(3, static_cast<int>(queue._queue.size()));
// but when we later extract that, it is just one "tilecombine" message
- MessageQueue::Payload payload(queue.get());
- std::string message(payload.data(), payload.size());
+ std::string message(payloadAsString(queue.get()));
CPPUNIT_ASSERT_EQUAL(std::string("tilecombine part=0 width=256 height=256 tileposx=7680,0,3840 tileposy=0,0,0 imgsize=0,0,0 tilewidth=3840 tileheight=3840"), message);
@@ -174,18 +184,79 @@ void TileQueueTests::testViewOrder()
for (auto &tile : tiles)
queue.put(tile);
- CPPUNIT_ASSERT_EQUAL(4, static_cast<int>(tiles.size()));
+ CPPUNIT_ASSERT_EQUAL(4, static_cast<int>(queue._queue.size()));
// should result in the 3, 2, 1, 0 order of the tiles thanks to the cursor
// positions
for (size_t i = 0; i < tiles.size(); ++i)
{
- MessageQueue::Payload payload(queue.get());
- std::string message(payload.data(), payload.size());
+ CPPUNIT_ASSERT_EQUAL(tiles[3 - i], payloadAsString(queue.get()));
+ }
+}
+
+void TileQueueTests::testPreviewsDeprioritization()
+{
+ TileQueue queue;
+
+ // simple case - put previews to the queue and get everything back again
+ const std::vector<std::string> previews =
+ {
+ "tile part=0 width=180 height=135 tileposx=0 tileposy=0 tilewidth=15875 tileheight=11906 ver=-1 id=0",
+ "tile part=1 width=180 height=135 tileposx=0 tileposy=0 tilewidth=15875 tileheight=11906 ver=-1 id=1",
+ "tile part=2 width=180 height=135 tileposx=0 tileposy=0 tilewidth=15875 tileheight=11906 ver=-1 id=2",
+ "tile part=3 width=180 height=135 tileposx=0 tileposy=0 tilewidth=15875 tileheight=11906 ver=-1 id=3"
+ };
- CPPUNIT_ASSERT_EQUAL(tiles[3 - i], message);
+ for (auto &preview : previews)
+ queue.put(preview);
+
+ for (size_t i = 0; i < previews.size(); ++i)
+ {
+ CPPUNIT_ASSERT_EQUAL(previews[i], payloadAsString(queue.get()));
}
+ // stays empty after all is done
+ CPPUNIT_ASSERT_EQUAL(0, static_cast<int>(queue._queue.size()));
+
+ // re-ordering case - put previews and normal tiles to the queue and get
+ // everything back again but this time the tiles have to interleave with
+ // the previews
+ const std::vector<std::string> tiles =
+ {
+ "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 ver=-1",
+ "tile part=0 width=256 height=256 tileposx=0 tileposy=7680 tilewidth=3840 tileheight=3840 ver=-1"
+ };
+
+ for (auto &preview : previews)
+ queue.put(preview);
+
+ queue.put(tiles[0]);
+
+ CPPUNIT_ASSERT_EQUAL(previews[0], payloadAsString(queue.get()));
+ CPPUNIT_ASSERT_EQUAL(tiles[0], payloadAsString(queue.get()));
+ CPPUNIT_ASSERT_EQUAL(previews[1], payloadAsString(queue.get()));
+
+ queue.put(tiles[1]);
+
+ CPPUNIT_ASSERT_EQUAL(previews[2], payloadAsString(queue.get()));
+ CPPUNIT_ASSERT_EQUAL(tiles[1], payloadAsString(queue.get()));
+ CPPUNIT_ASSERT_EQUAL(previews[3], payloadAsString(queue.get()));
+
+ // stays empty after all is done
+ CPPUNIT_ASSERT_EQUAL(0, static_cast<int>(queue._queue.size()));
+
+ // cursor positioning case - the cursor position should not prioritize the
+ // previews
+ queue.updateCursorPosition(0, 0, 0, 0, 10, 100);
+
+ queue.put(tiles[1]);
+ queue.put(previews[0]);
+
+ CPPUNIT_ASSERT_EQUAL(tiles[1], payloadAsString(queue.get()));
+ CPPUNIT_ASSERT_EQUAL(previews[0], payloadAsString(queue.get()));
+
+ // stays empty after all is done
+ CPPUNIT_ASSERT_EQUAL(0, static_cast<int>(queue._queue.size()));
}
CPPUNIT_TEST_SUITE_REGISTRATION(TileQueueTests);
More information about the Libreoffice-commits
mailing list