[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-4-0' - common/Png.hpp kit/Kit.cpp test/helpers.hpp test/TileCacheTests.cpp
Ashod Nakashian (via logerrit)
logerrit at kemper.freedesktop.org
Tue Oct 22 17:02:03 UTC 2019
common/Png.hpp | 2
kit/Kit.cpp | 14 ++---
test/TileCacheTests.cpp | 129 ++++++++++++++++++++----------------------------
test/helpers.hpp | 20 +++++++
4 files changed, 83 insertions(+), 82 deletions(-)
New commits:
commit 9603597fd1aaecb27893792cfd2d243e450b58b8
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
AuthorDate: Sat Oct 19 12:33:22 2019 -0400
Commit: Andras Timar <andras.timar at collabora.com>
CommitDate: Tue Oct 22 19:01:43 2019 +0200
test: improve TileCacheTests
Sometimes core renderes with sub-pixel differences
(the crosshair at the corners of the Writer pages
show line anti-aliasing differences). This causes
failure of the tests that count the tile deduplication.
We now tolerate when we get an unchanged tile twice,
assuming it was due to such a rendering difference,
but we re-trigger another change and this time we
don't expect any extra tiles, no more than two
variations of the anti-aliased crosshair was
observed.
We also move some duplicate code into utility
functions to improve readability and reuse.
Change-Id: I1a66732dd3443bfbd770d8dc65721571dfa08615
Reviewed-on: https://gerrit.libreoffice.org/81196
Reviewed-by: Andras Timar <andras.timar at collabora.com>
Tested-by: Andras Timar <andras.timar at collabora.com>
diff --git a/common/Png.hpp b/common/Png.hpp
index 34328ce8c..dd3c70d09 100644
--- a/common/Png.hpp
+++ b/common/Png.hpp
@@ -205,7 +205,7 @@ inline bool encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t st
++nCalls;
LOG_TRC("PNG compression took "
<< duration << " ms (" << output.size() << " bytes). Average after " << nCalls
- << " calls: " << (totalDuration / static_cast<double>(nCalls)));
+ << " calls: " << (totalDuration / static_cast<double>(nCalls)) << " ms.");
}
return res;
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 72937196f..66042d9d3 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -477,9 +477,9 @@ class PngCache
for (auto it = _cache.begin(); it != _cache.end(); ++it)
avgHits += it->second.getHitCount();
- LOG_DBG("cache " << _cache.size() << " items total size " <<
+ LOG_DBG("Cache " << _cache.size() << " items total size " <<
_cacheSize << " current hits " << avgHits << ", total hit rate " <<
- (_cacheHits * 100. / _cacheTests) << "% at balance start");
+ (_cacheHits * 100. / _cacheTests) << "% at balance start.");
avgHits /= _cache.size();
for (auto it = _cache.begin(); it != _cache.end();)
@@ -500,8 +500,8 @@ class PngCache
}
}
- LOG_DBG("cache " << _cache.size() << " items total size " <<
- _cacheSize << " after balance");
+ LOG_DBG("Cache " << _cache.size() << " items with total size of " <<
+ _cacheSize << " bytes after balance.");
}
if (_hashToWireId.size() > CacheWidHardLimit)
@@ -539,6 +539,7 @@ class PngCache
}
}
+ LOG_DBG("PNG cache with hash " << hash << " missed.");
return false;
}
@@ -548,7 +549,6 @@ class PngCache
std::vector<char>& output, LibreOfficeKitTileMode mode,
TileBinaryHash hash, TileWireId wid, TileWireId /*oldWid*/)
{
- LOG_DBG("PNG cache with hash " << hash << " missed.");
/*
*Disable for now - pushed in error.
*
@@ -558,7 +558,7 @@ class PngCache
return true;
*/
- LOG_DBG("Encode a new png for this tile.");
+ LOG_DBG("Encode a new png for this tile (" << startX << ',' << startY << ").");
CacheEntry newEntry(bufferWidth * bufferHeight * 1, wid);
if (Png::encodeSubBufferToPNG(pixmap, startX, startY, width, height,
bufferWidth, bufferHeight,
@@ -569,6 +569,8 @@ class PngCache
newEntry.getData()->shrink_to_fit();
_cache.emplace(hash, newEntry);
_cacheSize += newEntry.getData()->size();
+ LOG_TRC("Cached new tile (" << startX << ',' << startY << ") with hash " << hash
+ << ". New size: " << _cacheSize);
}
output.insert(output.end(),
diff --git a/test/TileCacheTests.cpp b/test/TileCacheTests.cpp
index 575caf9f0..add630168 100644
--- a/test/TileCacheTests.cpp
+++ b/test/TileCacheTests.cpp
@@ -1128,7 +1128,7 @@ void TileCacheTests::requestTiles(std::shared_ptr<LOOLWebSocket>& socket, const
sendTextFrame(socket, text, name);
tile = assertResponseString(socket, "tile:", name);
- // expected tile: nviewid= part= width= height= tileposx= tileposy= tilewidth= tileheight=
+ // expected tile: part= width= height= tileposx= tileposy= tilewidth= tileheight=
Poco::StringTokenizer tokens(tile, " ", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
CPPUNIT_ASSERT_EQUAL(std::string("tile:"), tokens[0]);
CPPUNIT_ASSERT_EQUAL(part, std::stoi(tokens[1].substr(std::string("nviewid=").size())));
@@ -1222,34 +1222,31 @@ void TileCacheTests::testTileWireIDHandling()
assertResponseString(socket, "invalidatetiles:", testname);
// For the first input wsd will send all invalidated tiles
- int arrivedTiles = 0;
- bool gotTile = false;
- do
- {
- // If we wait for too long, the cached tiles will get evicted.
- std::vector<char> tile = getResponseMessage(socket, "tile:", testname, 500);
- gotTile = !tile.empty();
- if(gotTile)
- ++arrivedTiles;
- } while(gotTile);
+ CPPUNIT_ASSERT_MESSAGE("Expected at least two tiles.", countMessages(socket, "tile:", testname, 500) > 1);
- CPPUNIT_ASSERT_MESSAGE("We expect two tiles at least!", arrivedTiles > 1);
+ // Let WSD know we got these so it wouldn't stop sending us modified tiles automatically.
+ sendTextFrame(socket, "tileprocessed tile=0:0:0:3840:3840:0");
+ sendTextFrame(socket, "tileprocessed tile=0:3840:0:3840:3840:0");
+ sendTextFrame(socket, "tileprocessed tile=0:7680:0:3840:3840:0");
// Type an other character
- sendChar(socket, 'x', skNone, testname);
+ sendChar(socket, 'y', skNone, testname);
assertResponseString(socket, "invalidatetiles:", testname);
// For the second input wsd will send one tile, since some of them are identical.
- arrivedTiles = 0;
- do
- {
- std::vector<char> tile = getResponseMessage(socket, "tile:", testname);
- gotTile = !tile.empty();
- if(gotTile)
- ++arrivedTiles;
- } while(gotTile);
+ const int arrivedTiles = countMessages(socket, "tile:", testname, 500);
+ if (arrivedTiles == 1)
+ return;
- CPPUNIT_ASSERT_EQUAL(1, arrivedTiles);
+ // Or, at most 2. The reason is that sometimes we get line antialiasing differences that
+ // are sub-pixel different, and that results in a different hash.
+ CPPUNIT_ASSERT_EQUAL(2, arrivedTiles);
+
+ // The third time, however, we shouldn't see anything but the tile we change.
+ sendChar(socket, 'z', skNone, testname);
+ assertResponseString(socket, "invalidatetiles:", testname);
+
+ CPPUNIT_ASSERT_MESSAGE("Expected exactly one tile.", countMessages(socket, "tile:", testname, 500) == 1);
}
void TileCacheTests::testTileProcessed()
@@ -1373,37 +1370,32 @@ void TileCacheTests::testTileBeingRenderedHandling()
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, 500);
- gotTile = !tile.empty();
- if(gotTile)
- ++arrivedTiles;
- } while(gotTile);
-
- CPPUNIT_ASSERT_MESSAGE("We expect two tiles at least!", arrivedTiles > 1);
+ CPPUNIT_ASSERT_MESSAGE("Expected at least two tiles.", countMessages(socket, "tile:", testname, 500) > 1);
// For the later inputs wsd will send one tile, since other ones are indentical
for(int i = 0; i < 5; ++i)
{
+ sendTextFrame(socket, "tileprocessed tile=0:0:0:3200:3200:0");
+
// Type an other character
- sendChar(socket, 'x', skNone, testname);
+ sendChar(socket, 'y', skNone, testname);
assertResponseString(socket, "invalidatetiles:", testname);
- arrivedTiles = 0;
- do
+ const int arrivedTiles = countMessages(socket, "tile:", testname, 500);
+ if (arrivedTiles != 1)
{
- std::vector<char> tile = getResponseMessage(socket, "tile:", testname, 500);
- gotTile = !tile.empty();
- if(gotTile)
- ++arrivedTiles;
- } while(gotTile);
+ // Or, at most 2. The reason is that sometimes we get line antialiasing differences that
+ // are sub-pixel different, and that results in a different hash.
+ CPPUNIT_ASSERT_EQUAL(2, arrivedTiles);
- CPPUNIT_ASSERT_EQUAL(1, arrivedTiles);
+ sendTextFrame(socket, "tileprocessed tile=0:0:0:3200:3200:0");
- sendTextFrame(socket, "tileprocessed tile=0:0:0:3200:3200:0");
+ // The third time, however, we shouldn't see anything but the tile we change.
+ sendChar(socket, 'z', skNone, testname);
+ assertResponseString(socket, "invalidatetiles:", testname);
+
+ CPPUNIT_ASSERT_MESSAGE("Expected exactly one tile.", countMessages(socket, "tile:", testname, 500) == 1);
+ }
}
}
@@ -1432,49 +1424,38 @@ void TileCacheTests::testWireIDFilteringOnWSDSide()
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, 5000);
- gotTile = !tile.empty();
- if(gotTile)
- ++arrivedTiles;
- } while(gotTile);
+ CPPUNIT_ASSERT_MESSAGE("Expected at least two tiles.", countMessages(socket1, "tile:", testname, 500) > 1);
- CPPUNIT_ASSERT_MESSAGE("We expect two tiles at least!", arrivedTiles > 1);
+ // Let WSD know we got these so it wouldn't stop sending us modified tiles automatically.
+ sendTextFrame(socket1, "tileprocessed tile=0:0:0:3840:3840:0");
+ sendTextFrame(socket1, "tileprocessed tile=0:3840:0:3840:3840:0");
+ sendTextFrame(socket1, "tileprocessed tile=0:7680:0:3840:3840:0");
// Type an other character
- sendChar(socket1, 'x', skNone, testname);
+ sendChar(socket1, 'y', skNone, testname);
assertResponseString(socket1, "invalidatetiles:", testname);
- // For the second input wsd will send one tile, since other tiles are indentical
- arrivedTiles = 0;
- do
- {
- std::vector<char> tile = getResponseMessage(socket1, "tile:", testname, 1000);
- gotTile = !tile.empty();
- if(gotTile)
- ++arrivedTiles;
- } while(gotTile);
+ // For the second input wsd will send one tile, since some of them are identical.
+ const int arrivedTiles = countMessages(socket1, "tile:", testname, 500);
+ if (arrivedTiles == 1)
+ return;
- CPPUNIT_ASSERT_EQUAL(1, arrivedTiles);
+ // Or, at most 2. The reason is that sometimes we get line antialiasing differences that
+ // are sub-pixel different, and that results in a different hash.
+ CPPUNIT_ASSERT_EQUAL(2, arrivedTiles);
+
+ // The third time, however, we shouldn't see anything but the tile we change.
+ sendChar(socket1, 'z', skNone, testname);
+ assertResponseString(socket1, "invalidatetiles:", testname);
+
+ CPPUNIT_ASSERT_MESSAGE("Expected exactly one tile.", countMessages(socket1, "tile:", testname, 500) == 1);
//2. Now request the same tiles by the other client (e.g. scroll to the same view)
sendTextFrame(socket2, "tilecombine nviewid=0 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;
- do
- {
- std::vector<char> tile = getResponseMessage(socket2, "tile:", testname, 1000);
- gotTile = !tile.empty();
- if(gotTile)
- ++arrivedTiles;
- } while(gotTile);
-
- CPPUNIT_ASSERT_EQUAL(3, arrivedTiles);
+ CPPUNIT_ASSERT_EQUAL(3, countMessages(socket2, "tile:", testname, 500));
// wsd should not send tiles messages for the first client
std::vector<char> tile = getResponseMessage(socket1, "tile:", testname, 1000);
diff --git a/test/helpers.hpp b/test/helpers.hpp
index 6dfd5315e..fb83b9bfb 100644
--- a/test/helpers.hpp
+++ b/test/helpers.hpp
@@ -264,7 +264,7 @@ std::vector<char> getResponseMessage(LOOLWebSocket& ws, const std::string& prefi
}
response.resize(READ_BUFFER_SIZE * 8);
- int bytes = ws.receiveFrame(response.data(), response.size(), flags);
+ const int bytes = ws.receiveFrame(response.data(), response.size(), flags);
response.resize(std::max(bytes, 0));
const auto message = LOOLProtocol::getFirstLine(response);
if (bytes > 0 && (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE)
@@ -360,6 +360,22 @@ std::string assertNotInResponse(T& ws, const std::string& prefix, const std::str
}
inline
+int countMessages(LOOLWebSocket& ws, const std::string& prefix, const std::string& testname, const size_t timeoutMs = 10000)
+{
+ int count = 0;
+ while (!getResponseMessage(ws, prefix, testname, timeoutMs).empty())
+ ++count;
+
+ return count;
+}
+
+inline
+int countMessages(const std::shared_ptr<LOOLWebSocket>& ws, const std::string& prefix, const std::string& testname, const size_t timeoutMs = 10000)
+{
+ return countMessages(*ws, prefix, testname, timeoutMs);
+}
+
+inline
bool isDocumentLoaded(LOOLWebSocket& ws, const std::string& testname, bool isView = true)
{
const std::string prefix = isView ? "status:" : "statusindicatorfinish:";
@@ -393,11 +409,13 @@ connectLOKit(const Poco::URI& uri,
std::unique_ptr<Poco::Net::HTTPClientSession> session(createSession(uri));
auto ws = std::make_shared<LOOLWebSocket>(*session, request, response);
const char* expected_response = "statusindicator: ready";
+ TST_LOG_END;
if (getResponseString(ws, expected_response, testname) == expected_response)
{
return ws;
}
+ TST_LOG_BEGIN("Connecting... ");
TST_LOG_APPEND(11 - retries);
}
catch (const std::exception& ex)
More information about the Libreoffice-commits
mailing list