[Libreoffice-commits] online.git: 10 commits - common/MessageQueue.cpp common/MessageQueue.hpp kit/Kit.cpp test/TileQueueTests.cpp test/WhiteBoxTests.cpp wsd/TileDesc.hpp
Jan Holesovsky
kendy at collabora.com
Thu Jan 26 11:27:36 UTC 2017
common/MessageQueue.cpp | 437 +++++++++++++++++++++++++++++++-----------------
common/MessageQueue.hpp | 45 +---
kit/Kit.cpp | 154 +++++-----------
test/TileQueueTests.cpp | 37 +++-
test/WhiteBoxTests.cpp | 28 +++
wsd/TileDesc.hpp | 13 -
6 files changed, 421 insertions(+), 293 deletions(-)
New commits:
commit 545e2a2abe7aa21b7723944ca6134114179bde1c
Author: Jan Holesovsky <kendy at collabora.com>
Date: Wed Jan 25 19:42:36 2017 +0100
Fix a size check.
Change-Id: I509d12dcde6f56a2a7a9ee244e721d8028dec501
diff --git a/common/MessageQueue.cpp b/common/MessageQueue.cpp
index 54f1871..05bdf58 100644
--- a/common/MessageQueue.cpp
+++ b/common/MessageQueue.cpp
@@ -158,7 +158,7 @@ bool extractRectangle(const std::vector<std::string>& tokens, int& x, int& y, in
h = INT_MAX;
part = 0;
- if (tokens.size() < 4)
+ if (tokens.size() < 5)
return false;
if (tokens[3] == "EMPTY,")
commit c44f7b8a76d2ddcaf90ef5092d3c32a5b885f423
Author: Jan Holesovsky <kendy at collabora.com>
Date: Wed Jan 25 19:14:38 2017 +0100
Revert "loolwsd: improved MessageQueue"
Now we don't get a situation where there would be a tremendous amount of
invalidates & tile render requests piled in the queue, so we can do it
deterministic again.
The only thing that could potentially pile in the queue are the keypresses
events sent from the clients, but that is a different problem anyway.
This reverts commit c326228774e720011b8845f0ee14b4c0a5f1d4f2.
Change-Id: I98e199eab0187bf5f47ce322ac1b1b2e3b976b85
diff --git a/common/MessageQueue.cpp b/common/MessageQueue.cpp
index f31d665..54f1871 100644
--- a/common/MessageQueue.cpp
+++ b/common/MessageQueue.cpp
@@ -401,174 +401,46 @@ int TileQueue::priority(const std::string& tileMsg)
return -1;
}
-int TileQueue::findFirstNonPreview(bool preferTiles) const
+void TileQueue::deprioritizePreviews()
{
- int firstTile = -1;
- int firstElse = -1;
for (size_t i = 0; i < _queue.size(); ++i)
{
- const auto& front = _queue[i];
- const bool isTile = LOOLProtocol::matchPrefix("tile", front);
- //LOG_WRN("#" << i << " " << (isTile ? "isTile" : "non-tile"));
+ const auto front = _queue.front();
+ const std::string message(front.data(), front.size());
- if (isTile && firstTile < 0)
- {
- const std::string msg(front.data(), front.size());
- std::string id;
- const bool isPreview = LOOLProtocol::getTokenStringFromMessage(msg, "id", id);
- //LOG_WRN("#" << i << " " << (isPreview ? "isPreview" : "isTile") << ": " << msg);
- if (!isPreview)
- {
- firstTile = i;
- //LOG_WRN("firstTile: #" << i);
- }
- }
- else if (!isTile && firstElse < 0)
- {
- firstElse = i;
- //LOG_WRN("firstElse: #" << i);
- }
- else if (firstTile >=0 && firstElse >= 0)
+ // stop at the first non-tile or non-'id' (preview) message
+ std::string id;
+ if (!LOOLProtocol::matchPrefix("tile", message) ||
+ !LOOLProtocol::getTokenStringFromMessage(message, "id", id))
{
break;
}
- }
-
- if (preferTiles && firstTile >= 0)
- {
- return firstTile;
- }
-
- if (firstElse >= 0)
- {
- return firstElse;
- }
-
- if (firstTile >= 0)
- {
- return firstTile;
- }
-
- return -1;
-}
-
-void TileQueue::bumpToTop(const size_t index)
-{
- if (index > 0)
- {
- Payload payload(_queue[index]);
- //LOG_WRN("Bumping: " << std::string(payload.data(), payload.size()));
-
- _queue.erase(_queue.begin() + index);
- _queue.insert(_queue.begin(), payload);
- }
-}
-
-void TileQueue::updateTimestamps(const bool isTile)
-{
- if (isTile)
- {
- _lastGetTile = true;
- _lastTileGetTime = std::chrono::steady_clock::now();
- }
- else if (_lastGetTile)
- {
- // Update non-tile timestamp when switching from tiles.
- _lastGetTile = false;
- _lastGetTime = std::chrono::steady_clock::now();
- }
-}
-
-bool TileQueue::shouldPreferTiles() const
-{
- if (_lastGetTile)
- {
- // If we had just done a tile, do something else.
- LOG_TRC("Last was tile, doing non-tiles.");
- return false;
- }
- // Check how long it's been since we'd done tiles.
- const auto tileDuration = (_lastGetTime - _lastTileGetTime);
- const auto tileDurationMs = std::chrono::duration_cast<std::chrono::milliseconds>(tileDuration).count();
- const auto duration = (std::chrono::steady_clock::now() - _lastGetTime);
- const auto durationMs = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count();
- LOG_TRC("Tile duration: " << tileDurationMs << "ms, nonTile duration: " << durationMs << "ms.");
-
- if (durationMs > MaxTileSkipDurationMs)
- {
- LOG_TRC("Capping non-tiles to 100ms. Prefer tiles now.");
- return true;
- }
-
- if (durationMs > tileDurationMs)
- {
- LOG_TRC("Capping non-tiles to tileDurationMs (" << tileDurationMs << "). Prefer tiles now.");
- return true;
+ _queue.erase(_queue.begin());
+ _queue.push_back(front);
}
-
- // We can still do some more non-tiles.
- LOG_TRC("Have time for more non-tiles.");
- return false;
}
TileQueue::Payload TileQueue::get_impl()
{
- LOG_TRC("MessageQueue depth: " << _queue.size());
+ const auto front = _queue.front();
- auto front = _queue.front();
- bool isTileFirst = LOOLProtocol::matchPrefix("tile", front);
- //LOG_WRN("isTileFirst: " << isTileFirst);
-
- if (_queue.size() == 1)
- {
- updateTimestamps(isTileFirst);
-
- //const auto msg = std::string(front.data(), front.size());
- //LOG_TRC("MessageQueue res only: " << msg);
- _queue.erase(_queue.begin());
- return front;
- }
+ auto msg = std::string(front.data(), front.size());
- // Drain callbacks as soon and as fast as possible.
- if (!isTileFirst && LOOLProtocol::matchPrefix("callback", front))
+ std::string id;
+ bool isTile = LOOLProtocol::matchPrefix("tile", msg);
+ bool isPreview = isTile && LOOLProtocol::getTokenStringFromMessage(msg, "id", id);
+ if (!isTile || isPreview)
{
- updateTimestamps(false);
-
- //const auto msg = std::string(front.data(), front.size());
- //LOG_TRC("MessageQueue res call: " << msg);
+ // Don't combine non-tiles or tiles with id.
+ LOG_TRC("MessageQueue res: " << msg);
_queue.erase(_queue.begin());
- return front;
- }
- // TODO: Try draining all callbacks first.
+ // de-prioritize the other tiles with id - usually the previews in
+ // Impress
+ if (isPreview)
+ deprioritizePreviews();
- const bool preferTiles = shouldPreferTiles();
- const int nonPreviewIndex = findFirstNonPreview(preferTiles);
- //LOG_WRN("First non-preview: " << nonPreviewIndex);
- if (nonPreviewIndex < 0)
- {
- // We are left with previews only.
- updateTimestamps(true); // We're doing a tile.
-
- //const auto msg = std::string(front.data(), front.size());
- //LOG_TRC("MessageQueue res prev: " << msg);
- _queue.erase(_queue.begin());
- return front;
- }
-
- bumpToTop(nonPreviewIndex);
- front = _queue.front();
- isTileFirst = LOOLProtocol::matchPrefix("tile", front);
- //LOG_WRN("New front: " << std::string(front.data(), front.size()));
-
- if (!isTileFirst)
- {
- updateTimestamps(false);
-
- //const auto msg = std::string(front.data(), front.size());
- //LOG_TRC("MessageQueue res call: " << msg);
- _queue.erase(_queue.begin());
return front;
}
@@ -576,7 +448,6 @@ TileQueue::Payload TileQueue::get_impl()
// position, otherwise handle the one that is at the front
int prioritized = 0;
int prioritySoFar = -1;
- std::string msg(front.data(), front.size());
for (size_t i = 0; i < _queue.size(); ++i)
{
auto& it = _queue[i];
@@ -585,7 +456,6 @@ TileQueue::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)
- std::string id;
if (!LOOLProtocol::matchPrefix("tile", prio) ||
LOOLProtocol::getTokenStringFromMessage(prio, "id", id))
{
@@ -617,7 +487,6 @@ TileQueue::Payload TileQueue::get_impl()
{
auto& it = _queue[i];
msg = std::string(it.data(), it.size());
- std::string id;
if (!LOOLProtocol::matchPrefix("tile", msg) ||
LOOLProtocol::getTokenStringFromMessage(msg, "id", id))
{
@@ -627,7 +496,7 @@ TileQueue::Payload TileQueue::get_impl()
}
auto tile2 = TileDesc::parse(msg);
- //LOG_TRC("Combining candidate: " << msg);
+ LOG_TRC("Combining candidate: " << msg);
// Check if it's on the same row.
if (tiles[0].onSameRow(tile2))
@@ -641,8 +510,6 @@ TileQueue::Payload TileQueue::get_impl()
}
}
- updateTimestamps(true);
-
LOG_TRC("Combined " << tiles.size() << " tiles, leaving " << _queue.size() << " in queue.");
if (tiles.size() == 1)
@@ -652,7 +519,7 @@ TileQueue::Payload TileQueue::get_impl()
return Payload(msg.data(), msg.data() + msg.size());
}
- const auto tileCombined = TileCombined::create(tiles).serialize("tilecombine");
+ auto tileCombined = TileCombined::create(tiles).serialize("tilecombine");
LOG_TRC("MessageQueue res: " << tileCombined);
return Payload(tileCombined.data(), tileCombined.data() + tileCombined.size());
}
diff --git a/common/MessageQueue.hpp b/common/MessageQueue.hpp
index 1138259..2ab30fb 100644
--- a/common/MessageQueue.hpp
+++ b/common/MessageQueue.hpp
@@ -147,14 +147,6 @@ private:
};
public:
-
- TileQueue() :
- _lastTileGetTime(std::chrono::steady_clock::now()),
- _lastGetTime(_lastTileGetTime),
- _lastGetTile(true)
- {
- }
-
void updateCursorPosition(int viewId, int part, int x, int y, int width, int height)
{
const auto cursorPosition = CursorPosition({ part, x, y, width, height });
@@ -212,25 +204,9 @@ private:
/// @return New message to put into the queue. If empty, use what was in callbackMsg.
std::string removeCallbackDuplicate(const std::string& callbackMsg);
- /// Find the index of the first non-preview entry.
- /// When preferTiles is false, it'll return index of
- /// the first non-tile, otherwise, the index of the
- /// first tile is returned.
- /// Returns -1 if only previews are left.
- int findFirstNonPreview(bool preferTiles) const;
-
- /// Returns true if we should try to return
- /// a tile, otherwise a non-tile.
- bool shouldPreferTiles() const;
-
- /// Update the tile/non-tile timestamps to
- /// track how much time we spend for each.
- /// isTile marks if the current message
- /// is a tile or not.
- void updateTimestamps(const bool isTile);
-
- /// Given a positive index, move it to the top.
- void bumpToTop(const size_t index);
+ /// 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),
@@ -243,13 +219,6 @@ private:
/// Check the views in the order of how the editing (cursor movement) has
/// been happening (0 == oldest, size() - 1 == newest).
std::vector<int> _viewOrder;
-
- std::chrono::steady_clock::time_point _lastTileGetTime;
- std::chrono::steady_clock::time_point _lastGetTime;
- bool _lastGetTile;
-
- /// For responsiveness, we shouldn't have higher latency.
- static const int MaxTileSkipDurationMs = 100;
};
#endif
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 30966f9..16e20ac 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -588,7 +588,6 @@ public:
{
assert(ws && "Expected a non-null websocket.");
auto tile = TileDesc::parse(tokens);
- const double area = tile.getWidth() * tile.getHeight();
// Send back the request with all optional parameters given in the request.
const auto tileMsg = tile.serialize("tile:");
@@ -619,8 +618,7 @@ public:
return;
}
- LOG_TRC("+paintTile at (" << tile.getPart() << ',' << tile.getTilePosX() << ',' << tile.getTilePosY() <<
- ") " << "ver: " << tile.getVersion());
+ const double area = tile.getWidth() * tile.getHeight();
Timestamp timestamp;
_loKitDocument->paintPartTile(pixmap.data(), tile.getPart(),
tile.getWidth(), tile.getHeight(),
@@ -683,8 +681,7 @@ public:
const size_t tilesByY = renderArea.getHeight() / tileCombined.getTileHeight();
const size_t pixmapWidth = tilesByX * tileCombined.getWidth();
const size_t pixmapHeight = tilesByY * tileCombined.getHeight();
- const double area = pixmapWidth * pixmapHeight;
- const size_t pixmapSize = 4 * area;
+ const size_t pixmapSize = 4 * pixmapWidth * pixmapHeight;
std::vector<unsigned char> pixmap(pixmapSize, 0);
std::unique_lock<std::mutex> lock(_documentMutex);
@@ -700,8 +697,7 @@ public:
return;
}
- LOG_DBG("+paintTile (combined) at (" << renderArea.getLeft() << ", " << renderArea.getTop() << "), (" <<
- renderArea.getWidth() << ", " << renderArea.getHeight() << ").");
+ const double area = pixmapWidth * pixmapHeight;
Timestamp timestamp;
_loKitDocument->paintPartTile(pixmap.data(), tileCombined.getPart(),
pixmapWidth, pixmapHeight,
diff --git a/test/TileQueueTests.cpp b/test/TileQueueTests.cpp
index 884ee51..8c4a5ad 100644
--- a/test/TileQueueTests.cpp
+++ b/test/TileQueueTests.cpp
@@ -66,10 +66,10 @@ class TileQueueTests : public CPPUNIT_NS::TestFixture
void TileQueueTests::testTileQueuePriority()
{
- const std::string reqHigh = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 oldhash=0 hash=0 ver=-1";
+ const std::string reqHigh = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 oldhash=0 hash=0";
const std::string resHigh = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 oldhash=0 hash=0 ver=-1";
const TileQueue::Payload payloadHigh(resHigh.data(), resHigh.data() + resHigh.size());
- const std::string reqLow = "tile part=0 width=256 height=256 tileposx=0 tileposy=253440 tilewidth=3840 tileheight=3840 oldhash=0 hash=0 ver=-1";
+ const std::string reqLow = "tile part=0 width=256 height=256 tileposx=0 tileposy=253440 tilewidth=3840 tileheight=3840 oldhash=0 hash=0";
const std::string resLow = "tile part=0 width=256 height=256 tileposx=0 tileposy=253440 tilewidth=3840 tileheight=3840 oldhash=0 hash=0 ver=-1";
const TileQueue::Payload payloadLow(resLow.data(), resLow.data() + resLow.size());
@@ -242,14 +242,14 @@ void TileQueueTests::testPreviewsDeprioritization()
queue.put(tiles[0]);
- CPPUNIT_ASSERT_EQUAL(tiles[0], payloadAsString(queue.get()));
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(tiles[1], payloadAsString(queue.get()));
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
commit 3c9f4e1e1f8f7cf26a17987c1dc8c351072d0b0c
Author: Jan Holesovsky <kendy at collabora.com>
Date: Wed Jan 25 18:21:56 2017 +0100
Deduplicate & remove obsolete invalidations from the queue.
There's no point in trying to paint something we know will be obsolete anyway.
Change-Id: I14f61f389b114f2cda1f97e5223b31fa2f01b06c
diff --git a/common/MessageQueue.cpp b/common/MessageQueue.cpp
index bc61341..f31d665 100644
--- a/common/MessageQueue.cpp
+++ b/common/MessageQueue.cpp
@@ -80,9 +80,15 @@ void TileQueue::put_impl(const Payload& value)
}
else if (firstToken == "callback")
{
- removeCallbackDuplicate(msg);
+ std::string newMsg = removeCallbackDuplicate(msg);
+
+ if (newMsg.empty())
+ MessageQueue::put_impl(value);
+ else
+ {
+ MessageQueue::put_impl(Payload(newMsg.data(), newMsg.data() + newMsg.size()));
+ }
- MessageQueue::put_impl(value);
return;
}
@@ -143,45 +149,164 @@ std::string extractUnoCommand(const std::string& command)
return command;
}
+/// Extract rectangle from the invalidation callback
+bool extractRectangle(const std::vector<std::string>& tokens, int& x, int& y, int& w, int& h, int& part)
+{
+ x = 0;
+ y = 0;
+ w = INT_MAX;
+ h = INT_MAX;
+ part = 0;
+
+ if (tokens.size() < 4)
+ return false;
+
+ if (tokens[3] == "EMPTY,")
+ {
+ part = std::atoi(tokens[4].c_str());
+ return true;
+ }
+
+ if (tokens.size() < 8)
+ return false;
+
+ x = std::atoi(tokens[3].c_str());
+ y = std::atoi(tokens[4].c_str());
+ w = std::atoi(tokens[5].c_str());
+ h = std::atoi(tokens[6].c_str());
+ part = std::atoi(tokens[7].c_str());
+
+ return true;
+}
+
}
-void TileQueue::removeCallbackDuplicate(const std::string& callbackMsg)
+std::string TileQueue::removeCallbackDuplicate(const std::string& callbackMsg)
{
assert(LOOLProtocol::matchPrefix("callback", callbackMsg, /*ignoreWhitespace*/ true));
std::vector<std::string> tokens = LOOLProtocol::tokenize(callbackMsg);
if (tokens.size() < 3)
- return;
+ return std::string();
// the message is "callback <view> <id> ..."
const std::string& callbackType = tokens[2];
if (callbackType == "0") // invalidation
{
- // TODO later add a more advanced merging of invalidates (like two
- // close ones merge into a bigger one); but for the moment remove just
- // the plain duplicates
- for (size_t i = 0; i < _queue.size(); ++i)
+ int msgX, msgY, msgW, msgH, msgPart;
+
+ if (!extractRectangle(tokens, msgX, msgY, msgW, msgH, msgPart))
+ return std::string();
+
+ bool performedMerge = false;
+
+ // we always travel the entire queue
+ size_t i = 0;
+ while (i < _queue.size())
{
auto& it = _queue[i];
- if (callbackMsg.size() == it.size() && LOOLProtocol::matchPrefix(callbackMsg, it))
+ std::vector<std::string> queuedTokens = LOOLProtocol::tokenize(it.data(), it.size());
+ if (queuedTokens.size() < 3)
+ {
+ ++i;
+ continue;
+ }
+
+ // not a invalidation callback
+ if (queuedTokens[0] != tokens[0] || queuedTokens[1] != tokens[1] || queuedTokens[2] != tokens[2])
+ {
+ ++i;
+ continue;
+ }
+
+ int queuedX, queuedY, queuedW, queuedH, queuedPart;
+
+ if (!extractRectangle(queuedTokens, queuedX, queuedY, queuedW, queuedH, queuedPart))
+ {
+ ++i;
+ continue;
+ }
+
+ if (msgPart != queuedPart)
{
- LOG_TRC("Remove duplicate invalidation callback: " << std::string(it.data(), it.size()) << " -> " << callbackMsg);
+ ++i;
+ continue;
+ }
+
+ // the invalidation in the queue is fully covered by the message,
+ // just remove it
+ if (msgX <= queuedX && queuedX + queuedW <= msgX + msgW && msgY <= queuedY && queuedY + queuedH <= msgY + msgH)
+ {
+ LOG_TRC("Removing smaller invalidation: " << std::string(it.data(), it.size()) << " -> " <<
+ tokens[0] << " " << tokens[1] << " " << tokens[2] << " " << msgX << " " << msgY << " " << msgW << " " << msgH << " " << msgPart);
+
+ // remove from the queue
_queue.erase(_queue.begin() + i);
- break;
+ continue;
+ }
+
+ // the invalidation just intersects, join those (if the result is
+ // small)
+ if (TileDesc::rectanglesIntersect(msgX, msgY, msgW, msgH, queuedX, queuedY, queuedW, queuedH))
+ {
+ int joinX = std::min(msgX, queuedX);
+ int joinY = std::min(msgY, queuedY);
+ int joinW = std::max(msgX + msgW, queuedX + queuedW) - joinX;
+ int joinH = std::max(msgY + msgH, queuedY + queuedH) - joinY;
+
+ const int reasonableSizeX = 4*3840; // 4x tile at 100% zoom
+ const int reasonableSizeY = 2*3840; // 2x tile at 100% zoom
+ if (joinW > reasonableSizeX || joinH > reasonableSizeY)
+ {
+ ++i;
+ continue;
+ }
+
+ LOG_TRC("Merging invalidations: " << std::string(it.data(), it.size()) << " and " <<
+ tokens[0] << " " << tokens[1] << " " << tokens[2] << " " << msgX << " " << msgY << " " << msgW << " " << msgH << " " << msgPart << " -> " <<
+ tokens[0] << " " << tokens[1] << " " << tokens[2] << " " << joinX << " " << joinY << " " << joinW << " " << joinH << " " << msgPart);
+
+ msgX = joinX;
+ msgY = joinY;
+ msgW = joinW;
+ msgH = joinH;
+ performedMerge = true;
+
+ // remove from the queue
+ _queue.erase(_queue.begin() + i);
+ continue;
}
+
+ ++i;
+ }
+
+ if (performedMerge)
+ {
+ size_t pre = tokens[0].size() + tokens[1].size() + tokens[2].size() + 3;
+ size_t post = pre + tokens[3].size() + tokens[4].size() + tokens[5].size() + tokens[6].size() + 4;
+
+ std::string result = callbackMsg.substr(0, pre) +
+ std::to_string(msgX) + ", " +
+ std::to_string(msgY) + ", " +
+ std::to_string(msgW) + ", " +
+ std::to_string(msgH) + ", " + callbackMsg.substr(post);
+
+ LOG_TRC("Merge result: " << result);
+
+ return result;
}
}
else if (callbackType == "8") // state changed
{
if (tokens.size() < 4)
- return;
+ return std::string();
std::string unoCommand = extractUnoCommand(tokens[3]);
- if (unoCommand.length() == 0)
- return;
+ if (unoCommand.empty())
+ return std::string();
// remove obsolete states of the same .uno: command
for (size_t i = 0; i < _queue.size(); ++i)
@@ -192,20 +317,20 @@ void TileQueue::removeCallbackDuplicate(const std::string& callbackMsg)
if (queuedTokens.size() < 4)
continue;
- if (queuedTokens[0] == tokens[0] && queuedTokens[1] == tokens[1] && queuedTokens[2] == tokens[2])
- {
- // callback, the same target, state changed; now check it's
- // the same .uno: command
- std::string queuedUnoCommand = extractUnoCommand(queuedTokens[3]);
- if (queuedUnoCommand.length() == 0)
- continue;
+ if (queuedTokens[0] != tokens[0] || queuedTokens[1] != tokens[1] || queuedTokens[2] != tokens[2])
+ continue;
- if (unoCommand == queuedUnoCommand)
- {
- LOG_TRC("Remove obsolete uno command: " << std::string(it.data(), it.size()) << " -> " << callbackMsg);
- _queue.erase(_queue.begin() + i);
- break;
- }
+ // callback, the same target, state changed; now check it's
+ // the same .uno: command
+ std::string queuedUnoCommand = extractUnoCommand(queuedTokens[3]);
+ if (queuedUnoCommand.empty())
+ continue;
+
+ if (unoCommand == queuedUnoCommand)
+ {
+ LOG_TRC("Remove obsolete uno command: " << std::string(it.data(), it.size()) << " -> " << callbackMsg);
+ _queue.erase(_queue.begin() + i);
+ break;
}
}
}
@@ -258,6 +383,8 @@ void TileQueue::removeCallbackDuplicate(const std::string& callbackMsg)
}
}
}
+
+ return std::string();
}
int TileQueue::priority(const std::string& tileMsg)
diff --git a/common/MessageQueue.hpp b/common/MessageQueue.hpp
index 92d5e16..1138259 100644
--- a/common/MessageQueue.hpp
+++ b/common/MessageQueue.hpp
@@ -208,7 +208,9 @@ private:
///
/// This removes also callbacks that are made invalid by the current
/// message, like the new cursor position invalidates the old one etc.
- void removeCallbackDuplicate(const std::string& callbackMsg);
+ ///
+ /// @return New message to put into the queue. If empty, use what was in callbackMsg.
+ std::string removeCallbackDuplicate(const std::string& callbackMsg);
/// Find the index of the first non-preview entry.
/// When preferTiles is false, it'll return index of
diff --git a/test/TileQueueTests.cpp b/test/TileQueueTests.cpp
index 2c33c08..884ee51 100644
--- a/test/TileQueueTests.cpp
+++ b/test/TileQueueTests.cpp
@@ -49,6 +49,7 @@ class TileQueueTests : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST(testSenderQueue);
CPPUNIT_TEST(testSenderQueueTileDeduplication);
CPPUNIT_TEST(testInvalidateViewCursorDeduplication);
+ CPPUNIT_TEST(testCallbackInvalidation);
CPPUNIT_TEST_SUITE_END();
@@ -60,6 +61,7 @@ class TileQueueTests : public CPPUNIT_NS::TestFixture
void testSenderQueue();
void testSenderQueueTileDeduplication();
void testInvalidateViewCursorDeduplication();
+ void testCallbackInvalidation();
};
void TileQueueTests::testTileQueuePriority()
@@ -415,6 +417,33 @@ void TileQueueTests::testInvalidateViewCursorDeduplication()
CPPUNIT_ASSERT_EQUAL(0UL, queue.size());
}
+void TileQueueTests::testCallbackInvalidation()
+{
+ TileQueue queue;
+
+ // join tiles
+ queue.put("callback all 0 284, 1418, 11105, 275, 0");
+ queue.put("callback all 0 4299, 1418, 7090, 275, 0");
+
+ CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(queue._queue.size()));
+
+ CPPUNIT_ASSERT_EQUAL(std::string("callback all 0 284, 1418, 11105, 275, 0"), payloadAsString(queue.get()));
+
+ // invalidate everywhing with EMPTY, but keep the different part intact
+ queue.put("callback all 0 284, 1418, 11105, 275, 0");
+ queue.put("callback all 0 4299, 1418, 7090, 275, 1");
+ queue.put("callback all 0 4299, 10418, 7090, 275, 0");
+ queue.put("callback all 0 4299, 20418, 7090, 275, 0");
+
+ CPPUNIT_ASSERT_EQUAL(4, static_cast<int>(queue._queue.size()));
+
+ queue.put("callback all 0 EMPTY, 0");
+
+ CPPUNIT_ASSERT_EQUAL(2, static_cast<int>(queue._queue.size()));
+ CPPUNIT_ASSERT_EQUAL(std::string("callback all 0 4299, 1418, 7090, 275, 1"), payloadAsString(queue.get()));
+ CPPUNIT_ASSERT_EQUAL(std::string("callback all 0 EMPTY, 0"), payloadAsString(queue.get()));
+}
+
CPPUNIT_TEST_SUITE_REGISTRATION(TileQueueTests);
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/test/WhiteBoxTests.cpp b/test/WhiteBoxTests.cpp
index 703a9b2..689f6c4 100644
--- a/test/WhiteBoxTests.cpp
+++ b/test/WhiteBoxTests.cpp
@@ -16,6 +16,7 @@
#include <Kit.hpp>
#include <MessageQueue.hpp>
#include <Protocol.hpp>
+#include <TileDesc.hpp>
#include <Util.hpp>
/// WhiteBox unit-tests.
@@ -29,6 +30,7 @@ class WhiteBoxTests : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST(testRegexListMatcher);
CPPUNIT_TEST(testRegexListMatcher_Init);
CPPUNIT_TEST(testEmptyCellCursor);
+ CPPUNIT_TEST(testRectanglesIntersect);
CPPUNIT_TEST_SUITE_END();
@@ -38,6 +40,7 @@ class WhiteBoxTests : public CPPUNIT_NS::TestFixture
void testRegexListMatcher();
void testRegexListMatcher_Init();
void testEmptyCellCursor();
+ void testRectanglesIntersect();
};
void WhiteBoxTests::testLOOLProtocolFunctions()
@@ -359,6 +362,31 @@ void WhiteBoxTests::testEmptyCellCursor()
documentViewCallback(LOK_CALLBACK_CELL_CURSOR, "EMPTY", &callbackDescriptor);
}
+void WhiteBoxTests::testRectanglesIntersect()
+{
+ // these intersect
+ CPPUNIT_ASSERT(TileDesc::rectanglesIntersect(1000, 1000, 2000, 1000,
+ 2000, 1000, 2000, 1000));
+ CPPUNIT_ASSERT(TileDesc::rectanglesIntersect(2000, 1000, 2000, 1000,
+ 1000, 1000, 2000, 1000));
+
+ CPPUNIT_ASSERT(TileDesc::rectanglesIntersect(1000, 1000, 2000, 1000,
+ 3000, 2000, 1000, 1000));
+ CPPUNIT_ASSERT(TileDesc::rectanglesIntersect(3000, 2000, 1000, 1000,
+ 1000, 1000, 2000, 1000));
+
+ // these don't
+ CPPUNIT_ASSERT(!TileDesc::rectanglesIntersect(1000, 1000, 2000, 1000,
+ 2000, 3000, 2000, 1000));
+ CPPUNIT_ASSERT(!TileDesc::rectanglesIntersect(2000, 3000, 2000, 1000,
+ 1000, 1000, 2000, 1000));
+
+ CPPUNIT_ASSERT(!TileDesc::rectanglesIntersect(1000, 1000, 2000, 1000,
+ 2000, 3000, 1000, 1000));
+ CPPUNIT_ASSERT(!TileDesc::rectanglesIntersect(2000, 3000, 1000, 1000,
+ 1000, 1000, 2000, 1000));
+}
+
CPPUNIT_TEST_SUITE_REGISTRATION(WhiteBoxTests);
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/TileDesc.hpp b/wsd/TileDesc.hpp
index e213c9a..0004c1e 100644
--- a/wsd/TileDesc.hpp
+++ b/wsd/TileDesc.hpp
@@ -84,12 +84,17 @@ public:
_broadcast == other._broadcast;
}
+ static bool rectanglesIntersect(int x1, int y1, int w1, int h1, int x2, int y2, int w2, int h2)
+ {
+ return x1 + w1 >= x2 &&
+ x1 <= x2 + w2 &&
+ y1 + h1 >= y2 &&
+ y1 <= y2 + h2;
+ }
+
bool intersectsWithRect(int x, int y, int w, int h) const
{
- return x + w >= getTilePosX() &&
- x <= getTilePosX() + getTileWidth() &&
- y + h >= getTilePosY() &&
- y <= getTilePosY() + getTileHeight();
+ return rectanglesIntersect(getTilePosX(), getTilePosY(), getTileWidth(), getTileHeight(), x, y, w, h);
}
bool intersects(const TileDesc& other) const
commit f77253f4f854cc9dc6b2c5fbaaf93e5be4b44808
Author: Jan Holesovsky <kendy at collabora.com>
Date: Wed Jan 25 12:21:38 2017 +0100
Deduplicate the .uno: command state changes too.
Change-Id: Iaf9204d39d90cb9289d279e35a4609fa68c2cce8
diff --git a/common/MessageQueue.cpp b/common/MessageQueue.cpp
index 72d302f..bc61341 100644
--- a/common/MessageQueue.cpp
+++ b/common/MessageQueue.cpp
@@ -130,6 +130,19 @@ std::string extractViewId(const std::string& origMsg, const std::vector<std::str
return json->get("viewId").toString();
}
+/// Extract the .uno: command ID from the potential command.
+std::string extractUnoCommand(const std::string& command)
+{
+ if (!LOOLProtocol::matchPrefix(".uno:", command))
+ return std::string();
+
+ size_t equalPos = command.find('=');
+ if (equalPos != std::string::npos)
+ return command.substr(0, equalPos);
+
+ return command;
+}
+
}
void TileQueue::removeCallbackDuplicate(const std::string& callbackMsg)
@@ -146,9 +159,9 @@ void TileQueue::removeCallbackDuplicate(const std::string& callbackMsg)
if (callbackType == "0") // invalidation
{
- // TODO later we can consider some more advanced merging of
- // invalidates (like two close ones merge into a bigger one); but for
- // the moment remove just the plain duplicates
+ // TODO later add a more advanced merging of invalidates (like two
+ // close ones merge into a bigger one); but for the moment remove just
+ // the plain duplicates
for (size_t i = 0; i < _queue.size(); ++i)
{
auto& it = _queue[i];
@@ -161,6 +174,41 @@ void TileQueue::removeCallbackDuplicate(const std::string& callbackMsg)
}
}
}
+ else if (callbackType == "8") // state changed
+ {
+ if (tokens.size() < 4)
+ return;
+
+ std::string unoCommand = extractUnoCommand(tokens[3]);
+ if (unoCommand.length() == 0)
+ return;
+
+ // remove obsolete states of the same .uno: command
+ for (size_t i = 0; i < _queue.size(); ++i)
+ {
+ auto& it = _queue[i];
+
+ std::vector<std::string> queuedTokens = LOOLProtocol::tokenize(it.data(), it.size());
+ if (queuedTokens.size() < 4)
+ continue;
+
+ if (queuedTokens[0] == tokens[0] && queuedTokens[1] == tokens[1] && queuedTokens[2] == tokens[2])
+ {
+ // callback, the same target, state changed; now check it's
+ // the same .uno: command
+ std::string queuedUnoCommand = extractUnoCommand(queuedTokens[3]);
+ if (queuedUnoCommand.length() == 0)
+ continue;
+
+ if (unoCommand == queuedUnoCommand)
+ {
+ LOG_TRC("Remove obsolete uno command: " << std::string(it.data(), it.size()) << " -> " << callbackMsg);
+ _queue.erase(_queue.begin() + i);
+ break;
+ }
+ }
+ }
+ }
else if (callbackType == "1" || // the cursor has moved
callbackType == "5" || // the cursor visibility has changed
callbackType == "17" || // the cell cursor has moved
commit 68e597b5127e1f097b9c9d22ed7252ac897c0132
Author: Jan Holesovsky <kendy at collabora.com>
Date: Tue Jan 24 14:53:17 2017 +0100
Merge the LOK_CALLBACK_INVALIDATE_VIEW_CURSOR messages together.
And LOK_CALLBACK_CELL_VIEW_CURSOR too - though that ones are not that
expensive.
Change-Id: I5f116c66b1ce4426694d5d58851e104f9061bf53
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 6f4e89b..30966f9 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -832,6 +832,9 @@ public:
"] [" << LOKitHelper::kitCallbackTypeToString(type) <<
"] [" << payload << "].");
+ // when we examine the content of the JSON
+ std::string targetViewId;
+
if (type == LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR ||
type == LOK_CALLBACK_CELL_CURSOR)
{
@@ -853,7 +856,7 @@ public:
Poco::JSON::Parser parser;
const auto result = parser.parse(payload);
const auto& command = result.extract<Poco::JSON::Object::Ptr>();
- auto viewId = command->get("viewId").toString();
+ targetViewId = command->get("viewId").toString();
auto part = command->get("part").toString();
auto text = command->get("rectangle").toString();
Poco::StringTokenizer tokens(text, ",", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
@@ -865,12 +868,23 @@ public:
auto cursorWidth = std::stoi(tokens[2]);
auto cursorHeight = std::stoi(tokens[3]);
- tileQueue->updateCursorPosition(std::stoi(viewId), std::stoi(part), cursorX, cursorY, cursorWidth, cursorHeight);
+ tileQueue->updateCursorPosition(std::stoi(targetViewId), std::stoi(part), cursorX, cursorY, cursorWidth, cursorHeight);
}
}
+ // merge various callback types together if possible
if (type == LOK_CALLBACK_INVALIDATE_TILES)
- tileQueue->put("callback -1 " + std::to_string(type) + ' ' + payload); // no point in handling invalidations per-view
+ {
+ // no point in handling invalidations per-view, all views have to
+ // be in sync
+ tileQueue->put("callback all " + std::to_string(type) + ' ' + payload);
+ }
+ else if (type == LOK_CALLBACK_INVALIDATE_VIEW_CURSOR ||
+ type == LOK_CALLBACK_CELL_VIEW_CURSOR)
+ {
+ // these should go to all views but the one that that triggered it
+ tileQueue->put("callback except-" + targetViewId + ' ' + std::to_string(type) + ' ' + payload);
+ }
else
tileQueue->put("callback " + std::to_string(descriptor->ViewId) + ' ' + std::to_string(type) + ' ' + payload);
}
@@ -881,7 +895,7 @@ private:
void broadcastCallbackToClients(const int type, const std::string& payload)
{
// "-1" means broadcast
- _tileQueue->put("callback -1 " + std::to_string(type) + ' ' + payload);
+ _tileQueue->put("callback all " + std::to_string(type) + ' ' + payload);
}
/// Load a document (or view) and register callbacks.
@@ -1387,7 +1401,21 @@ private:
{
if (tokens.size() >= 3)
{
- int viewId = std::stoi(tokens[1]); // -1 means broadcast
+ bool broadcast = false;
+ int viewId = -1;
+ int exceptViewId = -1;
+
+ const std::string& target = tokens[1];
+ if (target == "all")
+ broadcast = true;
+ else if (LOOLProtocol::matchPrefix("except-", target))
+ {
+ exceptViewId = std::stoi(target.substr(7));
+ broadcast = true;
+ }
+ else
+ viewId = std::stoi(target);
+
int type = std::stoi(tokens[2]);
// payload is the rest of the message
@@ -1400,7 +1428,7 @@ private:
for (auto& it : _sessions)
{
auto session = it.second;
- if (session && ((session->getViewId() == viewId) || (viewId == -1)))
+ if (session && ((broadcast && (session->getViewId() != exceptViewId)) || (!broadcast && (session->getViewId() == viewId))))
{
if (!it.second->isCloseFrame())
{
@@ -1414,8 +1442,7 @@ private:
"] payload [" << payload << "].");
}
- // finish if we are not broadcasting
- if (viewId != -1)
+ if (!broadcast)
break;
}
}
commit e1b40888103078d621dae3d1fad775c6e38eb575
Author: Jan Holesovsky <kendy at collabora.com>
Date: Tue Jan 24 12:41:55 2017 +0100
Make the broadcasting actually work.
Change-Id: I35ba20a45fdfe9fc0182c30648693d76a3e10f8a
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index f50fefb..6f4e89b 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -1414,7 +1414,9 @@ private:
"] payload [" << payload << "].");
}
- break;
+ // finish if we are not broadcasting
+ if (viewId != -1)
+ break;
}
}
commit 1b9e34a3f9d8a345bc1639e3f4f61697f81c7933
Author: Jan Holesovsky <kendy at collabora.com>
Date: Tue Jan 24 12:40:30 2017 +0100
Squash the invalidation messages coming from various view into one.
Change-Id: I65f7e4eb0b82a8c76eef372548ad3298ac70bdd7
diff --git a/common/MessageQueue.cpp b/common/MessageQueue.cpp
index 1c019da..72d302f 100644
--- a/common/MessageQueue.cpp
+++ b/common/MessageQueue.cpp
@@ -141,12 +141,27 @@ void TileQueue::removeCallbackDuplicate(const std::string& callbackMsg)
if (tokens.size() < 3)
return;
- // TODO probably we could deduplicate the invalidation callbacks (later
- // one wins) too?
-
// the message is "callback <view> <id> ..."
const std::string& callbackType = tokens[2];
- if (callbackType == "1" || // the cursor has moved
+
+ if (callbackType == "0") // invalidation
+ {
+ // TODO later we can consider some more advanced merging of
+ // invalidates (like two close ones merge into a bigger one); but for
+ // the moment remove just the plain duplicates
+ for (size_t i = 0; i < _queue.size(); ++i)
+ {
+ auto& it = _queue[i];
+
+ if (callbackMsg.size() == it.size() && LOOLProtocol::matchPrefix(callbackMsg, it))
+ {
+ LOG_TRC("Remove duplicate invalidation callback: " << std::string(it.data(), it.size()) << " -> " << callbackMsg);
+ _queue.erase(_queue.begin() + i);
+ break;
+ }
+ }
+ }
+ else if (callbackType == "1" || // the cursor has moved
callbackType == "5" || // the cursor visibility has changed
callbackType == "17" || // the cell cursor has moved
callbackType == "24" || // the view cursor has moved
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 4d862de..f50fefb 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -869,7 +869,10 @@ public:
}
}
- tileQueue->put("callback " + std::to_string(descriptor->ViewId) + ' ' + std::to_string(type) + ' ' + payload);
+ if (type == LOK_CALLBACK_INVALIDATE_TILES)
+ tileQueue->put("callback -1 " + std::to_string(type) + ' ' + payload); // no point in handling invalidations per-view
+ else
+ tileQueue->put("callback " + std::to_string(descriptor->ViewId) + ' ' + std::to_string(type) + ' ' + payload);
}
private:
commit 60a3d717cb8ddef0ec20edf453c19d7a82beb1f1
Author: Jan Holesovsky <kendy at collabora.com>
Date: Fri Jan 20 15:56:47 2017 +0100
Remove obsolete cursor positions from the message queue.
Change-Id: Ie54aa4a475f0a55baaf6a40a99ed341c8f941a04
diff --git a/common/MessageQueue.cpp b/common/MessageQueue.cpp
index 4d26a5a..1c019da 100644
--- a/common/MessageQueue.cpp
+++ b/common/MessageQueue.cpp
@@ -11,6 +11,9 @@
#include <algorithm>
+#include <Poco/JSON/JSON.h>
+#include <Poco/JSON/Object.h>
+#include <Poco/JSON/Parser.h>
#include <Poco/StringTokenizer.h>
#include <Protocol.hpp>
@@ -62,7 +65,7 @@ void TileQueue::put_impl(const Payload& value)
{
const std::string newMsg = tile.serialize("tile");
- removeDuplicate(newMsg);
+ removeTileDuplicate(newMsg);
MessageQueue::put_impl(Payload(newMsg.data(), newMsg.data() + newMsg.size()));
}
@@ -70,19 +73,23 @@ void TileQueue::put_impl(const Payload& value)
}
else if (firstToken == "tile")
{
- removeDuplicate(msg);
+ removeTileDuplicate(msg);
MessageQueue::put_impl(value);
return;
}
+ else if (firstToken == "callback")
+ {
+ removeCallbackDuplicate(msg);
- // TODO probably we could deduplacite the invalidation callbacks (later
- // one wins) the same way as we do for the tiles - to be tested.
+ MessageQueue::put_impl(value);
+ return;
+ }
MessageQueue::put_impl(value);
}
-void TileQueue::removeDuplicate(const std::string& tileMsg)
+void TileQueue::removeTileDuplicate(const std::string& tileMsg)
{
assert(LOOLProtocol::matchPrefix("tile", tileMsg, /*ignoreWhitespace*/ true));
@@ -102,13 +109,94 @@ void TileQueue::removeDuplicate(const std::string& tileMsg)
if (it.size() > newMsgPos &&
strncmp(tileMsg.data(), it.data(), newMsgPos) == 0)
{
- LOG_TRC("Remove duplicate message: " << std::string(it.data(), it.size()) << " -> " << tileMsg);
+ LOG_TRC("Remove duplicate tile request: " << std::string(it.data(), it.size()) << " -> " << tileMsg);
_queue.erase(_queue.begin() + i);
break;
}
}
}
+namespace {
+
+/// Read the viewId from the tokens.
+std::string extractViewId(const std::string& origMsg, const std::vector<std::string> tokens)
+{
+ size_t nonJson = tokens[0].size() + tokens[1].size() + tokens[2].size() + 3; // including spaces
+ std::string jsonString(origMsg.data() + nonJson, origMsg.size() - nonJson);
+
+ Poco::JSON::Parser parser;
+ const auto result = parser.parse(jsonString);
+ const auto& json = result.extract<Poco::JSON::Object::Ptr>();
+ return json->get("viewId").toString();
+}
+
+}
+
+void TileQueue::removeCallbackDuplicate(const std::string& callbackMsg)
+{
+ assert(LOOLProtocol::matchPrefix("callback", callbackMsg, /*ignoreWhitespace*/ true));
+
+ std::vector<std::string> tokens = LOOLProtocol::tokenize(callbackMsg);
+
+ if (tokens.size() < 3)
+ return;
+
+ // TODO probably we could deduplicate the invalidation callbacks (later
+ // one wins) too?
+
+ // the message is "callback <view> <id> ..."
+ const std::string& callbackType = tokens[2];
+ if (callbackType == "1" || // the cursor has moved
+ callbackType == "5" || // the cursor visibility has changed
+ callbackType == "17" || // the cell cursor has moved
+ callbackType == "24" || // the view cursor has moved
+ callbackType == "26" || // the view cell cursor has moved
+ callbackType == "28") // the view cursor visibility has changed
+ {
+ bool isViewCallback = (callbackType == "24" || callbackType == "26" || callbackType == "28");
+
+ std::string viewId;
+ if (isViewCallback)
+ {
+ viewId = extractViewId(callbackMsg, tokens);
+ }
+
+ for (size_t i = 0; i < _queue.size(); ++i)
+ {
+ auto& it = _queue[i];
+
+ // skip non-callbacks quickly
+ if (!LOOLProtocol::matchPrefix("callback", it))
+ continue;
+
+ std::vector<std::string> queuedTokens = LOOLProtocol::tokenize(it.data(), it.size());
+ if (queuedTokens.size() < 3)
+ continue;
+
+ if (!isViewCallback && (queuedTokens[1] == tokens[1] && queuedTokens[2] == tokens[2]))
+ {
+ LOG_TRC("Remove obsolete callback: " << std::string(it.data(), it.size()) << " -> " << callbackMsg);
+ _queue.erase(_queue.begin() + i);
+ break;
+ }
+ else if (isViewCallback && (queuedTokens[1] == tokens[1] && queuedTokens[2] == tokens[2]))
+ {
+ // we additionally need to ensure that the payload is about
+ // the same viewid (otherwise we'd merge them all views into
+ // one)
+ const std::string queuedViewId = extractViewId(std::string(it.data(), it.size()), queuedTokens);
+
+ if (viewId == queuedViewId)
+ {
+ LOG_TRC("Remove obsolete view callback: " << std::string(it.data(), it.size()) << " -> " << callbackMsg);
+ _queue.erase(_queue.begin() + i);
+ break;
+ }
+ }
+ }
+ }
+}
+
int TileQueue::priority(const std::string& tileMsg)
{
auto tile = TileDesc::parse(tileMsg); //FIXME: Expensive, avoid.
diff --git a/common/MessageQueue.hpp b/common/MessageQueue.hpp
index 34cbe8a..92d5e16 100644
--- a/common/MessageQueue.hpp
+++ b/common/MessageQueue.hpp
@@ -202,7 +202,13 @@ protected:
private:
/// Search the queue for a duplicate tile and remove it (if present).
- void removeDuplicate(const std::string& tileMsg);
+ void removeTileDuplicate(const std::string& tileMsg);
+
+ /// Search the queue for a duplicate callback and remove it (if present).
+ ///
+ /// This removes also callbacks that are made invalid by the current
+ /// message, like the new cursor position invalidates the old one etc.
+ void removeCallbackDuplicate(const std::string& callbackMsg);
/// Find the index of the first non-preview entry.
/// When preferTiles is false, it'll return index of
commit b5128d061d1b49b831b6c3516ce30a87e65e9c5a
Author: Jan Holesovsky <kendy at collabora.com>
Date: Mon Jan 23 17:22:39 2017 +0100
Revert "wsd: batched user-input processing"
This reverts commit 27e1428088905e0a48b59038f52f3469c1a8da6e.
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index dbe25a1..4d862de 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -1239,7 +1239,7 @@ private:
return _loKitDocument;
}
- size_t forwardToChild(const std::string& prefix, const std::vector<char>& payload, size_t batched)
+ bool forwardToChild(const std::string& prefix, const std::vector<char>& payload)
{
assert(payload.size() > prefix.size());
@@ -1281,46 +1281,14 @@ private:
// No longer needed, and allow session dtor to take it.
lock.unlock();
session.reset();
- return batched;
+ return true;
}
// No longer needed, and allow the handler to take it.
lock.unlock();
if (session)
{
- static std::string last;
-
- const auto current = LOOLProtocol::getFirstToken(data, size);
- if (batched)
- {
- if (current != last)
- {
- LOG_TRC("Ending batch of " << batched << " messages.");
- std::unique_lock<std::mutex> lockDocument(_documentMutex);
- batched = 0;
- getLOKitDocument()->endBatch();
- }
- }
-
- if (!batched)
- {
- std::unique_lock<std::mutex> lockDocument(_documentMutex);
- if (_loKitDocument)
- {
- ++batched;
- last = current;
- LOG_TRC("Beginning batch [" << last<< "].");
- getLOKitDocument()->beginBatch();
- }
- }
- else
- {
- ++batched;
- LOG_TRC("Processing batch item #" << batched << " [" << current << "].");
- }
-
- session->handleInput(data, size);
- return batched;
+ return session->handleInput(data, size);
}
}
@@ -1332,7 +1300,7 @@ private:
LOG_ERR("Failed to parse prefix of forward-to-child message: " << prefix);
}
- return batched;
+ return false;
}
std::string makeRenderParams(const std::string& userName)
@@ -1379,20 +1347,10 @@ private:
LOG_DBG("Thread started.");
- size_t batched = 0;
try
{
while (!_stop && !TerminationFlag)
{
- // End if we have no more.
- if (batched)
- {
- LOG_TRC("Ending batch of " << batched << " messages.");
- std::unique_lock<std::mutex> lock(_documentMutex);
- batched = 0;
- getLOKitDocument()->endBatch();
- }
-
const TileQueue::Payload input = _tileQueue->get();
LOG_TRC("Kit Recv " << LOOLProtocol::getAbbreviatedMessage(input));
@@ -1410,15 +1368,6 @@ private:
break;
}
- // Stop batching if not a child message.
- if (batched && tokens[0].compare(0, 5, "child"))
- {
- LOG_TRC("Ending batch of " << batched << " messages.");
- std::unique_lock<std::mutex> lock(_documentMutex);
- batched = 0;
- getLOKitDocument()->endBatch();
- }
-
if (tokens[0] == "tile")
{
renderTile(tokens, _ws);
@@ -1429,7 +1378,7 @@ private:
}
else if (LOOLProtocol::getFirstToken(tokens[0], '-') == "child")
{
- batched = forwardToChild(tokens[0], input, batched);
+ forwardToChild(tokens[0], input);
}
else if (tokens[0] == "callback")
{
commit febbb570c6b6886656ca028107f72f030d0d21c7
Author: Jan Holesovsky <kendy at collabora.com>
Date: Mon Jan 23 17:22:15 2017 +0100
Revert "wsd: cap batches by duration"
This reverts commit 0fe580d9ab3abe198e3143d30c83f1cffb589832.
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 3f14c19..dbe25a1 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -1379,50 +1379,22 @@ private:
LOG_DBG("Thread started.");
- static const unsigned MaxBatchDurationMs = 125;
size_t batched = 0;
- std::chrono::steady_clock::time_point timeBeginBatch;
try
{
while (!_stop && !TerminationFlag)
{
- unsigned timeoutMs = 0;
+ // End if we have no more.
if (batched)
{
- // Cap the wait so we eventually process the batch.
- const auto duration = std::chrono::steady_clock::now() - timeBeginBatch;
- const auto durationMs = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count();
- if (durationMs >= MaxBatchDurationMs)
- {
- LOG_TRC("Ending batch of " << batched << " messages.");
- std::unique_lock<std::mutex> lock(_documentMutex);
- batched = 0;
- getLOKitDocument()->endBatch();
- timeoutMs = 0;
- }
- else
- {
- timeoutMs = MaxBatchDurationMs - durationMs;
- }
+ LOG_TRC("Ending batch of " << batched << " messages.");
+ std::unique_lock<std::mutex> lock(_documentMutex);
+ batched = 0;
+ getLOKitDocument()->endBatch();
}
- LOG_TRC("Kit dequeue with max timeout of " << timeoutMs << " ms. Batched: " << batched);
- const TileQueue::Payload input = _tileQueue->get(timeoutMs);
- LOG_TRC("Kit dequeued " << LOOLProtocol::getAbbreviatedMessage(input));
- if (input.empty())
- {
- // End if we have no more.
- if (batched)
- {
- LOG_TRC("Ending batch of " << batched << " messages.");
- std::unique_lock<std::mutex> lock(_documentMutex);
- batched = 0;
- getLOKitDocument()->endBatch();
- }
-
- // Nothing to process.
- continue;
- }
+ const TileQueue::Payload input = _tileQueue->get();
+ LOG_TRC("Kit Recv " << LOOLProtocol::getAbbreviatedMessage(input));
if (_stop || TerminationFlag)
{
@@ -1458,11 +1430,6 @@ private:
else if (LOOLProtocol::getFirstToken(tokens[0], '-') == "child")
{
batched = forwardToChild(tokens[0], input, batched);
- if (batched == 1)
- {
- // New batch started.
- timeBeginBatch = std::chrono::steady_clock::now();
- }
}
else if (tokens[0] == "callback")
{
More information about the Libreoffice-commits
mailing list