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 bc2d1922e26bcf4e243150c3d439226743c6c2ba Author: Miklos Vajna <[email protected]> AuthorDate: Wed Mar 5 10:21:54 2025 +0100 Commit: Miklos Vajna <[email protected]> CommitDate: Wed Mar 5 17:40:35 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/+/182543 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins diff --git a/desktop/inc/lib/init.hxx b/desktop/inc/lib/init.hxx index 169d7e5b64d4..aa7df17c01bd 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; } + DESKTOP_DLLPUBLIC 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 43fe3a7960ef..6a25358134de 100644 --- a/desktop/qa/desktop_lib/test_desktop_lib.cxx +++ b/desktop/qa/desktop_lib/test_desktop_lib.cxx @@ -178,6 +178,7 @@ public: void testTileInvalidationCompression(); void testPartInInvalidation(); void testBinaryCallback(); + void testOmitInvalidate(); void testInput(); void testRedlineWriter(); void testRedlineCalc(); @@ -250,6 +251,7 @@ public: CPPUNIT_TEST(testTileInvalidationCompression); CPPUNIT_TEST(testPartInInvalidation); CPPUNIT_TEST(testBinaryCallback); + CPPUNIT_TEST(testOmitInvalidate); CPPUNIT_TEST(testInput); CPPUNIT_TEST(testRedlineWriter); CPPUNIT_TEST(testRedlineCalc); @@ -1991,6 +1993,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(); @@ -2005,6 +2008,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(); @@ -2015,6 +2019,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 818f3d3cf176..1bd3e6e2690b 100644 --- a/desktop/source/lib/init.cxx +++ b/desktop/source/lib/init.cxx @@ -1705,7 +1705,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); } @@ -2609,6 +2636,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) { @@ -4434,6 +4468,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*/)
