[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/MasterProcessSession.cpp loolwsd/test loolwsd/TileCache.cpp loolwsd/TileCache.hpp
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Sun May 15 23:01:05 UTC 2016
loolwsd/DocumentBroker.cpp | 87 +++++++++++----------------------------
loolwsd/DocumentBroker.hpp | 3 -
loolwsd/MasterProcessSession.cpp | 39 +++--------------
loolwsd/TileCache.cpp | 62 ++++++++++++---------------
loolwsd/TileCache.hpp | 14 +++---
loolwsd/test/TileCacheTests.cpp | 9 ++--
6 files changed, 73 insertions(+), 141 deletions(-)
New commits:
commit d18bca992c2774c2c7b41ae0999a7d5d30a8822c
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun May 15 18:47:08 2016 -0400
loolwsd: use TileDesc instead of explicit values
Change-Id: I56ba6c4e63a495500093e7353477175d40152d11
Reviewed-on: https://gerrit.libreoffice.org/25020
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index fe09607..35af2c3 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -399,32 +399,15 @@ bool DocumentBroker::handleInput(const std::vector<char>& payload)
return true;
}
-void DocumentBroker::handleTileRequest(int part, int width, int height, int tilePosX,
- int tilePosY, int tileWidth, int tileHeight, int id,
+void DocumentBroker::handleTileRequest(const TileDesc& tile,
const std::shared_ptr<MasterProcessSession>& session)
{
- Log::trace() << "Tile request for part: " << part << ", width: " << width << ", height: " << height
- << ", tilePosX: " << tilePosX << ", tilePosY: " << tilePosY << ", tileWidth: " << tileWidth
- << ", tileHeight: " << tileHeight << ", id: " << id << Log::end;
-
- std::ostringstream oss;
- oss << " part=" << part
- << " width=" << width
- << " height=" << height
- << " tileposx=" << tilePosX
- << " tileposy=" << tilePosY
- << " tilewidth=" << tileWidth
- << " tileheight=" << tileHeight;
- if (id >= 0)
- {
- oss << " id=" << id;
- }
-
- const std::string tileMsg = oss.str();
+ const auto tileMsg = tile.serialize();
+ Log::trace() << "Tile request for " << tileMsg << Log::end;
std::unique_lock<std::mutex> lock(_mutex);
- std::unique_ptr<std::fstream> cachedTile = _tileCache->lookupTile(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight);
+ std::unique_ptr<std::fstream> cachedTile = _tileCache->lookupTile(tile);
if (cachedTile)
{
@@ -435,7 +418,7 @@ void DocumentBroker::handleTileRequest(int part, int width, int height, int tile
#endif
std::vector<char> output;
- output.reserve(4 * width * height);
+ output.reserve(4 * tile.getWidth() * tile.getHeight());
output.resize(response.size());
std::memcpy(output.data(), response.data(), response.size());
@@ -452,12 +435,10 @@ void DocumentBroker::handleTileRequest(int part, int width, int height, int tile
return;
}
- if (tileCache().isTileBeingRenderedIfSoSubscribe(
- part, width, height, tilePosX, tilePosY, tileWidth,
- tileHeight, session))
+ if (tileCache().isTileBeingRenderedIfSoSubscribe(tile, session))
return;
- Log::debug() << "Sending render request for tile (" << part << ',' << tilePosX << ',' << tilePosY << ")." << Log::end;
+ Log::debug() << "Sending render request for tile (" << tile.getPart() << ',' << tile.getTilePosX() << ',' << tile.getTilePosY() << ")." << Log::end;
// Forward to child to render.
const std::string request = "tile " + tileMsg;
@@ -467,45 +448,29 @@ void DocumentBroker::handleTileRequest(int part, int width, int height, int tile
void DocumentBroker::handleTileResponse(const std::vector<char>& payload)
{
const std::string firstLine = getFirstLine(payload);
- Poco::StringTokenizer tokens(firstLine, " ", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
-
- int part, width, height, tilePosX, tilePosY, tileWidth, tileHeight;
- if (tokens.count() < 8 ||
- !getTokenInteger(tokens[1], "part", part) ||
- !getTokenInteger(tokens[2], "width", width) ||
- !getTokenInteger(tokens[3], "height", height) ||
- !getTokenInteger(tokens[4], "tileposx", tilePosX) ||
- !getTokenInteger(tokens[5], "tileposy", tilePosY) ||
- !getTokenInteger(tokens[6], "tilewidth", tileWidth) ||
- !getTokenInteger(tokens[7], "tileheight", tileHeight))
- {
- //FIXME: Return error.
- //sendTextFrame("error: cmd=tile kind=syntax");
- Log::error("Invalid tile request [" + firstLine + "].");
- return;
- }
-
- size_t index = 8;
- int id = -1;
- if (tokens.count() > index && tokens[index].find("id") == 0)
+ try
{
- getTokenInteger(tokens[index], "id", id);
- ++index;
- }
-
- const auto buffer = payload.data();
- const auto length = payload.size();
+ auto tile = TileDesc::parse(firstLine);
+ const auto buffer = payload.data();
+ const auto length = payload.size();
- if(firstLine.size() < static_cast<std::string::size_type>(length) - 1)
- {
- tileCache().saveTile(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight, buffer + firstLine.size() + 1, length - firstLine.size() - 1);
- tileCache().notifyAndRemoveSubscribers(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight, id);
+ if(firstLine.size() < static_cast<std::string::size_type>(length) - 1)
+ {
+ tileCache().saveTile(tile, buffer + firstLine.size() + 1, length - firstLine.size() - 1);
+ tileCache().notifyAndRemoveSubscribers(tile);
+ }
+ else
+ {
+ Log::debug() << "Render request declined for " << firstLine << Log::end;
+ std::unique_lock<std::mutex> tileBeingRenderedLock(tileCache().getTilesBeingRenderedLock());
+ tileCache().forgetTileBeingRendered(tile);
+ }
}
- else
+ catch (const std::exception& exc)
{
- Log::debug() << "Render request declined for " << firstLine << Log::end;
- std::unique_lock<std::mutex> tileBeingRenderedLock(tileCache().getTilesBeingRenderedLock());
- tileCache().forgetTileBeingRendered(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight);
+ Log::error("Failed to process tile response [" + firstLine + "]: " + exc.what() + ".");
+ //FIXME: Return error.
+ //sendTextFrame("error: cmd=tile kind=syntax");
}
}
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index ca22419..89b1036 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -197,8 +197,7 @@ public:
/// Removes a session by ID. Returns the new number of sessions.
size_t removeSession(const std::string& id);
- void handleTileRequest(int part, int width, int height, int tilePosX,
- int tilePosY, int tileWidth, int tileHeight, int id,
+ void handleTileRequest(const TileDesc& tile,
const std::shared_ptr<MasterProcessSession>& session);
void handleTileResponse(const std::vector<char>& payload);
diff --git a/loolwsd/MasterProcessSession.cpp b/loolwsd/MasterProcessSession.cpp
index c62ef32..afc864d 100644
--- a/loolwsd/MasterProcessSession.cpp
+++ b/loolwsd/MasterProcessSession.cpp
@@ -499,41 +499,16 @@ void MasterProcessSession::sendFontRendering(const char *buffer, int length, Str
void MasterProcessSession::sendTile(const char * /*buffer*/, int /*length*/, StringTokenizer& tokens)
{
- int part, width, height, tilePosX, tilePosY, tileWidth, tileHeight, id = -1;
- if (tokens.count() < 8 ||
- !getTokenInteger(tokens[1], "part", part) ||
- !getTokenInteger(tokens[2], "width", width) ||
- !getTokenInteger(tokens[3], "height", height) ||
- !getTokenInteger(tokens[4], "tileposx", tilePosX) ||
- !getTokenInteger(tokens[5], "tileposy", tilePosY) ||
- !getTokenInteger(tokens[6], "tilewidth", tileWidth) ||
- !getTokenInteger(tokens[7], "tileheight", tileHeight))
+ try
{
- sendTextFrame("error: cmd=tile kind=syntax");
- return;
+ auto tileDesc = TileDesc::parse(tokens);
+ _docBroker->handleTileRequest(tileDesc, shared_from_this());
}
-
- if (part < 0 ||
- width <= 0 ||
- height <= 0 ||
- tilePosX < 0 ||
- tilePosY < 0 ||
- tileWidth <= 0 ||
- tileHeight <= 0)
+ catch (const std::exception& exc)
{
+ Log::error(std::string("Failed to process tile command: ") + exc.what() + ".");
sendTextFrame("error: cmd=tile kind=invalid");
- return;
}
-
- size_t index = 8;
- if (tokens.count() > index && tokens[index].find("id") == 0)
- {
- getTokenInteger(tokens[index], "id", id);
- ++index;
- }
-
- _docBroker->handleTileRequest(part, width, height, tilePosX, tilePosY,
- tileWidth, tileHeight, id, shared_from_this());
}
void MasterProcessSession::sendCombinedTiles(const char* /*buffer*/, int /*length*/, StringTokenizer& tokens)
@@ -604,8 +579,8 @@ void MasterProcessSession::sendCombinedTiles(const char* /*buffer*/, int /*lengt
return;
}
- _docBroker->handleTileRequest(part, pixelWidth, pixelHeight, x, y,
- tileWidth, tileHeight, id, shared_from_this());
+ const TileDesc tile(part, pixelWidth, pixelHeight, x, y, tileWidth, tileHeight);
+ _docBroker->handleTileRequest(tile, shared_from_this());
}
}
diff --git a/loolwsd/TileCache.cpp b/loolwsd/TileCache.cpp
index a139490..44213e6 100644
--- a/loolwsd/TileCache.cpp
+++ b/loolwsd/TileCache.cpp
@@ -141,9 +141,9 @@ private:
std::chrono::steady_clock::time_point _startTime;
};
-std::shared_ptr<TileCache::TileBeingRendered> TileCache::findTileBeingRendered(int part, int width, int height, int tilePosX, int tilePosY, int tileWidth, int tileHeight)
+std::shared_ptr<TileCache::TileBeingRendered> TileCache::findTileBeingRendered(const TileDesc& tileDesc)
{
- const std::string cachedName = cacheFileName(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight);
+ const std::string cachedName = cacheFileName(tileDesc);
Util::assertIsLocked(_tilesBeingRenderedMutex);
@@ -151,9 +151,9 @@ std::shared_ptr<TileCache::TileBeingRendered> TileCache::findTileBeingRendered(i
return (tile != _tilesBeingRendered.end() ? tile->second : nullptr);
}
-void TileCache::forgetTileBeingRendered(int part, int width, int height, int tilePosX, int tilePosY, int tileWidth, int tileHeight)
+void TileCache::forgetTileBeingRendered(const TileDesc& tile)
{
- const std::string cachedName = cacheFileName(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight);
+ const std::string cachedName = cacheFileName(tile);
Util::assertIsLocked(_tilesBeingRenderedMutex);
@@ -161,12 +161,14 @@ void TileCache::forgetTileBeingRendered(int part, int width, int height, int til
_tilesBeingRendered.erase(cachedName);
}
-std::unique_ptr<std::fstream> TileCache::lookupTile(int part, int width, int height, int tilePosX, int tilePosY, int tileWidth, int tileHeight)
+std::unique_ptr<std::fstream> TileCache::lookupTile(const TileDesc& tile)
{
- const std::string fileName = _cacheDir + "/" + cacheFileName(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight);
+ const std::string fileName = _cacheDir + "/" + cacheFileName(tile);
std::unique_ptr<std::fstream> result(new std::fstream(fileName, std::ios::in));
- UnitWSD::get().lookupTile(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight, result);
+ UnitWSD::get().lookupTile(tile.getPart(), tile.getWidth(), tile.getHeight(),
+ tile.getTilePosX(), tile.getTilePosY(),
+ tile.getTileWidth(), tile.getTileHeight(), result);
if (result && result->is_open())
{
@@ -177,9 +179,9 @@ std::unique_ptr<std::fstream> TileCache::lookupTile(int part, int width, int hei
return nullptr;
}
-void TileCache::saveTile(int part, int width, int height, int tilePosX, int tilePosY, int tileWidth, int tileHeight, const char *data, size_t size)
+void TileCache::saveTile(const TileDesc& tile, const char *data, size_t size)
{
- const std::string fileName = _cacheDir + "/" + cacheFileName(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight);
+ const std::string fileName = _cacheDir + "/" + cacheFileName(tile);
Log::trace() << "Saving cache tile: " << fileName << Log::end;
@@ -331,12 +333,12 @@ void TileCache::removeFile(const std::string& fileName)
Log::info("Removed file: " + fullFileName);
}
-std::string TileCache::cacheFileName(int part, int width, int height, int tilePosX, int tilePosY, int tileWidth, int tileHeight)
+std::string TileCache::cacheFileName(const TileDesc& tile)
{
std::ostringstream oss;
- oss << part << '_' << width << 'x' << height << '.'
- << tilePosX << ',' << tilePosY << '.'
- << tileWidth << 'x' << tileHeight << ".png";
+ oss << tile.getPart() << '_' << tile.getWidth() << 'x' << tile.getHeight() << '.'
+ << tile.getTilePosX() << ',' << tile.getTilePosY() << '.'
+ << tile.getTileWidth() << 'x' << tile.getTileHeight() << ".png";
return oss.str();
}
@@ -386,28 +388,15 @@ void TileCache::saveLastModified(const Timestamp& timestamp)
modTimeFile.close();
}
-void TileCache::notifyAndRemoveSubscribers(int part, int width, int height, int tilePosX, int tilePosY, int tileWidth, int tileHeight, int id)
+void TileCache::notifyAndRemoveSubscribers(const TileDesc& tile)
{
std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex);
- std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight);
+ std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(tile);
if (!tileBeingRendered)
return;
- std::ostringstream oss;
- oss << "tile part=" << part
- << " width=" << width
- << " height=" << height
- << " tileposx=" << tilePosX
- << " tileposy=" << tilePosY
- << " tilewidth=" << tileWidth
- << " tileheight=" << tileHeight;
- if (id >= 0)
- {
- oss << " id=" << id;
- }
-
- const std::string message = oss.str();
+ const std::string message = tile.serialize("tile");
Log::debug("Sending tile message to subscribers: " + message);
for (const auto& i: tileBeingRendered->_subscribers)
@@ -427,19 +416,21 @@ void TileCache::notifyAndRemoveSubscribers(int part, int width, int height, int
}
}
}
- forgetTileBeingRendered(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight);
+
+ forgetTileBeingRendered(tile);
}
// FIXME: to be further simplified when we centralize tile messages.
-bool TileCache::isTileBeingRenderedIfSoSubscribe(int part, int width, int height, int tilePosX, int tilePosY, int tileWidth, int tileHeight, const std::shared_ptr<MasterProcessSession> &subscriber)
+bool TileCache::isTileBeingRenderedIfSoSubscribe(const TileDesc& tile, const std::shared_ptr<MasterProcessSession> &subscriber)
{
std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex);
- std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight);
+ std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(tile);
if (tileBeingRendered)
{
- Log::debug() << "Tile (" << part << ',' << tilePosX << ',' << tilePosY << ") is already being rendered, subscribing." << Log::end;
+ Log::debug() << "Tile (" << tile.getPart() << ',' << tile.getTilePosX() << ','
+ << tile.getTilePosY() << ") is already being rendered, subscribing." << Log::end;
assert(subscriber->getKind() == LOOLSession::Kind::ToClient);
for (const auto &s : tileBeingRendered->_subscribers)
@@ -463,9 +454,10 @@ bool TileCache::isTileBeingRenderedIfSoSubscribe(int part, int width, int height
}
else
{
- Log::debug() << "Tile (" << part << ',' << tilePosX << ',' << tilePosY << ") needs rendering, subscribing." << Log::end;
+ Log::debug() << "Tile (" << tile.getPart() << ',' << tile.getTilePosX() << ','
+ << tile.getTilePosY() << ") needs rendering, subscribing." << Log::end;
- const std::string cachedName = cacheFileName(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight);
+ const std::string cachedName = cacheFileName(tile);
assert(_tilesBeingRendered.find(cachedName) == _tilesBeingRendered.end());
diff --git a/loolwsd/TileCache.hpp b/loolwsd/TileCache.hpp
index 9afe4c6..8b102e8 100644
--- a/loolwsd/TileCache.hpp
+++ b/loolwsd/TileCache.hpp
@@ -83,7 +83,7 @@ class TileCache
{
struct TileBeingRendered;
- std::shared_ptr<TileBeingRendered> findTileBeingRendered(int part, int width, int height, int tilePosX, int tilePosY, int tileWidth, int tileHeight);
+ std::shared_ptr<TileBeingRendered> findTileBeingRendered(const TileDesc& tile);
public:
/// When the docURL is a non-file:// url, the timestamp has to be provided by the caller.
@@ -94,13 +94,13 @@ public:
TileCache(const TileCache&) = delete;
- bool isTileBeingRenderedIfSoSubscribe(int part, int width, int height, int tilePosX, int tilePosY, int tileWidth, int tileHeight, const std::shared_ptr<MasterProcessSession> &subscriber);
+ bool isTileBeingRenderedIfSoSubscribe(const TileDesc& tile, const std::shared_ptr<MasterProcessSession> &subscriber);
- std::unique_ptr<std::fstream> lookupTile(int part, int width, int height, int tilePosX, int tilePosY, int tileWidth, int tileHeight);
+ std::unique_ptr<std::fstream> lookupTile(const TileDesc& tile);
- void saveTile(int part, int width, int height, int tilePosX, int tilePosY, int tileWidth, int tileHeight, const char *data, size_t size);
+ void saveTile(const TileDesc& tile, const char *data, size_t size);
- void notifyAndRemoveSubscribers(int part, int width, int height, int tilePosX, int tilePosY, int tileWidth, int tileHeight, int id);
+ void notifyAndRemoveSubscribers(const TileDesc& tile);
std::string getTextFile(const std::string& fileName);
@@ -124,7 +124,7 @@ public:
std::unique_lock<std::mutex> getTilesBeingRenderedLock() { return std::unique_lock<std::mutex>(_tilesBeingRenderedMutex); }
- void forgetTileBeingRendered(int part, int width, int height, int tilePosX, int tilePosY, int tileWidth, int tileHeight);
+ void forgetTileBeingRendered(const TileDesc& tile);
private:
void invalidateTiles(int part, int x, int y, int width, int height);
@@ -132,7 +132,7 @@ private:
// Removes the given file from the cache
void removeFile(const std::string& fileName);
- std::string cacheFileName(int part, int width, int height, int tilePosX, int tilePosY, int tileWidth, int tileHeight);
+ std::string cacheFileName(const TileDesc& tile);
bool parseCacheFileName(const std::string& fileName, int& part, int& width, int& height, int& tilePosX, int& tilePosY, int& tileWidth, int& tileHeight);
/// Extract location from fileName, and check if it intersects with [x, y, width, height].
diff --git a/loolwsd/test/TileCacheTests.cpp b/loolwsd/test/TileCacheTests.cpp
index abd8865..8906cda 100644
--- a/loolwsd/test/TileCacheTests.cpp
+++ b/loolwsd/test/TileCacheTests.cpp
@@ -125,18 +125,19 @@ void TileCacheTests::testSimple()
int tilePosY = 0;
int tileWidth = 3840;
int tileHeight = 3840;
+ TileDesc tile(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight);
// No Cache
- auto file = tc.lookupTile(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight);
+ auto file = tc.lookupTile(tile);
CPPUNIT_ASSERT_MESSAGE("found tile when none was expected", !file);
// Cache Tile
const auto size = 1024;
const auto data = genRandomData(size);
- tc.saveTile(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight, data.data(), size);
+ tc.saveTile(tile, data.data(), size);
// Find Tile
- file = tc.lookupTile(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight);
+ file = tc.lookupTile(tile);
CPPUNIT_ASSERT_MESSAGE("tile not found when expected", file && file->is_open());
const auto tileData = readDataFromFile(file);
CPPUNIT_ASSERT_MESSAGE("cached tile corrupted", data == tileData);
@@ -145,7 +146,7 @@ void TileCacheTests::testSimple()
tc.invalidateTiles("invalidatetiles: EMPTY");
// No Cache
- file = tc.lookupTile(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight);
+ file = tc.lookupTile(tile);
CPPUNIT_ASSERT_MESSAGE("found tile when none was expected", !file);
}
More information about the Libreoffice-commits
mailing list