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 7ee444ff8da4cad9eafc1668e01a6cd6ef7e1b9c
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:11:16 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 e9a66a1510ec..468e2ee38633 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; }
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 e748997eb326..7e5f864e862a 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -1618,6 +1618,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()
@@ -1822,6 +1828,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)
{
@@ -4473,11 +4483,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);
}
}
