chart2/qa/extras/data/ods/tdf153706_XY_scatter_chart.ods |binary chart2/qa/extras/uichart.cxx | 54 ++++++++++ chart2/source/controller/chartapiwrapper/ChartDataWrapper.cxx | 10 + chart2/source/controller/chartapiwrapper/ChartDataWrapper.hxx | 11 +- sc/inc/clipcontext.hxx | 5 sc/source/core/data/clipcontext.cxx | 4 sc/source/core/data/column.cxx | 6 - sc/source/core/data/document.cxx | 14 +- sc/source/core/data/drwlayer.cxx | 29 +++++ 9 files changed, 123 insertions(+), 10 deletions(-)
New commits: commit 81c8c95b480ac32a18f8d7d0f4a0e3671b5b8b2b Author: Mike Kaganski <[email protected]> AuthorDate: Sun Mar 3 16:12:55 2024 +0600 Commit: Xisco Fauli <[email protected]> CommitDate: Tue Mar 5 09:32:23 2024 +0100 tdf#153706: do not add categories, when source data doesn't have them lcl_AllOperator is used in XChartDocument::attachData implementation. When a data without existing categories is passed there, like an XY scatter, lcl_AllOperator used to force creation of the categories in the target, by returning 'true' unconditionally from setsCategories. This meant, that a new sequence of numbers starting from 1 was used as X values, and the old X data was interpreted as an extra Y series. This changes lcl_AllOperator::setsCategories to try to check if its data actually contains categories. Thus, XChartDocument::attachData will use categories either when the chart already uses categories, and ChartDataWrapper::applyData detects that using a call to DataSourceHelper::detectRangeSegmentation; or when the new data has it; but not when neither had it. When it's not possible to detect if there were categories in the new data (e.g., with user data), old behavior is used, setting categories. It could be an alternative to detect the chart type using xOldDoc->getDiagram()->getDiagramType() == "com.sun.star.chart.XYDiagram" in XChartDocument::attachData; and then decide to force the creation or not. But it seems hackish, and not really universal: other chart types must be tested (bubble?), no idea how to handle hypothetical cases when applied data contains categories in case of XY chart, etc. Change-Id: I86b34f6799c30b103f7fc6b2faf6ec255a9d137b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164298 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164279 diff --git a/chart2/qa/extras/data/ods/tdf153706_XY_scatter_chart.ods b/chart2/qa/extras/data/ods/tdf153706_XY_scatter_chart.ods new file mode 100644 index 000000000000..2c6153ed6ab2 Binary files /dev/null and b/chart2/qa/extras/data/ods/tdf153706_XY_scatter_chart.ods differ diff --git a/chart2/qa/extras/uichart.cxx b/chart2/qa/extras/uichart.cxx index 84b8658d29ea..2638367c8506 100644 --- a/chart2/qa/extras/uichart.cxx +++ b/chart2/qa/extras/uichart.cxx @@ -436,6 +436,60 @@ CPPUNIT_TEST_FIXTURE(Chart2UiChartTest, testTdf158223) } } +CPPUNIT_TEST_FIXTURE(Chart2UiChartTest, testTdf153706) +{ + // Load a spreadsheet with a to-page XY scatter chart with the sheet as data source + loadFromFile(u"ods/tdf153706_XY_scatter_chart.ods"); + + // Select the cell range around the chart, and copy the range to clipboard, including the chart + dispatchCommand(mxComponent, u".uno:GoToCell"_ustr, + { comphelper::makePropertyValue(u"ToPoint"_ustr, u"D1:K23"_ustr) }); + dispatchCommand(mxComponent, u".uno:Copy"_ustr, {}); + + // create a new document + load(u"private:factory/scalc"_ustr); + + // Paste; this must create a chart with own data source having a proper copy of the data + dispatchCommand(mxComponent, u".uno:Paste"_ustr, {}); + + css::uno::Reference xChartDoc(getChartDocFromSheet(0, mxComponent), css::uno::UNO_SET_THROW); + auto xDataArray(xChartDoc->getDataProvider().queryThrow<chart::XChartDataArray>()); + + css::uno::Sequence<Sequence<double>> aData = xDataArray->getData(); + CPPUNIT_ASSERT_EQUAL(sal_Int32(3), aData.getLength()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), aData[0].getLength()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), aData[1].getLength()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), aData[2].getLength()); + CPPUNIT_ASSERT_EQUAL(2.0, aData[0][0]); + CPPUNIT_ASSERT_EQUAL(3.0, aData[0][1]); + CPPUNIT_ASSERT_EQUAL(3.0, aData[1][0]); + CPPUNIT_ASSERT_EQUAL(2.0, aData[1][1]); + CPPUNIT_ASSERT_EQUAL(4.0, aData[2][0]); + CPPUNIT_ASSERT_EQUAL(1.0, aData[2][1]); + + // Without the fix, this would fail with + // - Expected: 1 + // - Actual : 2 + // i.e., the X values were treated as another Y series + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), getNumberOfDataSeries(xChartDoc)); + + auto xSeries(getDataSeriesFromDoc(xChartDoc, 0).queryThrow<chart2::data::XDataSource>()); + auto sequences = xSeries->getDataSequences(); + // Without the fix, this would fail with + // - Expected: 2 + // - Actual : 1 + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), sequences.getLength()); + + auto propX(sequences[0]->getValues().queryThrow<beans::XPropertySet>()); + // Without the fix, this would fail with + // - Expected: values-x + // - Actual : values-y + CPPUNIT_ASSERT_EQUAL(u"values-x"_ustr, propX->getPropertyValue("Role").get<OUString>()); + + auto propY(sequences[1]->getValues().queryThrow<beans::XPropertySet>()); + CPPUNIT_ASSERT_EQUAL(u"values-y"_ustr, propY->getPropertyValue("Role").get<OUString>()); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/chart2/source/controller/chartapiwrapper/ChartDataWrapper.cxx b/chart2/source/controller/chartapiwrapper/ChartDataWrapper.cxx index 84b98f0d8018..7988ef90df94 100644 --- a/chart2/source/controller/chartapiwrapper/ChartDataWrapper.cxx +++ b/chart2/source/controller/chartapiwrapper/ChartDataWrapper.cxx @@ -121,6 +121,11 @@ struct lcl_AllOperator : public lcl_Operator virtual bool setsCategories( bool /*bDataInColumns*/ ) override { + // Do not force creation of categories, when original has no categories + if (auto pDataWrapper = dynamic_cast<const ChartDataWrapper*>(m_xDataToApply.get())) + if (auto xChartModel = pDataWrapper->getChartModel()) + if (auto xDiagram = xChartModel->getFirstChartDiagram()) + return xDiagram->getCategories().is(); return true; } @@ -698,6 +703,11 @@ css::uno::Sequence< OUString > SAL_CALL ChartDataWrapper::getSupportedServiceNam }; } +rtl::Reference<ChartModel> ChartDataWrapper::getChartModel() const +{ + return m_spChart2ModelContact->getDocumentModel(); +} + } // namespace chart /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/chart2/source/controller/chartapiwrapper/ChartDataWrapper.hxx b/chart2/source/controller/chartapiwrapper/ChartDataWrapper.hxx index 3c6602d4e028..9a44e53b8b12 100644 --- a/chart2/source/controller/chartapiwrapper/ChartDataWrapper.hxx +++ b/chart2/source/controller/chartapiwrapper/ChartDataWrapper.hxx @@ -20,6 +20,8 @@ #include <cppuhelper/implbase.hxx> #include <comphelper/interfacecontainer4.hxx> +#include <rtl/ref.hxx> + #include <com/sun/star/chart2/XAnyDescriptionAccess.hpp> #include <com/sun/star/chart/XDateCategories.hpp> #include <com/sun/star/lang/XComponent.hpp> @@ -28,7 +30,11 @@ #include <memory> -namespace chart::wrapper +namespace chart +{ +class ChartModel; + +namespace wrapper { class Chart2ModelContact; @@ -53,6 +59,8 @@ public: virtual sal_Bool SAL_CALL supportsService( const OUString& ServiceName ) override; virtual css::uno::Sequence< OUString > SAL_CALL getSupportedServiceNames() override; + rtl::Reference<ChartModel> getChartModel() const; + private: // ____ XDateCategories ____ virtual css::uno::Sequence< double > SAL_CALL getDateCategories() override; @@ -113,5 +121,6 @@ private: }; } // namespace chart::wrapper +} // namespace chart /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ commit 44141fbb32c181a1fea2730b194b2664521d0a79 Author: Mike Kaganski <[email protected]> AuthorDate: Sun Mar 3 02:04:24 2024 +0600 Commit: Xisco Fauli <[email protected]> CommitDate: Tue Mar 5 09:32:11 2024 +0100 tdf#99969: make sure to copy the chart source ranges to clipboard ... even when they are outside of the copied cell range. Otherwise, it is impossible to transfer the missing data when switching to own data on paste. The copy of the missing ranges avoids copying cell attributes, for performance reasons, but also to avoid overwriting the attributes of already copied cells. Otherwise, ScDrawLayer::CopyToClip would need the bKeepScenarioFlags, or the CopyToClipContext used in the caller ScDocument::CopyToClip, for consistent copy; or a method to avoid overwriting already copied cells (this change simply copies all chart ranges, withiut checking if they were copied already). Change-Id: Id02e0c20517e7e8a17bb0a31d1b230196cda1a58 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164294 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164397 diff --git a/sc/inc/clipcontext.hxx b/sc/inc/clipcontext.hxx index b3ce874a6a7f..d93e6acb453d 100644 --- a/sc/inc/clipcontext.hxx +++ b/sc/inc/clipcontext.hxx @@ -161,12 +161,15 @@ public: class CopyToClipContext final : public ClipContextBase { bool mbKeepScenarioFlags:1; + bool mbCopyChartRanges : 1 = false; // Copying ranges not included in selection: + // only copy data, not cell attributes public: - CopyToClipContext(ScDocument& rDoc, bool bKeepScenarioFlags); + CopyToClipContext(ScDocument& rDoc, bool bKeepScenarioFlags, bool bCopyChartRanges = false); virtual ~CopyToClipContext() override; bool isKeepScenarioFlags() const; + bool isCopyChartRanges() const { return mbCopyChartRanges; } }; class CopyToDocContext final : public ClipContextBase diff --git a/sc/source/core/data/clipcontext.cxx b/sc/source/core/data/clipcontext.cxx index ce6974d42334..be145f995451 100644 --- a/sc/source/core/data/clipcontext.cxx +++ b/sc/source/core/data/clipcontext.cxx @@ -403,8 +403,8 @@ bool CopyFromClipContext::isDateCell( const ScColumn& rCol, SCROW nRow ) const } CopyToClipContext::CopyToClipContext( - ScDocument& rDoc, bool bKeepScenarioFlags) : - ClipContextBase(rDoc), mbKeepScenarioFlags(bKeepScenarioFlags) {} + ScDocument& rDoc, bool bKeepScenarioFlags, bool bCopyChartRanges) : + ClipContextBase(rDoc), mbKeepScenarioFlags(bKeepScenarioFlags), mbCopyChartRanges(bCopyChartRanges) {} CopyToClipContext::~CopyToClipContext() {} diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx index 5bae9b7b9de7..ceadfc2f3e75 100644 --- a/sc/source/core/data/column.cxx +++ b/sc/source/core/data/column.cxx @@ -880,14 +880,16 @@ public: void ScColumn::CopyToClip( sc::CopyToClipContext& rCxt, SCROW nRow1, SCROW nRow2, ScColumn& rColumn ) const { - pAttrArray->CopyArea( nRow1, nRow2, 0, *rColumn.pAttrArray, - rCxt.isKeepScenarioFlags() ? (ScMF::All & ~ScMF::Scenario) : ScMF::All ); + if (!rCxt.isCopyChartRanges()) // No need to copy attributes for chart ranges + pAttrArray->CopyArea( nRow1, nRow2, 0, *rColumn.pAttrArray, + rCxt.isKeepScenarioFlags() ? (ScMF::All & ~ScMF::Scenario) : ScMF::All ); { CopyToClipHandler aFunc(GetDoc(), *this, rColumn, rCxt.getBlockPosition(rColumn.nTab, rColumn.nCol)); sc::ParseBlock(maCells.begin(), maCells, aFunc, nRow1, nRow2); } + if (!rCxt.isCopyChartRanges()) // No need to copy attributes for chart ranges { CopyTextAttrToClipHandler aFunc(rColumn.maCellTextAttrs); sc::ParseBlock(maCellTextAttrs.begin(), maCellTextAttrs, aFunc, nRow1, nRow2); diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx index 15bb28fe61f1..2bca3a798fb1 100644 --- a/sc/source/core/data/document.cxx +++ b/sc/source/core/data/document.cxx @@ -2218,6 +2218,7 @@ void ScDocument::CopyToClip(const ScClipParam& rClipParam, sc::CopyToClipContext aCxt(*pClipDoc, bKeepScenarioFlags); CopyRangeNamesToClip(pClipDoc, aClipRange, pMarks); + // 1. Copy selected cells for (SCTAB i = 0; i < nEndTab; ++i) { if (!maTabs[i] || i >= pClipDoc->GetTableCount() || !pClipDoc->maTabs[i]) @@ -2227,12 +2228,17 @@ void ScDocument::CopyToClip(const ScClipParam& rClipParam, continue; maTabs[i]->CopyToClip(aCxt, rClipParam.maRanges, pClipDoc->maTabs[i].get()); + } - if (mpDrawLayer && bIncludeObjects) + // 2. Copy drawing objects in the selection. Do in after the first "copy cells" pass, because + // the embedded objects (charts) coud reference cells from tabs not (yet) copied; doing it now + // allows to know what is already copied, to not owerwrite attributes of already copied data. + if (mpDrawLayer && bIncludeObjects) + { + for (SCTAB i = 0; i < nEndTab; ++i) { - // also copy drawing objects - tools::Rectangle aObjRect = GetMMRect( - aClipRange.aStart.Col(), aClipRange.aStart.Row(), aClipRange.aEnd.Col(), aClipRange.aEnd.Row(), i); + tools::Rectangle aObjRect = GetMMRect(aClipRange.aStart.Col(), aClipRange.aStart.Row(), + aClipRange.aEnd.Col(), aClipRange.aEnd.Row(), i); mpDrawLayer->CopyToClip(pClipDoc, i, aObjRect); } } diff --git a/sc/source/core/data/drwlayer.cxx b/sc/source/core/data/drwlayer.cxx index b4fed97fc391..d5f2cc09b979 100644 --- a/sc/source/core/data/drwlayer.cxx +++ b/sc/source/core/data/drwlayer.cxx @@ -88,6 +88,7 @@ #include <docpool.hxx> #include <detfunc.hxx> #include <basegfx/matrix/b2dhommatrix.hxx> +#include <clipcontext.hxx> #include <clipparam.hxx> #include <memory> @@ -1853,6 +1854,34 @@ void ScDrawLayer::CopyToClip( ScDocument* pClipDoc, SCTAB nTab, const tools::Rec pDestPage->InsertObject(pNewObject.get()); + // Store the chart's source data to the clipboad document, even when it's out of the + // copied range. It will be ignored when pasted to the same document; when pasted to + // another document, ScDocument::mpClipParam will provide the actually copied ranges, + // and the data copied here will be used to break connection and switch to own data + // in ScDrawLayer::CopyFromClip. + if (xOldChart && !xOldChart->hasInternalDataProvider()) + { + sc::CopyToClipContext aCxt(*pClipDoc, false, true); + OUString aChartName = static_cast<SdrOle2Obj*>(pOldObject)->GetPersistName(); + std::vector<ScRangeList> aRangesVector; + pDoc->GetChartRanges(aChartName, aRangesVector, *pDoc); + for (const ScRangeList& ranges : aRangesVector) + { + for (const ScRange& r : ranges) + { + for (SCTAB i = r.aStart.Tab(); i <= r.aEnd.Tab(); ++i) + { + ScTable* pTab = pDoc->FetchTable(i); + ScTable* pClipTab = pClipDoc->FetchTable(i); + if (!pTab || !pClipTab) + continue; + pTab->CopyToClip(aCxt, r.aStart.Col(), r.aStart.Row(), r.aEnd.Col(), + r.aEnd.Row(), pClipTab); + } + } + } + } + // no undo needed in clipboard document // charts are not updated }
