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 <michael.meeks@collabora.com>
pull/7810/head
Michael Meeks 2023-12-07 19:42:55 +00:00 committed by Caolán McNamara
parent 0e1972faba
commit a9c0db4bcb
3 changed files with 11 additions and 64 deletions

View File

@ -768,7 +768,6 @@ bool ClientSession::_handleInput(const char *buffer, int length)
} }
_clientVisibleArea = Util::Rectangle(x, y, width, height); _clientVisibleArea = Util::Rectangle(x, y, width, height);
resetWireIdMap();
return forwardToChild(std::string(buffer, length), docBroker); return forwardToChild(std::string(buffer, length), docBroker);
} }
} }
@ -786,7 +785,6 @@ bool ClientSession::_handleInput(const char *buffer, int length)
else else
{ {
_clientSelectedPart = temp; _clientSelectedPart = temp;
resetWireIdMap();
return forwardToChild(std::string(buffer, length), docBroker); return forwardToChild(std::string(buffer, length), docBroker);
} }
} }
@ -849,7 +847,6 @@ bool ClientSession::_handleInput(const char *buffer, int length)
_tileHeightPixel = tilePixelHeight; _tileHeightPixel = tilePixelHeight;
_tileWidthTwips = tileTwipWidth; _tileWidthTwips = tileTwipWidth;
_tileHeightTwips = tileTwipHeight; _tileHeightTwips = tileTwipHeight;
resetWireIdMap();
return forwardToChild(std::string(buffer, length), docBroker); return forwardToChild(std::string(buffer, length), docBroker);
} }
} }
@ -1716,12 +1713,10 @@ bool ClientSession::handleKitToClientMessage(const std::shared_ptr<Message>& pay
if(getTokenInteger(tokens[1], "part", setPart)) if(getTokenInteger(tokens[1], "part", setPart))
{ {
_clientSelectedPart = setPart; _clientSelectedPart = setPart;
resetWireIdMap();
} }
else if (stringToInteger(tokens[1], setPart)) else if (stringToInteger(tokens[1], setPart))
{ {
_clientSelectedPart = setPart; _clientSelectedPart = setPart;
resetWireIdMap();
} }
else else
return false; return false;
@ -2099,7 +2094,6 @@ bool ClientSession::handleKitToClientMessage(const std::shared_ptr<Message>& pay
if(getTokenInteger(tokens.getParam(token), "current", part)) if(getTokenInteger(tokens.getParam(token), "current", part))
{ {
_clientSelectedPart = part; _clientSelectedPart = part;
resetWireIdMap();
} }
int mode = 0; int mode = 0;
@ -2334,16 +2328,11 @@ void ClientSession::enqueueSendMessage(const std::shared_ptr<Message>& data)
docBroker->ASSERT_CORRECT_THREAD(); docBroker->ASSERT_CORRECT_THREAD();
std::unique_ptr<TileDesc> tile; std::unique_ptr<TileDesc> 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 // Avoid sending tile if it has the same wireID as the previously sent tile
tile = std::make_unique<TileDesc>(TileDesc::parse(data->firstLine())); tile = std::make_unique<TileDesc>(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()); LOG_TRC("Enqueueing client message " << data->id());
@ -2351,10 +2340,8 @@ void ClientSession::enqueueSendMessage(const std::shared_ptr<Message>& data)
std::size_t newSize = _senderQueue.enqueue(data); std::size_t newSize = _senderQueue.enqueue(data);
// Track sent tile // Track sent tile
if (tile) if (tile && sizeBefore != newSize)
{ addTileOnFly(*tile);
traceTileBySend(*tile, sizeBefore == newSize);
}
} }
void ClientSession::addTileOnFly(const TileDesc& tile) void ClientSession::addTileOnFly(const TileDesc& tile)
@ -2627,12 +2614,13 @@ void ClientSession::handleTileInvalidation(const std::string& message,
{ {
invalidTiles.push_back(desc); invalidTiles.push_back(desc);
TileWireId oldWireId = 0; TileWireId makeDelta = 1;
auto iter = _oldWireIds.find(invalidTiles.back().generateID()); // FIXME: mobile with no TileCache & flushed kit cache
if(iter != _oldWireIds.end()) // FIXME: out of (a)sync kit vs. TileCache re: keyframes ?
oldWireId = iter->second; if (getDocumentBroker()->hasTileCache() &&
!getDocumentBroker()->tileCache().lookupTile(desc))
invalidTiles.back().setOldWireId(oldWireId); makeDelta = 0; // force keyframe
invalidTiles.back().setOldWireId(makeDelta);
invalidTiles.back().setWireId(0); invalidTiles.back().setWireId(0);
} }
} }
@ -2723,35 +2711,6 @@ bool ClientSession::isTileInsideVisibleArea(const TileDesc& tile) const
return false; 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<std::string, TileWireId>(tileID, tile.getWireId()));
}
}
// Record that the tile is sent
if (!deduplicated)
addTileOnFly(tile);
}
// This removes the <meta name="origin" ...> tag which was added in // This removes the <meta name="origin" ...> tag which was added in
// ClientSession::postProcessCopyPayload(), else the payload parsing // ClientSession::postProcessCopyPayload(), else the payload parsing
// in ChildSession::setClipboard() will fail. // in ChildSession::setClipboard() will fail.

View File

@ -214,13 +214,6 @@ public:
int getTileWidthInTwips() const { return _tileWidthTwips; } int getTileWidthInTwips() const { return _tileWidthTwips; }
int getTileHeightInTwips() const { return _tileHeightTwips; } 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; } bool isTextDocument() const { return _isTextDocument; }
void setThumbnailSession(const bool val) { _thumbnailSession = val; } 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 /// Requested tiles are stored in this list, before we can send them to the client
std::deque<TileDesc> _requestedTiles; std::deque<TileDesc> _requestedTiles;
/// Store wireID's of the sent tiles inside the actual visible area
std::map<std::string, TileWireId> _oldWireIds;
/// Sockets to send binary selection content to /// Sockets to send binary selection content to
std::vector<std::weak_ptr<StreamSocket>> _clipSockets; std::vector<std::weak_ptr<StreamSocket>> _clipSockets;

View File

@ -21,8 +21,6 @@
void DocumentBroker::assertCorrectThread(const char*, int) const {} void DocumentBroker::assertCorrectThread(const char*, int) const {}
void ClientSession::traceTileBySend(const TileDesc& /*tile*/, bool /*deduplicated = false*/) {}
void ClientSession::enqueueSendMessage(const std::shared_ptr<Message>& /*data*/) {}; void ClientSession::enqueueSendMessage(const std::shared_ptr<Message>& /*data*/) {};
ClientSession::~ClientSession() = default; ClientSession::~ClientSession() = default;