[Libreoffice-commits] online.git: common/Png.hpp kit/Kit.cpp test/helpers.hpp test/TileCacheTests.cpp

Ashod Nakashian (via logerrit) logerrit at kemper.freedesktop.org
Tue Oct 29 01:32:24 UTC 2019


 common/Png.hpp          |    2 
 kit/Kit.cpp             |    5 +
 test/TileCacheTests.cpp |  129 ++++++++++++++++++++----------------------------
 test/helpers.hpp        |   20 +++++++
 4 files changed, 78 insertions(+), 78 deletions(-)

New commits:
commit d110f4cc589c0c655d29215e3e5362d60ae6712e
Author:     Ashod Nakashian <ashod.nakashian at collabora.co.uk>
AuthorDate: Sat Oct 19 12:33:22 2019 -0400
Commit:     Ashod Nakashian <ashnakash at gmail.com>
CommitDate: Tue Oct 29 02:32:05 2019 +0100

    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.
    
    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>
    (cherry picked from commit 9603597fd1aaecb27893792cfd2d243e450b58b8)
    
    Change-Id: I1a66732dd3443bfbd770d8dc65721571dfa08615
    Reviewed-on: https://gerrit.libreoffice.org/81572
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/common/Png.hpp b/common/Png.hpp
index d629158c2..c0a99c74e 100644
--- a/common/Png.hpp
+++ b/common/Png.hpp
@@ -212,7 +212,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 d6d255bf1..9eac5dc7e 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -500,8 +500,8 @@ public:
                 }
             }
 
-            LOG_DBG("PNG cache has " << _cache.size() << " items, total size " <<
-                    _cacheSize << " after balance.");
+            LOG_DBG("PNG cache has " << _cache.size() << " items with total size of " <<
+                    _cacheSize << " bytes after balance.");
         }
 
         if (_hashToWireId.size() > CacheWidHardLimit)
@@ -541,6 +541,7 @@ public:
             }
         }
 
+        LOG_DBG("PNG cache with hash " << hash << " missed.");
         return false;
     }
 
diff --git a/test/TileCacheTests.cpp b/test/TileCacheTests.cpp
index 174f3d5bb..396b0e031 100644
--- a/test/TileCacheTests.cpp
+++ b/test/TileCacheTests.cpp
@@ -1270,34 +1270,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);
+    // For the second input wsd will send one tile, since some of them are identical.
+    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()
@@ -1421,37 +1418,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 identical
     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);
+        }
     }
 }
 
@@ -1480,49 +1472,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 identical
-    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 6084e3337..fa106f993 100644
--- a/test/helpers.hpp
+++ b/test/helpers.hpp
@@ -271,7 +271,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)
@@ -367,6 +367,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:";
@@ -400,11 +416,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