desktop/inc/lib/init.hxx | 5 ++
desktop/qa/desktop_lib/test_desktop_lib.cxx | 63 ++++++++++++++++++++++++++++
desktop/source/lib/init.cxx | 43 ++++++++++++++++++-
3 files changed, 110 insertions(+), 1 deletion(-)
New commits:
commit 43965ae570ded21653dabb5ec0570f8aaa7c497e
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Wed Mar 5 10:21:54 2025 +0100
Commit: Michael Meeks <michael.me...@collabora.com>
CommitDate: Wed Mar 5 14:24:03 2025 +0100
cool#11254 desktop lok: avoid invalidations if no tiles are sent
If we have not rendered any tiles, then we should not need to emit
invalidations. To generalize that, for each view:
- when there is a clean-start or an EMPTY invalidation: reset a bbox of
all tiles rendered
- whenever we render a new tile we should grow the bbox
- whenever we get an invalidate we should crop it against the bbox
This has to be per-view: view1 may have a different view render state
(e.g. spellcheck on) vs view2, so render in view1 should not influance
invalidations in view2. COOL has a concept of "canonical view ids" (a
group of views with the same render state), but that's not a problem for
us, as long as we always render on the same single view for the group,
the consistency is maintained.
For Calc we also have multiple parts, and Impress additionally have part
modes; the bbox should not be shared for these inside a single view.
Solve this by using a "part -> mode -> bbox" map of maps.
This then requires updating DesktopLOKTest::testBinaryCallback() in
CppunitTest_desktop_lib, because now some invalidates will be only
emitted if there was a paint before.
Change-Id: I5b18330ce56a0200a493d30ad51524bb46e1de02
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/182524
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Michael Meeks <michael.me...@collabora.com>
diff --git a/desktop/inc/lib/init.hxx b/desktop/inc/lib/init.hxx
index 536faf00aff9..427a87a7fc97 100644
--- a/desktop/inc/lib/init.hxx
+++ b/desktop/inc/lib/init.hxx
@@ -115,6 +115,8 @@ namespace desktop {
void setViewId( int viewId ) { m_viewId = viewId; }
+ void tilePainted(int nPart, int nMode, const tools::Rectangle&
rRectangle);
+
// SfxLockCallbackInterface
virtual void libreOfficeKitViewCallback(int nType, const OString&
pPayload) override;
virtual void libreOfficeKitViewCallbackWithViewId(int nType, const
OString& pPayload, int nViewId) override;
@@ -210,6 +212,9 @@ namespace desktop {
std::unordered_map<OString, OString> m_lastStateChange;
std::unordered_map<int, std::unordered_map<int, OString>> m_viewStates;
+ /// BBox of already painted tiles: part number -> part mode ->
rectangle.
+ std::map<int, std::map<int, tools::Rectangle>> m_aPaintedTiles;
+
// For some types only the last message matters (see isUpdatedType())
or only the last message
// per each viewId value matters (see isUpdatedTypePerViewId()), so
instead of using push model
// where we'd get flooded by repeated messages (which might be costly
to generate and process),
diff --git a/desktop/qa/desktop_lib/test_desktop_lib.cxx
b/desktop/qa/desktop_lib/test_desktop_lib.cxx
index f4766980b026..b20533d6eb3b 100644
--- a/desktop/qa/desktop_lib/test_desktop_lib.cxx
+++ b/desktop/qa/desktop_lib/test_desktop_lib.cxx
@@ -184,6 +184,7 @@ public:
void testTileInvalidationCompression();
void testPartInInvalidation();
void testBinaryCallback();
+ void testOmitInvalidate();
void testInput();
void testRedlineWriter();
void testRedlineCalc();
@@ -256,6 +257,7 @@ public:
CPPUNIT_TEST(testTileInvalidationCompression);
CPPUNIT_TEST(testPartInInvalidation);
CPPUNIT_TEST(testBinaryCallback);
+ CPPUNIT_TEST(testOmitInvalidate);
CPPUNIT_TEST(testInput);
CPPUNIT_TEST(testRedlineWriter);
CPPUNIT_TEST(testRedlineCalc);
@@ -1997,6 +1999,7 @@ void DesktopLOKTest::testBinaryCallback()
std::unique_ptr<CallbackFlushHandler> handler(new
CallbackFlushHandler(pDocument, callbackBinaryCallbackTest, ¬ifs));
handler->setViewId(SfxLokHelper::getView());
+ handler->tilePainted(/*nPart=*/INT_MIN, /*nMode=*/0, rect1);
handler->libreOfficeKitViewInvalidateTilesCallback(&rect1, INT_MIN, 0);
Scheduler::ProcessEventsToIdle();
@@ -2011,6 +2014,7 @@ void DesktopLOKTest::testBinaryCallback()
std::unique_ptr<CallbackFlushHandler> handler(new
CallbackFlushHandler(pDocument, callbackBinaryCallbackTest, ¬ifs));
handler->setViewId(SfxLokHelper::getView());
+ handler->tilePainted(/*nPart=*/INT_MIN, /*nMode=*/0, rect1);
handler->libreOfficeKitViewInvalidateTilesCallback(nullptr, INT_MIN,
0);
Scheduler::ProcessEventsToIdle();
@@ -2021,6 +2025,65 @@ void DesktopLOKTest::testBinaryCallback()
}
}
+void DesktopLOKTest::testOmitInvalidate()
+{
+ LibLODocument_Impl* pDocument = loadDoc("blank_text.odt");
+ tools::Rectangle aRectangle{Point(0, 0), Size(10, 10)};
+
+ {
+ // Given a clean state:
+ std::vector<std::tuple<int, std::string>> aCallbacks;
+ std::unique_ptr<CallbackFlushHandler> pHandler(new
CallbackFlushHandler(pDocument, callbackBinaryCallbackTest, &aCallbacks));
+ pHandler->setViewId(0);
+
+ // When emitting just an invalidation:
+ pHandler->libreOfficeKitViewInvalidateTilesCallback(&aRectangle,
/*nPart=*/0, /*nMode=*/0);
+
+ // Then make sure that's filtered out:
+ Scheduler::ProcessEventsToIdle();
+ // Without the accompanying fix in place, this test would have failed
with:
+ // - Expected: 0
+ // - Actual : 1
+ // i.e. invalidation was emitted when we haven't rendered any tiles
yet.
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aCallbacks.size());
+ }
+
+ {
+ // Given a clean state:
+ std::vector<std::tuple<int, std::string>> aCallbacks;
+ std::unique_ptr<CallbackFlushHandler> pHandler(new
CallbackFlushHandler(pDocument, callbackBinaryCallbackTest, &aCallbacks));
+ pHandler->setViewId(0);
+
+ // When emitting an invalidation outside the painted area:
+ pHandler->tilePainted(/*nPart=*/0, /*nMode=*/0, aRectangle);
+ tools::Rectangle aElsewhere{Point(20, 20), Size(10, 10)};
+ pHandler->libreOfficeKitViewInvalidateTilesCallback(&aElsewhere,
/*nPart=*/0, /*nMode=*/0);
+
+ // Then make sure that's filtered out:
+ Scheduler::ProcessEventsToIdle();
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aCallbacks.size());
+ }
+
+ {
+ // Given a clean state:
+ std::vector<std::tuple<int, std::string>> aCallbacks;
+ std::unique_ptr<CallbackFlushHandler> pHandler(new
CallbackFlushHandler(pDocument, callbackBinaryCallbackTest, &aCallbacks));
+ pHandler->setViewId(0);
+
+ // When emitting an invalidation partly outside the painted area:
+ pHandler->tilePainted(/*nPart=*/0, /*nMode=*/0, aRectangle);
+ tools::Rectangle aLarger{Point(0, 0), Size(20, 10)};
+ pHandler->libreOfficeKitViewInvalidateTilesCallback(&aLarger,
/*nPart=*/0, /*nMode=*/0);
+
+ // Then make sure that's cropped:
+ Scheduler::ProcessEventsToIdle();
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aCallbacks.size());
+ CPPUNIT_ASSERT_EQUAL(int(LOK_CALLBACK_INVALIDATE_TILES),
std::get<0>(aCallbacks[0]));
+ // x, y, w, h, part, mode; so this is cropped.
+ CPPUNIT_ASSERT_EQUAL(std::string("0, 0, 9, 9, 0, 0"),
std::get<1>(aCallbacks[0]));
+ }
+}
+
void DesktopLOKTest::testInput()
{
// Load a Writer document, enable change recording and press a key.
diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx
index 9b3de8876ca3..b0ee1d36a431 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -1715,7 +1715,34 @@ void
CallbackFlushHandler::libreOfficeKitViewCallbackWithViewId(int nType, const
void CallbackFlushHandler::libreOfficeKitViewInvalidateTilesCallback(const
tools::Rectangle* pRect, int nPart, int nMode)
{
- CallbackData callbackData(pRect, nPart, nMode);
+ tools::Rectangle& rPaintedTiles = m_aPaintedTiles[nPart][nMode];
+ if (rPaintedTiles.IsEmpty())
+ {
+ // We have not sent any tiles: don't send invalidations.
+ return;
+ }
+
+ tools::Rectangle aRect;
+ if (pRect)
+ {
+ // We got an invalidate: crop it against the bbox.
+ aRect = *pRect;
+ aRect.Intersection(rPaintedTiles);
+ if (aRect.IsEmpty())
+ {
+ return;
+ }
+ }
+ else
+ {
+ // EMPTY invalidation: reset the bbox.
+ rPaintedTiles = tools::Rectangle();
+ // nullptr pRect means: invalidate everything.
+ aRect = RectangleAndPart::emptyAllRectangle;
+ }
+
+ // RectangleAndPart ctor doesn't store &aRect, so this is OK.
+ CallbackData callbackData(&aRect, nPart, nMode);
queue(LOK_CALLBACK_INVALIDATE_TILES, callbackData);
}
@@ -2620,6 +2647,13 @@ void CallbackFlushHandler::removeViewStates(int viewId)
m_viewStates.erase(viewId);
}
+void CallbackFlushHandler::tilePainted(int nPart, int nMode, const
tools::Rectangle& rRectangle)
+{
+ // Painted a new tile: grow the bbox.
+ tools::Rectangle& rPaintedTiles = m_aPaintedTiles[nPart][nMode];
+ rPaintedTiles.Union(rRectangle);
+}
+
static void doc_destroy(LibreOfficeKitDocument *pThis)
{
@@ -4468,6 +4502,13 @@ static void doc_paintPartTile(LibreOfficeKitDocument*
pThis,
}
enableViewCallbacks(pDocument, nOrigViewId);
+
+ auto it = pDocument->mpCallbackFlushHandlers.find(nOrigViewId);
+ if (it != pDocument->mpCallbackFlushHandlers.end())
+ {
+ tools::Rectangle aRectangle{Point(nTilePosX, nTilePosY),
Size(nTileWidth, nTileHeight)};
+ it->second->tilePainted(nPart, nMode, aRectangle);
+ }
}
static int doc_getTileMode(SAL_UNUSED_PARAMETER LibreOfficeKitDocument*
/*pThis*/)
