[Libreoffice-commits] online.git: 2 commits - common/Protocol.cpp common/Protocol.hpp kit/Kit.cpp loleaflet/src test/httpwstest.cpp test/Makefile.am test/TileQueueTests.cpp wsd/protocol.txt wsd/TileDesc.hpp

Michael Meeks michael.meeks at collabora.com
Tue Jun 20 20:50:45 UTC 2017


 common/Protocol.cpp                          |   32 ++++++
 common/Protocol.hpp                          |    2 
 kit/Kit.cpp                                  |  133 ++++++++++++++++++---------
 loleaflet/src/core/Socket.js                 |    4 
 loleaflet/src/layer/tile/CalcTileLayer.js    |   14 +-
 loleaflet/src/layer/tile/ImpressTileLayer.js |   14 +-
 loleaflet/src/layer/tile/TileLayer.js        |    4 
 loleaflet/src/layer/tile/WriterTileLayer.js  |   14 +-
 test/Makefile.am                             |    2 
 test/TileQueueTests.cpp                      |   28 ++---
 test/httpwstest.cpp                          |    2 
 wsd/TileDesc.hpp                             |   85 ++++++++---------
 wsd/protocol.txt                             |   16 +--
 13 files changed, 219 insertions(+), 131 deletions(-)

New commits:
commit 73a87493f0d35629d5a16a6be7c720ec2fd726c2
Author: Michael Meeks <michael.meeks at collabora.com>
Date:   Wed Jun 14 17:28:07 2017 +0100

    Use WireIds instead of long hashes to identify tiles efficiently.
    
    Changes protocol to use 'wid' instead of 'hash' everywhere. Wire-ids
    are monotonically increasing integers that can be mapped to hash
    values for all of the hash values and tiles we cache internally.
    
    Change-Id: Ibcb25817bab0f453e93d52a6f99d3ff65059e47d

diff --git a/common/Protocol.cpp b/common/Protocol.cpp
index be1a0b4c..b5e04665 100644
--- a/common/Protocol.cpp
+++ b/common/Protocol.cpp
@@ -59,6 +59,20 @@ namespace LOOLProtocol
         return true;
     }
 
+    bool stringToUInt32(const std::string& input, uint32_t& value)
+    {
+        try
+        {
+            value = std::stoul(input);
+        }
+        catch (std::invalid_argument&)
+        {
+            return false;
+        }
+
+        return true;
+    }
+
     bool stringToUInt64(const std::string& input, uint64_t& value)
     {
         try
@@ -103,6 +117,21 @@ namespace LOOLProtocol
         return false;
     }
 
