wsd: tilecombine now includes the versions of the tiles

Previously tilecombine had its own version, which is
nonesensical, since it's not really a tile.

Now it passes the version to the tiles when
parsing and serializes version per-tile.

Change-Id: I5db8d94880431e3d2a40b6787c6fe51a05771305
Reviewed-on: https://gerrit.libreoffice.org/32633
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>

kit/Kit.cpp

@@ -683,7 +683,7 @@ public:
         }
 
         LOG_DBG("+paintTile (combined) at (" << renderArea.getLeft() << ", " << renderArea.getTop() << "), (" <<
-                renderArea.getWidth() << ", " << renderArea.getHeight() << ") ver: " << tileCombined.getVersion());
+                renderArea.getWidth() << ", " << renderArea.getHeight() << ").");
         Timestamp timestamp;
         _loKitDocument->paintPartTile(pixmap.data(), tileCombined.getPart(),
                                       pixmapWidth, pixmapHeight,
@@ -691,7 +691,7 @@ public:
                                       renderArea.getWidth(), renderArea.getHeight());
         const auto elapsed = timestamp.elapsed();
         LOG_DBG("paintTile (combined) at (" << renderArea.getLeft() << ", " << renderArea.getTop() << "), (" <<
-                renderArea.getWidth() << ", " << renderArea.getHeight() << ") ver: " << tileCombined.getVersion() <<
+                renderArea.getWidth() << ", " << renderArea.getHeight() << ") " <<
                 " rendered in " << (elapsed/1000.) << " ms (" << area / elapsed << " MP/s).");
         const auto mode = static_cast<LibreOfficeKitTileMode>(_loKitDocument->getTileMode());
 

test/TileQueueTests.cpp

@@ -114,11 +114,11 @@ void TileQueueTests::testTileCombinedRendering()
     const std::string req3 = "tile part=0 width=256 height=256 tileposx=0 tileposy=3840 tilewidth=3840 tileheight=3840";
     const std::string req4 = "tile part=0 width=256 height=256 tileposx=3840 tileposy=3840 tilewidth=3840 tileheight=3840";
 
-    const std::string resHor = "tilecombine part=0 width=256 height=256 tileposx=0,3840 tileposy=0,0 imgsize=0,0 tilewidth=3840 tileheight=3840";
+    const std::string resHor = "tilecombine part=0 width=256 height=256 tileposx=0,3840 tileposy=0,0 imgsize=0,0 tilewidth=3840 tileheight=3840 ver=-1,-1,";
     const TileQueue::Payload payloadHor(resHor.data(), resHor.data() + resHor.size());
-    const std::string resVer = "tilecombine part=0 width=256 height=256 tileposx=0,0 tileposy=0,3840 imgsize=0,0 tilewidth=3840 tileheight=3840";
+    const std::string resVer = "tilecombine part=0 width=256 height=256 tileposx=0,0 tileposy=0,3840 imgsize=0,0 tilewidth=3840 tileheight=3840 ver=-1,-1,";
     const TileQueue::Payload payloadVer(resVer.data(), resVer.data() + resVer.size());
-    const std::string resFull = "tilecombine part=0 width=256 height=256 tileposx=0,3840,0 tileposy=0,0,3840 imgsize=0,0,0 tilewidth=3840 tileheight=3840";
+    const std::string resFull = "tilecombine part=0 width=256 height=256 tileposx=0,3840,0 tileposy=0,0,3840 imgsize=0,0,0 tilewidth=3840 tileheight=3840 ver=-1,-1,-1,";
     const TileQueue::Payload payloadFull(resFull.data(), resFull.data() + resFull.size());
 
     TileQueue queue;
@@ -162,7 +162,7 @@ void TileQueueTests::testTileRecombining()
     // but when we later extract that, it is just one "tilecombine" message
     std::string message(payloadAsString(queue.get()));
 
