desktop/inc/lib/init.hxx | 4 +++
desktop/qa/desktop_lib/test_desktop_lib.cxx | 34 +++++++++++++++++++++++++++
desktop/source/lib/init.cxx | 35 ++++++++++++++++++++++++----
3 files changed, 69 insertions(+), 4 deletions(-)
New commits:
commit 33b411c51578d47c84b4fd1d0f9176bd01744d5d
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Fri Mar 7 09:05:55 2025 +0100
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Fri Mar 7 12:08:04 2025 +0100
cool#11289 desktop lok: don't assume all views invalidate the same area
Have 2 Calc views, go to e.g. F30 in the first view and F31 in the
second view. Type enough in the 2nd view that the text goes outside the
cell, to a next tile, e.g. till the column I area is reached; text
doesn't show up in the new tile area on typing.
This went wrong in commit 43965ae570ded21653dabb5ec0570f8aaa7c497e
(cool#11254 desktop lok: avoid invalidations if no tiles are sent,
2025-03-05), because it assumed that all views get the same
invalidations, but they do not in this case:
debug:12008:11949: ScGridWindow::LogicInvalidate: view #5, rect is
6670x345@(20452,14102)
debug:12008:11949: ScGridWindow::LogicInvalidate: view #4, rect is
2144x372@(20452,14102)
debug:12008:11949: ScGridWindow::LogicInvalidate: view #5, rect is
6670x345@(20452,14102)
debug:12008:11949: ScGridWindow::LogicInvalidate: view #4, rect is
2144x372@(20452,14102)
debug:12008:11949: ScGridWindow::PaintTile: view #4, x is 11520, y is 7680,
w is 3840, h is 3840
Fix the problem by tracking paints in all views of a view group sharing
the same render state ("have the same canonicalviewid").
Getting the view render state on tile paint for each view would be
expensive, so get that once and then subscribe to the update callback.
The view render state is then still compared as string -- if that shows
up in a profile, we could switch to integer-based tracking instead with
some added complexity ("canonical view id").
Change-Id: I151b1ff9713ea13f2ee218b4cb44e4b0a227abcb
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/182615
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
diff --git a/desktop/inc/lib/init.hxx b/desktop/inc/lib/init.hxx
index aa7df17c01bd..e42f378f757e 100644
--- a/desktop/inc/lib/init.hxx
+++ b/desktop/inc/lib/init.hxx
@@ -116,6 +116,7 @@ namespace desktop {
void setViewId( int viewId ) { m_viewId = viewId; }
DESKTOP_DLLPUBLIC 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;
@@ -235,6 +236,7 @@ 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;
@@ -256,6 +258,8 @@ 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 6a25358134de..bb957e0611a5 100644
--- a/desktop/qa/desktop_lib/test_desktop_lib.cxx
+++ b/desktop/qa/desktop_lib/test_desktop_lib.cxx
@@ -179,6 +179,7 @@ public:
void testPartInInvalidation();
void testBinaryCallback();
void testOmitInvalidate();
+ void test2ViewsOmitInvalidate();
void testInput();
void testRedlineWriter();
void testRedlineCalc();
@@ -252,6 +253,7 @@ public:
CPPUNIT_TEST(testPartInInvalidation);
CPPUNIT_TEST(testBinaryCallback);
CPPUNIT_TEST(testOmitInvalidate);
+ CPPUNIT_TEST(test2ViewsOmitInvalidate);
CPPUNIT_TEST(testInput);
CPPUNIT_TEST(testRedlineWriter);
CPPUNIT_TEST(testRedlineCalc);
@@ -2078,6 +2080,38 @@ void DesktopLOKTest::testOmitInvalidate()
}
}
+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 1bd3e6e2690b..328a3674021b 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -1617,6 +1617,12 @@
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()
@@ -1821,6 +1827,10 @@ 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)
{
@@ -4469,11 +4479,28 @@ static void doc_paintPartTile(LibreOfficeKitDocument*
pThis,
enableViewCallbacks(pDocument, nOrigViewId);
- auto it = pDocument->mpCallbackFlushHandlers.find(nOrigViewId);
- if (it != pDocument->mpCallbackFlushHandlers.end())
+ // 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())
{
- tools::Rectangle aRectangle{Point(nTilePosX, nTilePosY),
Size(nTileWidth, nTileHeight)};
- it->second->tilePainted(nPart, nMode, aRectangle);
+ return;
+ }
+
+ const OString& rViewRenderState = it->second->getViewRenderState();
+ for (const auto& rHandler : mpCallbackFlushHandlers)
+ {
+ if (rHandler.second->getViewRenderState() != rViewRenderState)
+ {
+ continue;
+ }
+ rHandler.second->tilePainted(nPart, nMode, rRectangle);
}
}
