[Libreoffice-commits] online.git: Branch 'feature/deduplication' - common/MessageQueue.cpp common/MessageQueue.hpp test/TileQueueTests.cpp test/WhiteBoxTests.cpp wsd/TileDesc.hpp
Jan Holesovsky
kendy at collabora.com
Wed Jan 25 17:23:47 UTC 2017
common/MessageQueue.cpp | 181 ++++++++++++++++++++++++++++++++++++++++--------
common/MessageQueue.hpp | 4 -
test/TileQueueTests.cpp | 29 +++++++
test/WhiteBoxTests.cpp | 28 +++++++
wsd/TileDesc.hpp | 13 ++-
5 files changed, 223 insertions(+), 32 deletions(-)
New commits:
commit 37daf37cf3af7346336b1d6dba3017b2438d52bd
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
More information about the Libreoffice-commits
mailing list