+    bool getTokenUInt32(const std::string& token, const std::string& name, uint32_t& value)
+    {
+        if (token.size() > (name.size() + 1) &&
+            token.compare(0, name.size(), name) == 0 &&
+            token[name.size()] == '=')
+        {
+            const char* str = token.data() + name.size() + 1;
+            char* endptr = nullptr;
+            value = strtoul(str, &endptr, 10);
+            return (endptr > str);
+        }
+
+        return false;
+    }
+
     bool getTokenString(const std::string& token, const std::string& name, std::string& value)
     {
         if (token.size() > (name.size() + 1) &&
diff --git a/common/Protocol.hpp b/common/Protocol.hpp
index e26703d5..1e2158a9 100644
--- a/common/Protocol.hpp
+++ b/common/Protocol.hpp
@@ -42,6 +42,7 @@ namespace LOOLProtocol
     std::tuple<int, int, std::string> ParseVersion(const std::string& version);
 
     bool stringToInteger(const std::string& input, int& value);
+    bool stringToUInt32(const std::string& input, uint32_t& value);
     bool stringToUInt64(const std::string& input, uint64_t& value);
 
     inline
@@ -66,6 +67,7 @@ namespace LOOLProtocol
     }
 
     bool getTokenInteger(const std::string& token, const std::string& name, int& value);
+    bool getTokenUInt32(const std::string& token, const std::string& name, uint32_t& value);
     bool getTokenUInt64(const std::string& token, const std::string& name, uint64_t& value);
     bool getTokenString(const std::string& token, const std::string& name, std::string& value);
     bool getTokenKeyword(const std::string& token, const std::string& name, const std::map<std::string, int>& map, int& value);
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index edf4942f..be07bbb8 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -284,11 +284,14 @@ namespace
 class PngCache
 {
     typedef std::shared_ptr< std::vector< char > > CacheData;
+
     struct CacheEntry {
         size_t    _hitCount;
+        TileWireId _wireId;
         CacheData _data;
-        CacheEntry(size_t defaultSize) :
+        CacheEntry(size_t defaultSize, TileWireId id) :
             _hitCount(1),   // Every entry is used at least once; prevent removal at birth.
+            _wireId(id),
             _data( new std::vector< char >() )
         {
             _data->reserve( defaultSize );
@@ -299,7 +302,32 @@ class PngCache
     static const size_t CacheSizeHardLimit = CacheSizeSoftLimit * 2;
     size_t _cacheHits;
     size_t _cacheTests;
-    std::map< uint64_t, CacheEntry > _cache;
+    TileWireId _nextId;
+
+    std::map< TileBinaryHash, CacheEntry > _cache;
+    std::map< TileWireId, TileBinaryHash > _wireToHash;
+
+    void clearCache(bool logStats = false)
+    {
+        if (logStats)
+            LOG_DBG("cache clear " << _cache.size() << " items total size " <<
+                    _cacheSize << " current hits " << _cacheHits);
+        _cache.clear();
+        _cacheSize = 0;
+        _cacheHits = 0;
+        _cacheTests = 0;
+        _nextId = 1;
+    }
+
+    // Keep these ids small and wrap them.
+    TileWireId createNewWireId()
+    {
+        TileWireId id = ++_nextId;
+        // FIXME: if we wrap - we should flush the clients too really ...
+        if (id < 1)
+            clearCache(true);
+        return id;
+    }
 
     void balanceCache()
     {
@@ -324,6 +352,11 @@ class PngCache
                     // Shrink cache when we exceed the size to maximize
                     // the chance of hitting these entries in the future.
                     _cacheSize -= it->second._data->size();
+
+                    auto wIt = _wireToHash.find(it->second._wireId);
+                    assert(wIt != _wireToHash.end());
+                    _wireToHash.erase(wIt);
+
                     it = _cache.erase(it);
                 }
                 else
@@ -366,10 +399,10 @@ class PngCache
                                    int width, int height,
                                    int bufferWidth, int bufferHeight,
                                    std::vector<char>& output, LibreOfficeKitTileMode mode,
-                                   const uint64_t hash)
+                                   TileBinaryHash hash, TileWireId wid)
     {
         LOG_DBG("PNG cache with hash " << hash << " missed.");
-        CacheEntry newEntry(bufferWidth * bufferHeight * 1);
+        CacheEntry newEntry(bufferWidth * bufferHeight * 1, wid);
         if (Png::encodeSubBufferToPNG(pixmap, startX, startY, width, height,
                                       bufferWidth, bufferHeight,
                                       *newEntry._data, mode))
@@ -392,39 +425,51 @@ class PngCache
     }
 
 public:
-    PngCache() :
-        _cacheSize(0),
-        _cacheHits(0),
-        _cacheTests(0)
+    PngCache()
+    {
+        clearCache();
+    }
+
+    TileWireId hashToWireId(TileBinaryHash id)
     {
+        TileWireId wid;
+        if (id == 0)
+            return 0;
+        auto it = _cache.find(id);
+        if (it != _cache.end())
+            wid = it->second._wireId;
+        else
+        {
+            wid = createNewWireId();
+            _wireToHash.emplace(wid, id);
+        }
+        return wid;
     }
 
     bool encodeBufferToPNG(unsigned char* pixmap, int width, int height,
                            std::vector<char>& output, LibreOfficeKitTileMode mode,
-                           uint64_t hash)
+                           TileBinaryHash hash, TileWireId wid)
     {
         if (cacheTest(hash, output))
-        {
             return true;
-        }
 
         return cacheEncodeSubBufferToPNG(pixmap, 0, 0, width, height,
-                                         width, height, output, mode, hash);
+                                         width, height, output, mode,
+                                         hash, wid);
     }
 
     bool encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t startY,
                               int width, int height,
                               int bufferWidth, int bufferHeight,
                               std::vector<char>& output, LibreOfficeKitTileMode mode,
-                              uint64_t hash)
+                              TileBinaryHash hash, TileWireId wid)
     {
         if (cacheTest(hash, output))
-        {
             return true;
-        }
 
         return cacheEncodeSubBufferToPNG(pixmap, startX, startY, width, height,
-                                         bufferWidth, bufferHeight, output, mode, hash);
+                                         bufferWidth, bufferHeight, output, mode,
+                                         hash, wid);
     }
 };
 
@@ -606,21 +651,9 @@ public:
         assert(ws && "Expected a non-null websocket.");
         auto tile = TileDesc::parse(tokens);
 
-        // Send back the request with all optional parameters given in the request.
-        const auto tileMsg = tile.serialize("tile:");
-#if ENABLE_DEBUG
-        const std::string response = tileMsg + " renderid=" + Util::UniqueId() + "\n";
-#else
-        const std::string response = tileMsg + "\n";
-#endif
-
-        std::vector<char> output;
-        output.reserve(response.size() + (4 * tile.getWidth() * tile.getHeight()));
-        output.resize(response.size());
-        std::memcpy(output.data(), response.data(), response.size());
-
+        size_t pixmapDataSize = 4 * tile.getWidth() * tile.getHeight();
         std::vector<unsigned char> pixmap;
-        pixmap.resize(output.capacity());
+        pixmap.resize(pixmapDataSize);
 
         std::unique_lock<std::mutex> lock(_documentMutex);
         if (!_loKitDocument)
