desktop/inc/lib/init.hxx | 9 --
desktop/qa/desktop_lib/test_desktop_lib.cxx | 97 ----------------------------
desktop/source/lib/init.cxx | 70 --------------------
3 files changed, 1 insertion(+), 175 deletions(-)
New commits:
commit 2c675a489aa3e8991f8f54d408b294f54d66e2c4
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Thu Mar 27 08:37:36 2025 +0100
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Thu Mar 27 09:53:16 2025 +0100
cool#11289 revert invalidation optimization on co-24.04
Revert "cool#11289 desktop lok: don't assume all views invalidate the same
area"
This reverts commit 6f2b99b10f27a8523864d26028b10b9c478a7d16.
Revert "cool#11254 desktop lok: avoid invalidations if no tiles are sent"
This reverts commit 43965ae570ded21653dabb5ec0570f8aaa7c497e.
Too risky for co-24.04, do it on co-25.04 only.
Change-Id: Iac0191def4cd0fd419a573bab2bb01427f537883
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183362
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
Tested-by: Caolán McNamara <caolan.mcnam...@collabora.com>
diff --git a/desktop/inc/lib/init.hxx b/desktop/inc/lib/init.hxx
index 0341cc0aa9c8..536faf00aff9 100644
--- a/desktop/inc/lib/init.hxx
+++ b/desktop/inc/lib/init.hxx
@@ -115,9 +115,6 @@ namespace desktop {
void setViewId( int viewId ) { m_viewId = viewId; }
- void tilePainted(int nPart, int nMode, const tools::Rectangle&
rRectangle);
- const OString& getViewRenderState() const { return m_aViewRenderState;
}
-
// SfxLockCallbackInterface
virtual void libreOfficeKitViewCallback(int nType, const OString&
pPayload) override;
virtual void libreOfficeKitViewCallbackWithViewId(int nType, const
OString& pPayload, int nViewId) override;
@@ -213,9 +210,6 @@ 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),
@@ -236,7 +230,6 @@ namespace desktop {
boost::container::flat_map<int, std::vector<PerViewIdData>>
m_updatedTypesPerViewId; // key is view, index is type
LibreOfficeKitDocument* m_pDocument;
- OString m_aViewRenderState;
int m_viewId = -1; // view id of the associated SfxViewShell
LibreOfficeKitCallback m_pCallback;
ImplSVEvent* m_pFlushEvent;
@@ -258,8 +251,6 @@ namespace desktop {
explicit LibLODocument_Impl(css::uno::Reference<css::lang::XComponent>
xComponent,
int nDocumentId);
~LibLODocument_Impl();
-
- void updateViewsForPaintedTile(int nOrigViewId, int nPart, int nMode,
const tools::Rectangle& rRectangle);
};
struct DESKTOP_DLLPUBLIC LibLibreOffice_Impl : public _LibreOfficeKit
diff --git a/desktop/qa/desktop_lib/test_desktop_lib.cxx
b/desktop/qa/desktop_lib/test_desktop_lib.cxx
index 088026e52685..f4766980b026 100644
--- a/desktop/qa/desktop_lib/test_desktop_lib.cxx
+++ b/desktop/qa/desktop_lib/test_desktop_lib.cxx
@@ -184,8 +184,6 @@ public:
void testTileInvalidationCompression();
void testPartInInvalidation();
void testBinaryCallback();
- void testOmitInvalidate();
- void test2ViewsOmitInvalidate();
void testInput();
void testRedlineWriter();
void testRedlineCalc();
@@ -258,8 +256,6 @@ public:
CPPUNIT_TEST(testTileInvalidationCompression);
CPPUNIT_TEST(testPartInInvalidation);
CPPUNIT_TEST(testBinaryCallback);
- CPPUNIT_TEST(testOmitInvalidate);
- CPPUNIT_TEST(test2ViewsOmitInvalidate);
CPPUNIT_TEST(testInput);
CPPUNIT_TEST(testRedlineWriter);
CPPUNIT_TEST(testRedlineCalc);
@@ -2001,7 +1997,6 @@ 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();
@@ -2016,7 +2011,6 @@ 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();
@@ -2027,97 +2021,6 @@ 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::test2ViewsOmitInvalidate()
-{
- // Given a document with 2 views:
- LibLODocument_Impl* pDocument = loadDoc("blank_text.odt");
- std::vector<std::tuple<int, std::string>> aCallbacks1;
- std::shared_ptr<CallbackFlushHandler> pHandler1(new
CallbackFlushHandler(pDocument, callbackBinaryCallbackTest, &aCallbacks1));
- pHandler1->setViewId(0);
- pDocument->mpCallbackFlushHandlers[0] = pHandler1;
- std::vector<std::tuple<int, std::string>> aCallbacks2;
- std::shared_ptr<CallbackFlushHandler> pHandler2(new
CallbackFlushHandler(pDocument, callbackBinaryCallbackTest, &aCallbacks2));
- pHandler2->setViewId(1);
- pDocument->mpCallbackFlushHandlers[1] = pHandler2;
-
- // When painting a tile for a larger area, and then 2 invalidates: the
first view gets a smaller
- // invalidate, the second view gets a larger invalidate:
- tools::Rectangle aPaint{Point(0, 0), Size(20, 10)};
- pDocument->updateViewsForPaintedTile(/*nOrigViewId=*/0, /*nPart=*/0,
/*nMode=*/0, aPaint);
- tools::Rectangle aSmaller{Point(0, 0), Size(10, 10)};
- pHandler1->libreOfficeKitViewInvalidateTilesCallback(&aSmaller,
/*nPart=*/0, /*nMode=*/0);
- tools::Rectangle aLarger{Point(0, 0), Size(20, 10)};
- pHandler2->libreOfficeKitViewInvalidateTilesCallback(&aLarger,
/*nPart=*/0, /*nMode=*/0);
-
- // Then make sure this larger invalidate for the 2nd view is not lost:
- Scheduler::ProcessEventsToIdle();
- // Without the accompanying fix in place, this test would have failed with:
- // - Expected: 1
- // - Actual : 0
- // i.e. the 2nd view's (larger) invalidate was lost.
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aCallbacks2.size());
- CPPUNIT_ASSERT_EQUAL(std::string("0, 0, 19, 9, 0, 0"),
std::get<1>(aCallbacks2[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 8a18622b79f2..9b3de8876ca3 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -1627,12 +1627,6 @@
CallbackFlushHandler::CallbackFlushHandler(LibreOfficeKitDocument* pDocument, Li
m_states.emplace(LOK_CALLBACK_RULER_UPDATE, "NIL"_ostr);
m_states.emplace(LOK_CALLBACK_VERTICAL_RULER_UPDATE, "NIL"_ostr);
m_states.emplace(LOK_CALLBACK_STATUS_INDICATOR_SET_VALUE, "NIL"_ostr);
-
- if (char* pViewRenderState =
pDocument->pClass->getCommandValues(pDocument, ".uno:ViewRenderState"))
- {
- m_aViewRenderState = pViewRenderState;
- free(pViewRenderState);
- }
}
void CallbackFlushHandler::stop()
@@ -1721,34 +1715,7 @@ void
CallbackFlushHandler::libreOfficeKitViewCallbackWithViewId(int nType, const
void CallbackFlushHandler::libreOfficeKitViewInvalidateTilesCallback(const
tools::Rectangle* pRect, int nPart, int 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);
+ CallbackData callbackData(pRect, nPart, nMode);
queue(LOK_CALLBACK_INVALIDATE_TILES, callbackData);
}
@@ -1837,10 +1804,6 @@ void CallbackFlushHandler::queue(const int type,
CallbackData& aCallbackData)
{
bIsComment = true;
}
- else if (type == LOK_CALLBACK_VIEW_RENDER_STATE)
- {
- m_aViewRenderState = aCallbackData.getPayload();
- }
if (callbacksDisabled() && !bIsChartActive && !bIsComment)
{
@@ -2657,13 +2620,6 @@ 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)
{
@@ -4512,30 +4468,6 @@ static void doc_paintPartTile(LibreOfficeKitDocument*
pThis,
}
enableViewCallbacks(pDocument, nOrigViewId);
-
- // Inform all views with the same view render state about the paint, so
they know if makes sense
- // to invalidate those areas later.
- tools::Rectangle aRectangle{Point(nTilePosX, nTilePosY), Size(nTileWidth,
nTileHeight)};
- pDocument->updateViewsForPaintedTile(nOrigViewId, nPart, nMode,
aRectangle);
-}
-
-void LibLODocument_Impl::updateViewsForPaintedTile(int nOrigViewId, int nPart,
int nMode, const tools::Rectangle& rRectangle)
-{
- auto it = mpCallbackFlushHandlers.find(nOrigViewId);
- if (it == mpCallbackFlushHandlers.end())
- {
- return;
- }
-
- const OString& rViewRenderState = it->second->getViewRenderState();
- for (const auto& rHandler : mpCallbackFlushHandlers)
- {
- if (rHandler.second->getViewRenderState() != rViewRenderState)
- {
- continue;
- }
- rHandler.second->tilePainted(nPart, nMode, rRectangle);
- }
}
static int doc_getTileMode(SAL_UNUSED_PARAMETER LibreOfficeKitDocument*
/*pThis*/)
