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 <[email protected]> AuthorDate: Sat Oct 19 12:33:22 2019 -0400 Commit: Andras Timar <[email protected]> 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 <[email protected]> Tested-by: Andras Timar <[email protected]> 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) _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