@@ -647,18 +680,36 @@ public:
                 " ms (" << area / elapsed << " MP/s).");
         const auto mode = static_cast<LibreOfficeKitTileMode>(_loKitDocument->getTileMode());
 
-        const uint64_t hash = Png::hashBuffer(pixmap.data(), tile.getWidth(), tile.getHeight());
-        if (hash != 0 && tile.getOldHash() == hash)
+        const TileBinaryHash hash = Png::hashBuffer(pixmap.data(), tile.getWidth(), tile.getHeight());
+        TileWireId wid = _pngCache.hashToWireId(hash);
+
+        tile.setWireId(wid);
+
+        // Send back the request with all optional parameters given in the request.
+        const auto tileMsg = tile.serialize("tile:");
+#if ENABLE_DEBUG
+        const std::string response = tileMsg + " renderid=" + Util::UniqueId() + "\n";
+#else
+        const std::string response = tileMsg + "\n";
+#endif
+
+        std::vector<char> output;
+        output.reserve(response.size() + pixmapDataSize);
+        output.resize(response.size());
+        std::memcpy(output.data(), response.data(), response.size());
+
+        if (hash != 0 && tile.getOldWireId() == wid)
         {
             // The tile content is identical to what the client already has, so skip it
-            LOG_TRC("Match oldhash==hash (" << hash << "), skipping");
+            LOG_TRC("Match oldWireId==wid (" << wid << " for hash " << hash << "), skipping");
             return;
         }
 
-        if (!_pngCache.encodeBufferToPNG(pixmap.data(), tile.getWidth(), tile.getHeight(), output, mode, hash))
+        if (!_pngCache.encodeBufferToPNG(pixmap.data(), tile.getWidth(), tile.getHeight(), output, mode, hash, wid))
         {
             //FIXME: Return error.
             //sendTextFrame("error: cmd=tile kind=failure");
+
             LOG_ERR("Failed to encode tile into PNG.");
             return;
         }
@@ -742,17 +793,19 @@ public:
             const uint64_t hash = Png::hashSubBuffer(pixmap.data(), positionX * pixelWidth, positionY * pixelHeight,
                                                      pixelWidth, pixelHeight, pixmapWidth, pixmapHeight);
 
-            if (hash != 0 && tiles[tileIndex].getOldHash() == hash)
+            TileWireId wireId = _pngCache.hashToWireId(hash);
+            if (hash != 0 && tiles[tileIndex].getOldWireId() == wireId)
             {
                 // The tile content is identical to what the client already has, so skip it
                 LOG_TRC("Match for tile #" << tileIndex << " at (" << positionX << "," <<
-                        positionY << ") oldhash==hash (" << hash << "), skipping");
+                        positionY << ") oldhash==hash (" << hash << "), wireId: " << wireId << " skipping");
                 tiles.erase(tiles.begin() + tileIndex);
                 continue;
             }
 
             if (!_pngCache.encodeSubBufferToPNG(pixmap.data(), positionX * pixelWidth, positionY * pixelHeight,
-                                                pixelWidth, pixelHeight, pixmapWidth, pixmapHeight, output, mode, hash))
+                                                pixelWidth, pixelHeight, pixmapWidth, pixmapHeight, output, mode,
+                                                hash, wireId))
             {
                 //FIXME: Return error.
                 //sendTextFrame("error: cmd=tile kind=failure");
@@ -761,9 +814,9 @@ public:
             }
 
             const auto imgSize = output.size() - oldSize;
