chart2/qa/extras/chart2import2.cxx | 16 ++ chart2/qa/extras/data/xlsx/DataTable-MultipleLegendEntriesForOneDataSeries.xlsx |binary chart2/source/view/charttypes/VSeriesPlotter.cxx | 79 ---------- chart2/source/view/main/DataTableView.cxx | 30 ++- 4 files changed, 38 insertions(+), 87 deletions(-)
New commits: commit ae60b32042f0702c9eeee8d4d34a36d8f6192deb Author: Tomaž Vajngerl <tomaz.vajng...@collabora.co.uk> AuthorDate: Mon Aug 22 16:46:41 2022 +0200 Commit: Tomaž Vajngerl <qui...@gmail.com> CommitDate: Fri Aug 26 12:33:52 2022 +0200 chart2: always use the series for the keys (symbols) in data table In special case the legend keys are not series, but can be each value of a series. This can't be done for the data table where so this has been disabled. This caused a crash when positioning the symbols, as in some cases we got 0 symbols (due to another bug that didn't put the symbols into the collection) or more symbols that there were series (and the symbols couldn't be positioned correctly). With this change we now get the correct number of symbols for the data table. Also added the test document and test to open the document, to make sure we don't crash anymore. Change-Id: Idbebdd5295595a823b5420958e803b13258df035 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138697 Tested-by: Jenkins Reviewed-by: Tomaž Vajngerl <qui...@gmail.com> (cherry picked from commit e5583150a3953f08782865e0f90d41f5c35cc6c5) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138806 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> diff --git a/chart2/qa/extras/chart2import2.cxx b/chart2/qa/extras/chart2import2.cxx index 95ee2428b47d..596531990e36 100644 --- a/chart2/qa/extras/chart2import2.cxx +++ b/chart2/qa/extras/chart2import2.cxx @@ -76,6 +76,7 @@ public: void testTdf121281(); void testTdf139658(); void testTdf146066(); + void testChartDataTableWithMultipleLegendEntriesForOneDataSeries(); CPPUNIT_TEST_SUITE(Chart2ImportTest2); @@ -116,6 +117,7 @@ public: CPPUNIT_TEST(testTdf121281); CPPUNIT_TEST(testTdf139658); CPPUNIT_TEST(testTdf146066); + CPPUNIT_TEST(testChartDataTableWithMultipleLegendEntriesForOneDataSeries); CPPUNIT_TEST_SUITE_END(); }; @@ -922,6 +924,20 @@ void Chart2ImportTest2::testTdf146066() CPPUNIT_ASSERT_EQUAL(OUString("35"), xLabel7->getString()); } +void Chart2ImportTest2::testChartDataTableWithMultipleLegendEntriesForOneDataSeries() +{ + load(u"/chart2/qa/extras/data/xlsx/", u"DataTable-MultipleLegendEntriesForOneDataSeries.xlsx"); + // Loading this file caused a crash in the data table code + + Reference<chart::XChartDocument> xChartDoc(getChartDocFromSheet(0, mxComponent), + UNO_QUERY_THROW); + Reference<drawing::XDrawPageSupplier> xDrawPageSupplier(xChartDoc, UNO_QUERY_THROW); + Reference<drawing::XDrawPage> xDrawPage(xDrawPageSupplier->getDrawPage(), UNO_SET_THROW); + Reference<drawing::XShapes> xShapes(xDrawPage->getByIndex(0), UNO_QUERY_THROW); + Reference<drawing::XShape> xDataTableShape = getShapeByName(xShapes, "CID/D=0:DataTable="); + CPPUNIT_ASSERT(xDataTableShape.is()); +} + CPPUNIT_TEST_SUITE_REGISTRATION(Chart2ImportTest2); CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/chart2/qa/extras/data/xlsx/DataTable-MultipleLegendEntriesForOneDataSeries.xlsx b/chart2/qa/extras/data/xlsx/DataTable-MultipleLegendEntriesForOneDataSeries.xlsx new file mode 100644 index 000000000000..b077fd2e0315 Binary files /dev/null and b/chart2/qa/extras/data/xlsx/DataTable-MultipleLegendEntriesForOneDataSeries.xlsx differ diff --git a/chart2/source/view/charttypes/VSeriesPlotter.cxx b/chart2/source/view/charttypes/VSeriesPlotter.cxx index 0cd15188e84f..3dafa94fd99d 100644 --- a/chart2/source/view/charttypes/VSeriesPlotter.cxx +++ b/chart2/source/view/charttypes/VSeriesPlotter.cxx @@ -2538,11 +2538,6 @@ std::vector<ViewLegendSymbol> VSeriesPlotter::createSymbols(const awt::Size& rEn if (!pSeries) continue; - if (!pSeries->getPropertiesOfSeries()->getPropertyValue("ShowLegendEntry").get<sal_Bool>()) - { - continue; - } - std::vector<ViewLegendSymbol> aSeriesSymbols = createSymbolsForSeries(rEntryKeyAspectRatio, *pSeries, xTarget, xShapeFactory, xContext); //add series entries to the result now @@ -2924,77 +2919,15 @@ std::vector<ViewLegendSymbol> VSeriesPlotter::createSymbolsForSeries( try { ViewLegendSymbol aEntry; - bool bVaryColorsByPoint = rSeries.isVaryColorsByPoint(); - bool bIsPie = m_xChartTypeModel->getChartType().equalsIgnoreAsciiCase(CHART2_SERVICE_NAME_CHARTTYPE_PIE); - try - { - if (bIsPie) - { - bool bDonut = false; - if ((m_xChartTypeModelProps->getPropertyValue("UseRings") >>= bDonut) && bDonut) - bIsPie = false; - } - } - catch (const uno::Exception&) - { - } - - if (bVaryColorsByPoint || bIsPie) - { - Sequence< OUString > aCategoryNames; - if (m_pExplicitCategoriesProvider) - aCategoryNames = m_pExplicitCategoriesProvider->getSimpleCategories(); - Sequence<sal_Int32> deletedLegendEntries; - try - { - rSeries.getPropertiesOfSeries()->getPropertyValue("DeletedLegendEntries") >>= deletedLegendEntries; - } - catch (const uno::Exception&) - { - } - - for (sal_Int32 nIdx=0; nIdx < aCategoryNames.getLength(); ++nIdx) - { - bool deletedLegendEntry = false; - for (const auto& deletedLegendEntryIdx : std::as_const(deletedLegendEntries)) - { - if (nIdx == deletedLegendEntryIdx) - { - deletedLegendEntry = true; - break; - } - } - if (deletedLegendEntry) - continue; - - // symbol - uno::Reference< drawing::XShapes > xSymbolGroup( ShapeFactory::getOrCreateShapeFactory(xShapeFactory)->createGroup2D( xTarget )); + // symbol + uno::Reference< drawing::XShapes > xSymbolGroup( ShapeFactory::getOrCreateShapeFactory(xShapeFactory)->createGroup2D(xTarget)); - // create the symbol - Reference< drawing::XShape > xShape( createLegendSymbolForSeries( - rEntryKeyAspectRatio, rSeries, xSymbolGroup, xShapeFactory ) ); + // create the symbol + Reference<drawing::XShape> xShape(createLegendSymbolForSeries(rEntryKeyAspectRatio, rSeries, xSymbolGroup, xShapeFactory)); - // set CID to symbol for selection - if( xShape.is()) - { - aEntry.aSymbol.set(xSymbolGroup, uno::UNO_QUERY); - } - } - } - else + if (xShape.is()) { - // symbol - uno::Reference< drawing::XShapes > xSymbolGroup( ShapeFactory::getOrCreateShapeFactory(xShapeFactory)->createGroup2D(xTarget)); - - // create the symbol - Reference< drawing::XShape > xShape( createLegendSymbolForSeries( - rEntryKeyAspectRatio, rSeries, xSymbolGroup, xShapeFactory ) ); - - // set CID to symbol for selection - if( xShape.is()) - { - aEntry.aSymbol.set( xSymbolGroup, uno::UNO_QUERY ); - } + aEntry.aSymbol.set(xSymbolGroup, uno::UNO_QUERY); } } catch (const uno::Exception &) diff --git a/chart2/source/view/main/DataTableView.cxx b/chart2/source/view/main/DataTableView.cxx index 636690cb4e48..ea06de875ba8 100644 --- a/chart2/source/view/main/DataTableView.cxx +++ b/chart2/source/view/main/DataTableView.cxx @@ -404,7 +404,8 @@ void DataTableView::createShapes(basegfx::B2DVector const& rStart, basegfx::B2DV if (bKeys) { xCellPropertySet->setPropertyValue( - "ParaLeftMargin", uno::makeAny(nMaxSymbolWidth + sal_Int32(2 * constSymbolMargin))); + "ParaLeftMargin", + uno::makeAny(nMaxSymbolWidth + sal_Int32(2 * constSymbolMargin))); } } nRow++; @@ -479,21 +480,22 @@ void DataTableView::createShapes(basegfx::B2DVector const& rStart, basegfx::B2DV for (sal_Int32 i = 0; i < xTableRows->getCount(); i++) { sal_Int32 nSymbolIndex = i - 1; - if (nSymbolIndex >= sal_Int32(aSymbols.size())) - continue; - xPropertySet.set(xTableRows->getByIndex(i), uno::UNO_QUERY); - sal_Int32 nHeight = 0; - xPropertySet->getPropertyValue("Height") >>= nHeight; - if (i > 0) + if (nSymbolIndex < sal_Int32(aSymbols.size())) { - auto& rSymbol = aSymbols[nSymbolIndex].aSymbol; - sal_Int32 nSymbolHeight = rSymbol->getSize().Height; - sal_Int32 nSymbolY - = basegfx::fround(double(nHeight) / 2.0 - double(nSymbolHeight) / 2.0); - rSymbol->setPosition( - { nTableX + constSymbolMargin, nTableY + nTotalHeight + nSymbolY }); + xPropertySet.set(xTableRows->getByIndex(i), uno::UNO_QUERY); + sal_Int32 nHeight = 0; + xPropertySet->getPropertyValue("Height") >>= nHeight; + if (i > 0) + { + auto& rSymbol = aSymbols[nSymbolIndex].aSymbol; + sal_Int32 nSymbolHeight = rSymbol->getSize().Height; + sal_Int32 nSymbolY + = basegfx::fround(double(nHeight) / 2.0 - double(nSymbolHeight) / 2.0); + rSymbol->setPosition( + { nTableX + constSymbolMargin, nTableY + nTotalHeight + nSymbolY }); + } + nTotalHeight += nHeight; } - nTotalHeight += nHeight; } } xBroadcaster->unlockBroadcasts();