-    CPPUNIT_ASSERT_EQUAL(std::string("tilecombine part=0 width=256 height=256 tileposx=7680,0,3840 tileposy=0,0,0 imgsize=0,0,0 tilewidth=3840 tileheight=3840"), message);
+    CPPUNIT_ASSERT_EQUAL(std::string("tilecombine part=0 width=256 height=256 tileposx=7680,0,3840 tileposy=0,0,0 imgsize=0,0,0 tilewidth=3840 tileheight=3840 ver=-1,-1,-1,"), message);
 
     // and nothing remains in the queue
     CPPUNIT_ASSERT_EQUAL(0, static_cast<int>(queue._queue.size()));

wsd/DocumentBroker.cpp

@@ -751,7 +751,7 @@ void DocumentBroker::handleTileRequest(TileDesc& tile,
 
     tile.setVersion(++_tileVersion);
     const auto tileMsg = tile.serialize();
-    LOG_TRC("Tile request for " << tile.serialize());
+    LOG_TRC("Tile request for " << tileMsg);
 
     std::unique_ptr<std::fstream> cachedTile = _tileCache->lookupTile(tile);
     if (cachedTile)
@@ -795,7 +795,7 @@ void DocumentBroker::handleTileRequest(TileDesc& tile,
     // Forward to child to render.
     LOG_DBG("Sending render request for tile (" << tile.getPart() << ',' <<
             tile.getTilePosX() << ',' << tile.getTilePosY() << ").");
-    const std::string request = "tile " + tile.serialize();
+    const std::string request = "tile " + tileMsg;
     _childProcess->sendTextFrame(request);
     _debugRenderedTileCount++;
 }
@@ -850,7 +850,6 @@ void DocumentBroker::handleTileCombinedRequest(TileCombined& tileCombined,
     if (!tiles.empty())
     {
         auto newTileCombined = TileCombined::create(tiles);
-        newTileCombined.setVersion(++_tileVersion);
 
         // Forward to child to render.
         const auto req = newTileCombined.serialize("tilecombine");

wsd/TileDesc.hpp

@@ -135,14 +135,14 @@ public:
             << " tilewidth=" << _tileWidth
             << " tileheight=" << _tileHeight;
 
+        // Anything after ver is optional.
+        oss << " ver=" << _ver;
+
         if (_id >= 0)
         {
             oss << " id=" << _id;
         }
 
-        // Anything after ver is optional.
-        oss << " ver=" << _ver;
-
         if (_imgSize > 0)
         {
             oss << " imgsize=" << _imgSize;
@@ -219,14 +219,13 @@ class TileCombined
 private:
     TileCombined(int part, int width, int height,
                  const std::string& tilePositionsX, const std::string& tilePositionsY,
-                 int tileWidth, int tileHeight, int ver,
+                 int tileWidth, int tileHeight, const std::string& vers,
                  const std::string& imgSizes, int id) :
         _part(part),
         _width(width),
         _height(height),
         _tileWidth(tileWidth),
         _tileHeight(tileHeight),
-        _ver(ver),
         _id(id)
     {
         if (_part < 0 ||
@@ -241,11 +240,14 @@ private:
         Poco::StringTokenizer positionXtokens(tilePositionsX, ",", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
         Poco::StringTokenizer positionYtokens(tilePositionsY, ",", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
         Poco::StringTokenizer sizeTokens(imgSizes, ",", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
+        Poco::StringTokenizer verTokens(vers, ",", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
 
         const auto numberOfPositions = positionYtokens.count();
 
         // check that number of positions for X and Y is the same
-        if (numberOfPositions != positionXtokens.count() || (!imgSizes.empty() && numberOfPositions != sizeTokens.count()))
+        if (numberOfPositions != positionXtokens.count() ||
+            (!imgSizes.empty() && numberOfPositions != sizeTokens.count()) ||
+            (!vers.empty() && numberOfPositions != verTokens.count()))
         {
             throw BadArgumentException("Invalid tilecombine descriptor. Uneven number of tiles.");
         }
@@ -255,19 +257,25 @@ private:
             int x = 0;
             if (!LOOLProtocol::stringToInteger(positionXtokens[i], x))
             {
-                throw BadArgumentException("Invalid tilecombine descriptor.");
+                throw BadArgumentException("Invalid 'tileposx' in tilecombine descriptor.");
             }
 
             int y = 0;
             if (!LOOLProtocol::stringToInteger(positionYtokens[i], y))
             {
-                throw BadArgumentException("Invalid tilecombine descriptor.");
+                throw BadArgumentException("Invalid 'tileposy' in tilecombine descriptor.");
             }
 
             int size = 0;
             if (sizeTokens.count() && !LOOLProtocol::stringToInteger(sizeTokens[i], size))
             {
-                throw BadArgumentException("Invalid tilecombine descriptor.");
+                throw BadArgumentException("Invalid 'imgsize' in tilecombine descriptor.");
+            }
+
+            int ver = -1;
+            if (verTokens.count() && !verTokens[i].empty() && !LOOLProtocol::stringToInteger(verTokens[i], ver))
+            {
+                throw BadArgumentException("Invalid 'ver' in tilecombine descriptor.");
             }
 
             _tiles.emplace_back(_part, _width, _height, x, y, _tileWidth, _tileHeight, ver, size, id, false);
@@ -280,8 +288,6 @@ public:
     int getHeight() const { return _height; }
     int getTileWidth() const { return _tileWidth; }
     int getTileHeight() const { return _tileHeight; }
-    int getVersion() const { return _ver; }
-    void setVersion(const int ver) { _ver = ver; }
 
     const std::vector<TileDesc>& getTiles() const { return _tiles; }
     std::vector<TileDesc>& getTiles() { return _tiles; }
@@ -322,16 +328,19 @@ public:
         oss << " tilewidth=" << _tileWidth
             << " tileheight=" << _tileHeight;
 
+        oss << " ver=";
+        for (const auto& tile : _tiles)
+        {
+            oss << tile.getVersion() << ',';
+        }
+
+        oss.seekp(-1, std::ios_base::cur); // Remove last comma.
+
         if (_id >= 0)
         {
             oss << " id=" << _id;
         }
 
-        if (_ver >= 0)
-        {
-            oss << " ver=" << _ver;
-        }
-
         return oss.str();
     }
 
@@ -343,12 +352,12 @@ public:
         std::map<std::string, int> pairs;
 
         // Optional.
-        pairs["ver"] = -1;
         pairs["id"] = -1;
 
         std::string tilePositionsX;
         std::string tilePositionsY;
         std::string imgSizes;
+        std::string versions;
         for (size_t i = 0; i < tokens.count(); ++i)
         {
             std::string name;
@@ -367,6 +376,10 @@ public:
                 {
                     imgSizes = value;
                 }
+                else if (name == "ver")
+                {
+                    versions = value;
+                }
                 else
                 {
                     int v = 0;
@@ -381,7 +394,7 @@ public:
         return TileCombined(pairs["part"], pairs["width"], pairs["height"],
                             tilePositionsX, tilePositionsY,
                             pairs["tilewidth"], pairs["tileheight"],
-                            pairs["ver"],
+                            versions,
                             imgSizes, pairs["id"]);
     }
 
@@ -399,17 +412,19 @@ public:
 
         std::ostringstream xs;
         std::ostringstream ys;
-        int ver = -1;
+        std::ostringstream vers;
 
-        for (auto& tile : tiles)
+        for (const auto& tile : tiles)
         {
             xs << tile.getTilePosX() << ',';
             ys << tile.getTilePosY() << ',';
-            ver = std::max(tile.getVersion(), ver);
+            vers << tile.getVersion() << ',';
         }
 
+        vers.seekp(-1, std::ios_base::cur); // Remove last comma.
         return TileCombined(tiles[0].getPart(), tiles[0].getWidth(), tiles[0].getHeight(),
-                            xs.str(), ys.str(), tiles[0].getTileWidth(), tiles[0].getTileHeight(), ver, "", -1);
+                            xs.str(), ys.str(), tiles[0].getTileWidth(), tiles[0].getTileHeight(),
+                            vers.str(), "", -1);
     }
 
 private:
@@ -419,7 +434,6 @@ private:
     int _height;
     int _tileWidth;
     int _tileHeight;
-    int _ver; //< Versioning support.
     int _id;
 };