-            LOG_TRC("Encoded tile #" << tileIndex << " at (" << positionX << "," << positionY << ") with oldhash=" <<
-                    tiles[tileIndex].getOldHash() << ", hash=" << hash << " in " << imgSize << " bytes.");
-            tiles[tileIndex].setHash(hash);
+            LOG_TRC("Encoded tile #" << tileIndex << " at (" << positionX << "," << positionY << ") with oldWireId=" <<
+                    tiles[tileIndex].getOldWireId() << ", hash=" << hash << " wireId: " << wireId << " in " << imgSize << " bytes.");
+            tiles[tileIndex].setWireId(wireId);
             tiles[tileIndex].setImgSize(imgSize);
             tileIndex++;
         }
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index 433c0318..235a5948 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -760,8 +760,8 @@ L.Socket = L.Class.extend({
 			else if (tokens[i].substring(0, 12) === 'rendercount=') {
 				command.rendercount = parseInt(tokens[i].substring(12));
 			}
-			else if (tokens[i].startsWith('hash=')) {
-				command.hash = this.getParameterValue(tokens[i]);
+			else if (tokens[i].startsWith('wid=')) {
+				command.wireId = this.getParameterValue(tokens[i]);
 			}
 		}
 		if (command.tileWidth && command.tileHeight && this._map._docLayer) {
diff --git a/loleaflet/src/layer/tile/CalcTileLayer.js b/loleaflet/src/layer/tile/CalcTileLayer.js
index f0a39644..a863e4a1 100644
--- a/loleaflet/src/layer/tile/CalcTileLayer.js
+++ b/loleaflet/src/layer/tile/CalcTileLayer.js
@@ -244,7 +244,7 @@ L.CalcTileLayer = L.TileLayer.extend({
 
 		var tilePositionsX = '';
 		var tilePositionsY = '';
-		var oldHashes = '';
+		var oldWireIds = '';
 		var needsNewTiles = false;
 
 		for (var key in this._tiles) {
@@ -268,14 +268,14 @@ L.CalcTileLayer = L.TileLayer.extend({
 						tilePositionsY += ',';
 					}
 					tilePositionsY += tileTopLeft.y;
-					if (oldHashes !== '') {
-						oldHashes += ',';
+					if (oldWireIds !== '') {
+						oldWireIds += ',';
 					}
-					if (this._tiles[key].oldhash === undefined) {
-						oldHashes += '0';
+					if (this._tiles[key].oldWireId === undefined) {
+						oldWireIds += '0';
 					}
 					else {
-						oldHashes += this._tiles[key].oldhash;
+						oldWireIds += this._tiles[key].oldWireId;
 					}
 					needsNewTiles = true;
 					if (this._debug) {
@@ -300,7 +300,7 @@ L.CalcTileLayer = L.TileLayer.extend({
 				'tileposy=' + tilePositionsY + ' ' +
 				'tilewidth=' + this._tileWidthTwips + ' ' +
 				'tileheight=' + this._tileHeightTwips + ' ' +
-				'oldhash=' + oldHashes;
+				'oldwid=' + oldWireIds;
 
 			this._map._socket.sendMessage(message, '');
 			if (this._debug) {
diff --git a/loleaflet/src/layer/tile/ImpressTileLayer.js b/loleaflet/src/layer/tile/ImpressTileLayer.js
index 209232d8..d2392d1c 100644
--- a/loleaflet/src/layer/tile/ImpressTileLayer.js
+++ b/loleaflet/src/layer/tile/ImpressTileLayer.js
@@ -351,7 +351,7 @@ L.ImpressTileLayer = L.TileLayer.extend({
 
 		var tilePositionsX = '';
 		var tilePositionsY = '';
-		var oldHashes = '';
+		var oldWireIds = '';
 		var needsNewTiles = false;
 
 		for (var key in this._tiles) {
@@ -375,14 +375,14 @@ L.ImpressTileLayer = L.TileLayer.extend({
 						tilePositionsY += ',';
 					}
 					tilePositionsY += tileTopLeft.y;
-					if (oldHashes !== '') {
-						oldHashes += ',';
+					if (oldWireIds !== '') {
+						oldWireIds += ',';
 					}
-					if (this._tiles[key].oldhash === undefined) {
-						oldHashes += '0';
+					if (this._tiles[key].oldWireId === undefined) {
+						oldWireIds += '0';
 					}
 					else {
-						oldHashes += this._tiles[key].oldhash;
+						oldWireIds += this._tiles[key].oldWireId;
 					}
 					needsNewTiles = true;
 					if (this._debug) {
@@ -407,7 +407,7 @@ L.ImpressTileLayer = L.TileLayer.extend({
 				'tileposy=' + tilePositionsY + ' ' +
 				'tilewidth=' + this._tileWidthTwips + ' ' +
 				'tileheight=' + this._tileHeightTwips + ' ' +
-				'oldhash=' + oldHashes;
+				'oldwid=' + oldWireIds;
 
 			this._map._socket.sendMessage(message, '');
 			if (this._debug) {
diff --git a/loleaflet/src/layer/tile/TileLayer.js b/loleaflet/src/layer/tile/TileLayer.js
index 109569ce..986a2ca0 100644
--- a/loleaflet/src/layer/tile/TileLayer.js
+++ b/loleaflet/src/layer/tile/TileLayer.js
@@ -1175,8 +1175,8 @@ L.TileLayer = L.GridLayer.extend({
 			});
 		}
 		else if (tile) {
-			if (command.hash != undefined) {
-				tile.oldhash = command.hash;
+			if (command.wireId != undefined) {
+				tile.oldWireId = command.wireId;
 			}
 			if (this._tiles[key]._invalidCount > 0) {
 				this._tiles[key]._invalidCount -= 1;
diff --git a/loleaflet/src/layer/tile/WriterTileLayer.js b/loleaflet/src/layer/tile/WriterTileLayer.js
index 5610d010..0b1f1487 100644
--- a/loleaflet/src/layer/tile/WriterTileLayer.js
+++ b/loleaflet/src/layer/tile/WriterTileLayer.js
@@ -100,7 +100,7 @@ L.WriterTileLayer = L.TileLayer.extend({
 		var visibleArea = new L.Bounds(visibleTopLeft, visibleBottomRight);
 		var tilePositionsX = '';
 		var tilePositionsY = '';
-		var oldHashes = '';
+		var oldWireIds = '';
 		var needsNewTiles = false;
 		for (var key in this._tiles) {
 			var coords = this._tiles[key].coords;
@@ -123,14 +123,14 @@ L.WriterTileLayer = L.TileLayer.extend({
 						tilePositionsY += ',';
 					}
 					tilePositionsY += tileTopLeft.y;
-					if (oldHashes !== '') {
-						oldHashes += ',';
+					if (oldWireIds !== '') {
+						oldWireIds += ',';
 					}
-					if (this._tiles[key].oldhash === undefined) {
-						oldHashes += '0';
+					if (this._tiles[key].oldWireId === undefined) {
+						oldWireIds += '0';
 					}
 					else {
-						oldHashes += this._tiles[key].oldhash;
+						oldWireIds += this._tiles[key].oldWireId;
 					}
 					needsNewTiles = true;
 					if (this._debug) {
@@ -158,7 +158,7 @@ L.WriterTileLayer = L.TileLayer.extend({
 				'tileposy=' + tilePositionsY + ' ' +
 				'tilewidth=' + this._tileWidthTwips + ' ' +
 				'tileheight=' + this._tileHeightTwips + ' ' +
-				'oldhash=' + oldHashes;
+				'oldwid=' + oldWireIds;
 
 			this._map._socket.sendMessage(message, '');
 
diff --git a/test/Makefile.am b/test/Makefile.am
index 50629277..f7f4c090 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -75,7 +75,7 @@ check-local:
 	./run_unit.sh --log-file test.log --trs-file test.trs
 # FIXME 2: unit-oob.la fails with symbol undefined:
 # UnitWSD::testHandleRequest(UnitWSD::TestRequest, UnitHTTPServerRequest&, UnitHTTPServerResponse&) ,
-TESTS = unit-prefork.la unit-tilecache.la unit-timeout.la # unit-client.la - enable to run unit-tests in wsd ...
+TESTS = unit-prefork.la unit-tilecache.la unit-timeout.la # unit-client.la # - enable to run unit-tests in wsd ...
 else
 TESTS = ${top_builddir}/test/test
 endif
diff --git a/test/TileQueueTests.cpp b/test/TileQueueTests.cpp
index 2bf8e7a7..72f263eb 100644
--- a/test/TileQueueTests.cpp
+++ b/test/TileQueueTests.cpp
@@ -70,11 +70,11 @@ class TileQueueTests : public CPPUNIT_NS::TestFixture
 
 void TileQueueTests::testTileQueuePriority()
 {
-    const std::string reqHigh = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 oldhash=0 hash=0";
-    const std::string resHigh = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 oldhash=0 hash=0 ver=-1";
+    const std::string reqHigh = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 oldwid=0 wid=0";
+    const std::string resHigh = "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 oldwid=0 wid=0 ver=-1";
     const TileQueue::Payload payloadHigh(resHigh.data(), resHigh.data() + resHigh.size());
-    const std::string reqLow = "tile part=0 width=256 height=256 tileposx=0 tileposy=253440 tilewidth=3840 tileheight=3840 oldhash=0 hash=0";
-    const std::string resLow = "tile part=0 width=256 height=256 tileposx=0 tileposy=253440 tilewidth=3840 tileheight=3840 oldhash=0 hash=0 ver=-1";
+    const std::string reqLow = "tile part=0 width=256 height=256 tileposx=0 tileposy=253440 tilewidth=3840 tileheight=3840 oldwid=0 wid=0";
+    const std::string resLow = "tile part=0 width=256 height=256 tileposx=0 tileposy=253440 tilewidth=3840 tileheight=3840 oldwid=0 wid=0 ver=-1";
     const TileQueue::Payload payloadLow(resLow.data(), resLow.data() + resLow.size());
 
     TileQueue queue;
@@ -121,11 +121,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 ver=-1,-1 oldhash=0,0 hash=0,0";
+    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 oldwid=0,0 wid=0,0";
     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 ver=-1,-1 oldhash=0,0 hash=0,0";
+    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 oldwid=0,0 wid=0,0";
     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 ver=-1,-1,-1 oldhash=0,0,0 hash=0,0,0";
+    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 oldwid=0,0,0 wid=0,0,0";
     const TileQueue::Payload payloadFull(resFull.data(), resFull.data() + resFull.size());
 
     TileQueue queue;
@@ -169,7 +169,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 ver=-1,-1,-1 oldhash=0,0,0 hash=0,0,0"), 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 oldwid=0,0,0 wid=0,0,0"), message);
 
     // and nothing remains in the queue
     CPPUNIT_ASSERT_EQUAL(0, static_cast<int>(queue._queue.size()));
@@ -189,10 +189,10 @@ void TileQueueTests::testViewOrder()
 
     const std::vector<std::string> tiles =
     {
-        "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 oldhash=0 hash=0 ver=-1",
-        "tile part=0 width=256 height=256 tileposx=0 tileposy=7680 tilewidth=3840 tileheight=3840 oldhash=0 hash=0 ver=-1",
-        "tile part=0 width=256 height=256 tileposx=0 tileposy=15360 tilewidth=3840 tileheight=3840 oldhash=0 hash=0 ver=-1",
-        "tile part=0 width=256 height=256 tileposx=0 tileposy=23040 tilewidth=3840 tileheight=3840 oldhash=0 hash=0 ver=-1"
+        "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 oldwid=0 wid=0 ver=-1",
+        "tile part=0 width=256 height=256 tileposx=0 tileposy=7680 tilewidth=3840 tileheight=3840 oldwid=0 wid=0 ver=-1",
+        "tile part=0 width=256 height=256 tileposx=0 tileposy=15360 tilewidth=3840 tileheight=3840 oldwid=0 wid=0 ver=-1",
+        "tile part=0 width=256 height=256 tileposx=0 tileposy=23040 tilewidth=3840 tileheight=3840 oldwid=0 wid=0 ver=-1"
     };
 
     for (auto &tile : tiles)
@@ -237,8 +237,8 @@ void TileQueueTests::testPreviewsDeprioritization()
     // the previews
     const std::vector<std::string> tiles =
     {
-        "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 oldhash=0 hash=0 ver=-1",
-        "tile part=0 width=256 height=256 tileposx=0 tileposy=7680 tilewidth=3840 tileheight=3840 oldhash=0 hash=0 ver=-1"
+        "tile part=0 width=256 height=256 tileposx=0 tileposy=0 tilewidth=3840 tileheight=3840 oldwid=0 wid=0 ver=-1",
+        "tile part=0 width=256 height=256 tileposx=0 tileposy=7680 tilewidth=3840 tileheight=3840 oldwid=0 wid=0 ver=-1"
     };
 
     for (auto &preview : previews)
diff --git a/test/httpwstest.cpp b/test/httpwstest.cpp
index 79c64fa4..c207b5ea 100644
--- a/test/httpwstest.cpp
+++ b/test/httpwstest.cpp
@@ -1876,7 +1876,7 @@ void HTTPWSTest::testCalcRenderAfterNewView53()
     sendTextFrame(socket, "key type=input char=0 key=1031", testname);
 
     // Get tile.
-    const auto req = "tilecombine part=0 width=256 height=256 tileposx=0 tileposy=291840 tilewidth=3840 tileheight=3840 oldhash=0";
+    const auto req = "tilecombine part=0 width=256 height=256 tileposx=0 tileposy=291840 tilewidth=3840 tileheight=3840 oldwid=0";
     const std::vector<char> tile1 = getTileAndSave(socket, req, "/tmp/calc_render_53_orig.png", testname);
 
 
diff --git a/wsd/TileDesc.hpp b/wsd/TileDesc.hpp
index 0004c1e7..a4c61636 100644
--- a/wsd/TileDesc.hpp
+++ b/wsd/TileDesc.hpp
@@ -20,6 +20,9 @@
 #include "Exceptions.hpp"
 #include "Protocol.hpp"
 
+typedef uint32_t TileWireId;
+typedef uint64_t TileBinaryHash;
+
 /// Tile Descriptor
 /// Represents a tile's coordinates and dimensions.
 class TileDesc
@@ -37,8 +40,8 @@ public:
         _imgSize(imgSize),
         _id(id),
         _broadcast(broadcast),
-        _oldHash(0),
-        _hash(0)
+        _oldWireId(0),
+        _wireId(0)
     {
         if (_part < 0 ||
             _width <= 0 ||
@@ -66,10 +69,10 @@ public:
     void setImgSize(const int imgSize) { _imgSize = imgSize; }
     int getId() const { return _id; }
     bool getBroadcast() const { return _broadcast; }
-    void setOldHash(uint64_t hash) { _oldHash = hash; }
-    uint64_t getOldHash() const { return _oldHash; }
-    void setHash(uint64_t hash) { _hash = hash; }
-    uint64_t getHash() const { return _hash; }
+    void setOldWireId(TileWireId id) { _oldWireId = id; }
+    TileWireId getOldWireId() const { return _oldWireId; }
+    void setWireId(TileWireId id) { _wireId = id; }
+    TileWireId getWireId() const { return _wireId; }
 
     bool operator==(const TileDesc& other) const
     {
@@ -145,8 +148,8 @@ public:
             << " tileposy=" << _tilePosY
             << " tilewidth=" << _tileWidth
             << " tileheight=" << _tileHeight
-            << " oldhash=" << _oldHash
-            << " hash=" << _hash;
+            << " oldwid=" << _oldWireId
+            << " wid=" << _wireId;
 
         // Anything after ver is optional.
         oss << " ver=" << _ver;
@@ -181,13 +184,13 @@ public:
         pairs["imgsize"] = 0;
         pairs["id"] = -1;
 
-        uint64_t oldHash = 0;
-        uint64_t hash = 0;
+        TileWireId oldWireId = 0;
+        TileWireId wireId = 0;
         for (size_t i = 0; i < tokens.size(); ++i)
         {
-            if (LOOLProtocol::getTokenUInt64(tokens[i], "oldhash", oldHash))
+            if (LOOLProtocol::getTokenUInt32(tokens[i], "oldwid", oldWireId))
                 ;
-            else if (LOOLProtocol::getTokenUInt64(tokens[i], "hash", hash))
+            else if (LOOLProtocol::getTokenUInt32(tokens[i], "wid", wireId))
                 ;
             else
             {
@@ -209,8 +212,8 @@ public:
                                pairs["tilewidth"], pairs["tileheight"],
                                pairs["ver"],
                                pairs["imgsize"], pairs["id"], broadcast);
-        result.setOldHash(oldHash);
-        result.setHash(hash);
+        result.setOldWireId(oldWireId);
+        result.setWireId(wireId);
 
         return result;
     }
@@ -233,8 +236,8 @@ private:
     int _imgSize; //< Used for responses.
     int _id;
     bool _broadcast;
-    uint64_t _oldHash;
-    uint64_t _hash;
+    TileWireId _oldWireId;
+    TileWireId _wireId;
 };
 
 /// One or more tile header.
@@ -247,8 +250,8 @@ private:
                  const std::string& tilePositionsX, const std::string& tilePositionsY,
                  int tileWidth, int tileHeight, const std::string& vers,
                  const std::string& imgSizes, int id,
-                 const std::string& oldHashes,
-                 const std::string& hashes) :
+                 const std::string& oldWireIds,
+                 const std::string& wireIds) :
         _part(part),
         _width(width),
         _height(height),
@@ -269,8 +272,8 @@ private:
         Poco::StringTokenizer positionYtokens(tilePositionsY, ",", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
         Poco::StringTokenizer imgSizeTokens(imgSizes, ",", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
         Poco::StringTokenizer verTokens(vers, ",", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
-        Poco::StringTokenizer oldHashTokens(oldHashes, ",", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
-        Poco::StringTokenizer hashTokens(hashes, ",", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
+        Poco::StringTokenizer oldWireIdTokens(oldWireIds, ",", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
+        Poco::StringTokenizer wireIdTokens(wireIds, ",", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
 
         const auto numberOfPositions = positionXtokens.count();
 
@@ -278,8 +281,8 @@ private:
         if (numberOfPositions != positionYtokens.count() ||
             (!imgSizes.empty() && numberOfPositions != imgSizeTokens.count()) ||
             (!vers.empty() && numberOfPositions != verTokens.count()) ||
-            (!oldHashes.empty() && numberOfPositions != oldHashTokens.count()) ||
-            (!hashes.empty() && numberOfPositions != hashTokens.count()))
+            (!oldWireIds.empty() && numberOfPositions != oldWireIdTokens.count()) ||
+            (!wireIds.empty() && numberOfPositions != wireIdTokens.count()))
         {
             throw BadArgumentException("Invalid tilecombine descriptor. Unequal number of tiles in parameters.");
         }
@@ -310,21 +313,21 @@ private:
                 throw BadArgumentException("Invalid 'ver' in tilecombine descriptor.");
             }
 
-            uint64_t oldHash = 0;
-            if (oldHashTokens.count() && !LOOLProtocol::stringToUInt64(oldHashTokens[i], oldHash))
+            TileWireId oldWireId = 0;
+            if (oldWireIdTokens.count() && !LOOLProtocol::stringToUInt32(oldWireIdTokens[i], oldWireId))
             {
                 throw BadArgumentException("Invalid tilecombine descriptor.");
             }
 
-            uint64_t hash = 0;
-            if (hashTokens.count() && !LOOLProtocol::stringToUInt64(hashTokens[i], hash))
+            TileWireId wireId = 0;
+            if (wireIdTokens.count() && !LOOLProtocol::stringToUInt32(wireIdTokens[i], wireId))
             {
                 throw BadArgumentException("Invalid tilecombine descriptor.");
             }
 
             _tiles.emplace_back(_part, _width, _height, x, y, _tileWidth, _tileHeight, ver, imgSize, id, false);
-            _tiles.back().setOldHash(oldHash);
-            _tiles.back().setHash(hash);
+            _tiles.back().setOldWireId(oldWireId);
+            _tiles.back().setWireId(wireId);
         }
     }
 
@@ -378,17 +381,17 @@ public:
         }
         oss.seekp(-1, std::ios_base::cur); // Ditto.
 
-        oss << " oldhash=";
+        oss << " oldwid=";
         for (const auto& tile : _tiles)
         {
-            oss << tile.getOldHash() << ',';
+            oss << tile.getOldWireId() << ',';
         }
         oss.seekp(-1, std::ios_base::cur); // Ditto
 
-        oss << " hash=";
+        oss << " wid=";
         for (const auto& tile : _tiles)
         {
-            oss << tile.getHash() << ',';
+            oss << tile.getWireId() << ',';
         }
         oss.seekp(-1, std::ios_base::cur); // See beow.
 
@@ -416,8 +419,8 @@ public:
         std::string tilePositionsY;
         std::string imgSizes;
         std::string versions;
-        std::string oldhashes;
-        std::string hashes;
+        std::string oldwireIds;
+        std::string wireIds;
 
         for (const auto& token : tokens)
         {
@@ -441,13 +444,13 @@ public:
                 {
                     versions = value;
                 }
-                else if (name == "oldhash")
+                else if (name == "oldwid")
                 {
-                    oldhashes = value;
+                    oldwireIds = value;
                 }
-                else if (name == "hash")
+                else if (name == "wid")
                 {
-                    hashes = value;
+                    wireIds = value;
                 }
                 else
                 {
@@ -464,7 +467,7 @@ public:
                             tilePositionsX, tilePositionsY,
                             pairs["tilewidth"], pairs["tileheight"],
                             versions,
-                            imgSizes, pairs["id"], oldhashes, hashes);
+                            imgSizes, pairs["id"], oldwireIds, wireIds);
     }
 
     /// Deserialize a TileDesc from a string format.
@@ -488,8 +491,8 @@ public:
             xs << tile.getTilePosX() << ',';
             ys << tile.getTilePosY() << ',';
             vers << tile.getVersion() << ',';
-            oldhs << tile.getOldHash() << ',';
-            hs << tile.getHash() << ',';
+            oldhs << tile.getOldWireId() << ',';
+            hs << tile.getWireId() << ',';
         }
 
         vers.seekp(-1, std::ios_base::cur); // Remove last comma.
diff --git a/wsd/protocol.txt b/wsd/protocol.txt
index eaa30727..281aa257 100644
--- a/wsd/protocol.txt
+++ b/wsd/protocol.txt
@@ -129,11 +129,10 @@ status
 
 styles
 
-tile part=<partNumber> width=<width> height=<height> tileposx=<xpos> tileposy=<ypos> tilewidth=<tileWidth> tileheight=<tileHeight> [timestamp=<time>] [id=<id> broadcast=<yesOrNo>] [oldhash=<hash>]
+tile part=<partNumber> width=<width> height=<height> tileposx=<xpos> tileposy=<ypos> tilewidth=<tileWidth> tileheight=<tileHeight> [timestamp=<time>] [id=<id> broadcast=<yesOrNo>] [oldwid=<wireId>]
 
     Parameters are numbers except broadcast which is 'yes' or 'no' and
-    hash which is a 64-bit hash. (There is no need for the client to
-    parse it into a number, it can be treated as an opaque string.)
+    wireId which is an opaque string identifier unique to this tile.
 
     Note: id must be echoed back in the response verbatim. It and the
     following parameter, broadcast, are used when rendering slide
@@ -144,7 +143,7 @@ tile part=<partNumber> width=<width> height=<height> tileposx=<xpos> tileposy=<y
 tilecombine <parameters>
 
     Accepts same parameters as the 'tile' message except that
-    parameters 'tileposx', 'tileposy' and 'oldhash' are
+    parameters 'tileposx', 'tileposy' and 'oldwid are
     comma-separated lists, and the number of elements in each must be
     same.
 
@@ -323,16 +322,17 @@ textselectioncontent: <content>
 
     Current selection's content
 
-tile: part=<partNumber> width=<width> height=<height> tileposx=<xpos> tileposy=<ypos> tilewidth=<tileWidth> tileheight=<tileHeight> [timestamp=<time>] [renderid=<id>] [hash=<hash>]
+tile: part=<partNumber> width=<width> height=<height> tileposx=<xpos> tileposy=<ypos> tilewidth=<tileWidth> tileheight=<tileHeight> [timestamp=<time>] [renderid=<id>] [wid=<wireId>]
 <binaryPngImage>
 
     The parameters from the corresponding 'tile' command.
 
     In a debug build, the renderid is either a unique identifier,
     different for each actual call to LibreOfficeKit to render a tile,
-    or the string 'cached' if the tile was found in the cache. hash is
-    a hash of the tile contents, and can be included by the client in
-    the next 'tile' message requesting the same tile.
+    or the string 'cached' if the tile was found in the cache. WireId
+    is a unique reference to a hashed tile on the server, and can
+    be included by the client in the next 'tile' message requesting
+    the same tile.
 
 commandresult: <payload>
     This is used to acknowledge the commands from the client.
commit 8fb48811eccbf8f6d77919ce91c3cd918de15f56
Author: Michael Meeks <michael.meeks at collabora.com>
Date:   Tue Jun 20 21:49:02 2017 +0100

    Protocol - only match complete token names.
    
    Change-Id: I027e29da8cc6c48a0d896fa41516934a3ff71b43

diff --git a/common/Protocol.cpp b/common/Protocol.cpp
index 0aa50c47..be1a0b4c 100644
--- a/common/Protocol.cpp
+++ b/common/Protocol.cpp
@@ -188,8 +188,9 @@ namespace LOOLProtocol
             auto pos = message.find(name);
             while (pos != std::string::npos)
             {
+                bool spaceBefore = pos == 0 || message[pos-1] == ' ';
                 const auto beg = pos + name.size();
-                if (message[beg] == '=')
+                if (spaceBefore && message[beg] == '=')
                 {
                     const auto end = message.find_first_of(" \n", beg);
                     value = message.substr(beg + 1, end - beg - 1);


More information about the Libreoffice-commits mailing list