From a9c0db4bcbbe448daa2b8383a304c7ffde8a687f Mon Sep 17 00:00:00 2001 From: Michael Meeks Date: Thu, 7 Dec 2023 19:42:55 +0000 Subject: [PATCH] Significantly simplify tile rendering on invalidation. Accurate per client tracking of wire-id and avoidance of duplicates is vital for good delta application, and is done precisely by the ClientDeltaTracker class. The inaccurate, _oldWireId tracker imperfectly tracked wireids of the last tile per session, and used this integer for the boolean oldWireId used to determine whether to create a delta; in an un-necessarily complex way. Instead we more accurately use the TileCache to determine if we already have a key-frame, and if so use this to encourage Kit to generate a delta if possible. there seems to be a potential race though between the Kit's delta-cache size, and what keyframes we have in the TileCache requiring further investigation, but this is not a new issue. Change-Id: If6cec4c1da31f7b715336c60d1dd6f358e1ef6a4 Signed-off-by: Michael Meeks --- wsd/ClientSession.cpp | 63 ++++++++----------------------------------- wsd/ClientSession.hpp | 10 ------- wsd/TestStubs.cpp | 2 -- 3 files changed, 11 insertions(+), 64 deletions(-) diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index 2935766dcd..00938c2c03 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -768,7 +768,6 @@ bool ClientSession::_handleInput(const char *buffer, int length) } _clientVisibleArea = Util::Rectangle(x, y, width, height); - resetWireIdMap(); return forwardToChild(std::string(buffer, length), docBroker); } } @@ -786,7 +785,6 @@ bool ClientSession::_handleInput(const char *buffer, int length) else { _clientSelectedPart = temp; - resetWireIdMap(); return forwardToChild(std::string(buffer, length), docBroker); } } @@ -849,7 +847,6 @@ bool ClientSession::_handleInput(const char *buffer, int length) _tileHeightPixel = tilePixelHeight; _tileWidthTwips = tileTwipWidth; _tileHeightTwips = tileTwipHeight; - resetWireIdMap(); return forwardToChild(std::string(buffer, length), docBroker); } } @@ -1716,12 +1713,10 @@ bool ClientSession::handleKitToClientMessage(const std::shared_ptr& pay if(getTokenInteger(tokens[1], "part", setPart)) { _clientSelectedPart = setPart; - resetWireIdMap(); } else if (stringToInteger(tokens[1], setPart)) { _clientSelectedPart = setPart; - resetWireIdMap(); } else return false; @@ -2099,7 +2094,6 @@ bool ClientSession::handleKitToClientMessage(const std::shared_ptr& pay if(getTokenInteger(tokens.getParam(token), "current", part)) { _clientSelectedPart = part; - resetWireIdMap(); } int mode = 0; @@ -2334,16 +2328,11 @@ void ClientSession::enqueueSendMessage(const std::shared_ptr& data) docBroker->ASSERT_CORRECT_THREAD(); std::unique_ptr tile; - if (data->firstTokenMatches("tile:")) + if (data->firstTokenMatches("tile:") || + data->firstTokenMatches("delta:")) { // Avoid sending tile if it has the same wireID as the previously sent tile tile = std::make_unique(TileDesc::parse(data->firstLine())); - auto iter = _oldWireIds.find(tile->generateID()); - if(iter != _oldWireIds.end() && tile->getWireId() != 0 && tile->getWireId() == iter->second) - { - LOG_INF("WSD filters out a tile with the same wireID: " << tile->serialize("tile:")); - return; - } } LOG_TRC("Enqueueing client message " << data->id()); @@ -2351,10 +2340,8 @@ void ClientSession::enqueueSendMessage(const std::shared_ptr& data) std::size_t newSize = _senderQueue.enqueue(data); // Track sent tile - if (tile) - { - traceTileBySend(*tile, sizeBefore == newSize); - } + if (tile && sizeBefore != newSize) + addTileOnFly(*tile); } void ClientSession::addTileOnFly(const TileDesc& tile) @@ -2627,12 +2614,13 @@ void ClientSession::handleTileInvalidation(const std::string& message, { invalidTiles.push_back(desc); - TileWireId oldWireId = 0; - auto iter = _oldWireIds.find(invalidTiles.back().generateID()); - if(iter != _oldWireIds.end()) - oldWireId = iter->second; - - invalidTiles.back().setOldWireId(oldWireId); + TileWireId makeDelta = 1; + // FIXME: mobile with no TileCache & flushed kit cache + // FIXME: out of (a)sync kit vs. TileCache re: keyframes ? + if (getDocumentBroker()->hasTileCache() && + !getDocumentBroker()->tileCache().lookupTile(desc)) + makeDelta = 0; // force keyframe + invalidTiles.back().setOldWireId(makeDelta); invalidTiles.back().setWireId(0); } } @@ -2723,35 +2711,6 @@ bool ClientSession::isTileInsideVisibleArea(const TileDesc& tile) const return false; } -void ClientSession::resetWireIdMap() -{ - _oldWireIds.clear(); -} - -void ClientSession::traceTileBySend(const TileDesc& tile, bool deduplicated) -{ - const std::string tileID = tile.generateID(); - - // Store wireId first - auto iter = _oldWireIds.find(tileID); - if(iter != _oldWireIds.end()) - { - iter->second = tile.getWireId(); - } - else - { - // Track only tile inside the visible area - if(_clientVisibleArea.hasSurface() && isTileInsideVisibleArea(tile)) - { - _oldWireIds.insert(std::pair(tileID, tile.getWireId())); - } - } - - // Record that the tile is sent - if (!deduplicated) - addTileOnFly(tile); -} - // This removes the tag which was added in // ClientSession::postProcessCopyPayload(), else the payload parsing // in ChildSession::setClipboard() will fail. diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp index 366446cdcf..523c2d2baf 100644 --- a/wsd/ClientSession.hpp +++ b/wsd/ClientSession.hpp @@ -214,13 +214,6 @@ public: int getTileWidthInTwips() const { return _tileWidthTwips; } int getTileHeightInTwips() const { return _tileHeightTwips; } - /// This method updates internal data related to sent tiles (wireID and tiles-on-fly) - /// Call this method anytime when a new tile is sent to the client - void traceTileBySend(const TileDesc& tile, bool deduplicated = false); - - /// Clear wireId map anytime when client visible area changes (visible area, zoom, part number) - void resetWireIdMap(); - bool isTextDocument() const { return _isTextDocument; } void setThumbnailSession(const bool val) { _thumbnailSession = val; } @@ -411,9 +404,6 @@ private: /// Requested tiles are stored in this list, before we can send them to the client std::deque _requestedTiles; - /// Store wireID's of the sent tiles inside the actual visible area - std::map _oldWireIds; - /// Sockets to send binary selection content to std::vector> _clipSockets; diff --git a/wsd/TestStubs.cpp b/wsd/TestStubs.cpp index 5796a933df..27a7ef7e71 100644 --- a/wsd/TestStubs.cpp +++ b/wsd/TestStubs.cpp @@ -21,8 +21,6 @@ void DocumentBroker::assertCorrectThread(const char*, int) const {} -void ClientSession::traceTileBySend(const TileDesc& /*tile*/, bool /*deduplicated = false*/) {} - void ClientSession::enqueueSendMessage(const std::shared_ptr& /*data*/) {}; ClientSession::~ClientSession() = default;