From 22b73ccfdc7db2cd39af1f1fcd10ca161518dea7 Mon Sep 17 00:00:00 2001 From: Devansh Varshney Date: Sat, 15 Aug 2026 00:06:30 +0530 Subject: [PATCH] chart2: keep histogram axis labels in sync with bin settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Histogram bin-property changes regenerated the category sequence stored on the data series, while the X axis continued referencing the previous sequence. This left the chart bars updated but displayed stale bin labels. Keep the category sequence produced by HistogramDataInterpreter on both the diagram axis and data series. Update its binning parameters in place when histogram properties change, allowing existing listeners to recalculate and redraw the axis labels. Signed-off-by: Devansh Varshney Change-Id: Ibd6679bddf29de5fa1fd903af9274259b2e9c243 Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/9282 Reviewed-by: Tomaž Vajngerl Tested-by: Tomaž Vajngerl (cherry picked from commit 0103afa2a6a746ce54a0e3e4297e1fa60698ef03) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/209218 Tested-by: Jenkins Reviewed-by: Xisco Fauli --- chart2/qa/extras/chart2import2.cxx | 46 ++++++++++++++- chart2/source/inc/HistogramDataSequence.hxx | 4 ++ .../model/template/HistogramChartType.cxx | 56 ++++++++++++------- .../template/HistogramDataInterpreter.cxx | 5 +- chart2/source/tools/HistogramDataSequence.cxx | 31 ++++++++++ 5 files changed, 117 insertions(+), 25 deletions(-) diff --git a/chart2/qa/extras/chart2import2.cxx b/chart2/qa/extras/chart2import2.cxx index 52f271c4b1e2..b957ddb16b6f 100644 --- a/chart2/qa/extras/chart2import2.cxx +++ b/chart2/qa/extras/chart2import2.cxx @@ -1215,9 +1215,9 @@ CPPUNIT_TEST_FIXTURE(Chart2ImportTest2, testHistogramXLSXRoundtrip) = getDataSequenceFromDocByRole(xChartDoc, u"calculated-y"); CPPUNIT_ASSERT(!xCalculatedY.is()); - // The X axis must carry the bin range labels from the histogram template, - // not the generic "1", "2", ... labels that the OOXML axis converter - // produces by default. + // The X axis carries the bin range labels from the histogram template, not the generic + // "1", "2", ... labels that the OOXML axis converter produces by default. The axis and the + // series read the same sequence, so the labels match the ones asserted for the series. Reference xXAxis = getAxisFromDoc(xChartDoc, 0, 0, 0); CPPUNIT_ASSERT(xXAxis.is()); chart2::ScaleData aScaleData = xXAxis->getScaleData(); @@ -1257,6 +1257,46 @@ CPPUNIT_TEST_FIXTURE(Chart2ImportTest2, testHistogramXLSXRoundtrip) CPPUNIT_ASSERT_EQUAL(2.5, fBinWidth); } +CPPUNIT_TEST_FIXTURE(Chart2ImportTest2, testHistogramBinCountLiveUpdate) +{ + loadFromFile(u"fods/tdf163727_histogram_roundtrip.fods"); + + uno::Reference xChartDoc = getChartDocFromSheet(0); + CPPUNIT_ASSERT(xChartDoc.is()); + + Reference xChartType = getChartTypeFromDoc(xChartDoc, 0, 0); + CPPUNIT_ASSERT(xChartType.is()); + + // Keep the category sequence currently held by the X axis. Changing the + // bin count must update this same sequence instead of leaving it stale + Reference xXAxis = getAxisFromDoc(xChartDoc, 0, 0, 0); + CPPUNIT_ASSERT(xXAxis.is()); + + chart2::ScaleData aScaleData = xXAxis->getScaleData(); + CPPUNIT_ASSERT(aScaleData.Categories.is()); + + Reference xAxisCategories( + aScaleData.Categories->getValues(), uno::UNO_QUERY); + CPPUNIT_ASSERT(xAxisCategories.is()); + + const Sequence aOriginalLabels = xAxisCategories->getTextualData(); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), aOriginalLabels.getLength()); + + Reference xProperties(xChartType, uno::UNO_QUERY_THROW); + + // Switching to fixed-count mode initially uses the stored default of ten bins. + xProperties->setPropertyValue(u"FrequencyType"_ustr, uno::Any(sal_Int32(2))); + + const Sequence aFixedCountLabels = xAxisCategories->getTextualData(); + CPPUNIT_ASSERT_EQUAL(sal_Int32(10), aFixedCountLabels.getLength()); + + // Changing the count must update the same sequence without save/reload. + xProperties->setPropertyValue(u"BinCount"_ustr, uno::Any(sal_Int32(3))); + + const Sequence aUpdatedLabels = xAxisCategories->getTextualData(); + CPPUNIT_ASSERT_EQUAL(sal_Int32(3), aUpdatedLabels.getLength()); +} + CPPUNIT_TEST_FIXTURE(Chart2ImportTest2, testHistogramBinCountRoundtrip_ODS) { // ODF: fixed number of bins survives export/reload at the model level. diff --git a/chart2/source/inc/HistogramDataSequence.hxx b/chart2/source/inc/HistogramDataSequence.hxx index ba33c73bd531..192bb394b88b 100644 --- a/chart2/source/inc/HistogramDataSequence.hxx +++ b/chart2/source/inc/HistogramDataSequence.hxx @@ -58,6 +58,10 @@ public: bool bUseOverflowBin = false, double fOverflowBinValue = 0.0); virtual ~HistogramDataSequence() override; + void setBinningParameters(sal_Int32 nFrequencyType, double fBinWidth, sal_Int32 nBinCount, + bool bUseUnderflowBin, double fUnderflowBinValue, + bool bUseOverflowBin, double fOverflowBinValue); + /// declare XServiceInfo methods virtual OUString SAL_CALL getImplementationName() override; virtual sal_Bool SAL_CALL supportsService(const OUString& ServiceName) override; diff --git a/chart2/source/model/template/HistogramChartType.cxx b/chart2/source/model/template/HistogramChartType.cxx index 6e1495c2e12e..a4b512785dd7 100644 --- a/chart2/source/model/template/HistogramChartType.cxx +++ b/chart2/source/model/template/HistogramChartType.cxx @@ -241,37 +241,51 @@ void HistogramChartType::createCalculatedDataSeries() = new LabeledDataSequence(xCalcSeq); xSeries->setCalculatedYSequence(xLabeledCalc); - // 3. Regenerate the categories (bins) - rtl::Reference xCatSeq = new HistogramDataSequence( - xValuesY, true, nFrequencyType, fBinWidth, nBinCount, bUseUnderflowBin, - fUnderflowBinValue, bUseOverflowBin, fOverflowBinValue); + auto createCategorySequence = [&]() { + rtl::Reference xCatSeq = new HistogramDataSequence( + xValuesY, true, nFrequencyType, fBinWidth, nBinCount, bUseUnderflowBin, + fUnderflowBinValue, bUseOverflowBin, fOverflowBinValue); - uno::Reference xCatDataSeq(xCatSeq); - uno::Reference xCatProp(xCatDataSeq, uno::UNO_QUERY); - if (xCatProp.is()) - { - xCatProp->setPropertyValue(u"Role"_ustr, uno::Any(u"categories"_ustr)); - } + uno::Reference xCatDataSeq(xCatSeq); + uno::Reference xCatProp(xCatDataSeq, uno::UNO_QUERY); + if (xCatProp.is()) + { + xCatProp->setPropertyValue(u"Role"_ustr, uno::Any(u"categories"_ustr)); + } - uno::Reference xLabeledCat - = new LabeledDataSequence(xCatSeq); + return uno::Reference( + new LabeledDataSequence(xCatSeq)); + }; - // 4. Attach the categories to the DataSeries so the View can find them + // 3. Update existing histogram categories in place. The X axis and the DataSeries hold + // the same sequence, so new bin parameters reach the axis labels through it. bool bHasCategories = false; for (auto& seq : aSeqs) { - if (seq.is() && DataSeriesHelper::getRole(seq) == u"categories"_ustr) + if (!seq.is() || DataSeriesHelper::getRole(seq) != u"categories"_ustr) + continue; + + rtl::Reference xHistogramSequence + = dynamic_cast(seq->getValues().get()); + + if (xHistogramSequence.is()) { - seq = xLabeledCat; - bHasCategories = true; - break; + xHistogramSequence->setBinningParameters(nFrequencyType, fBinWidth, nBinCount, + bUseUnderflowBin, fUnderflowBinValue, + bUseOverflowBin, fOverflowBinValue); } + else + { + seq = createCategorySequence(); + } + + bHasCategories = true; + break; } + // 4. Attach generated categories when the series does not have them yet if (!bHasCategories) - { - aSeqs.push_back(xLabeledCat); - } + aSeqs.push_back(createCategorySequence()); xSeries->setData(aSeqs); } @@ -375,4 +389,4 @@ com_sun_star_comp_chart_HistogramChartType_get_implementation( return cppu::acquire(new ::chart::HistogramChartType()); } -/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ \ No newline at end of file +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/chart2/source/model/template/HistogramDataInterpreter.cxx b/chart2/source/model/template/HistogramDataInterpreter.cxx index 9edf9abf6489..13beb7d52940 100644 --- a/chart2/source/model/template/HistogramDataInterpreter.cxx +++ b/chart2/source/model/template/HistogramDataInterpreter.cxx @@ -90,7 +90,10 @@ InterpretedData HistogramDataInterpreter::interpretDataSource( resultSeries = new DataSeries(); } - resultSeries->setData(DataSeries::tDataSequenceContainer{ valuesYSeqLabeled }); + // The diagram axis and the DataSeries hold the same generated category sequence, so new + // bin settings reach the X axis labels through it. + resultSeries->setData( + DataSeries::tDataSequenceContainer{ valuesYSeqLabeled, xLabeledCategorySeq }); resultSeries->setCalculatedYSequence(xLabeledFreqSeq); // Structure the result: one group, one series for a standard histogram diff --git a/chart2/source/tools/HistogramDataSequence.cxx b/chart2/source/tools/HistogramDataSequence.cxx index d84b9dc8c5d3..ea065af843ab 100644 --- a/chart2/source/tools/HistogramDataSequence.cxx +++ b/chart2/source/tools/HistogramDataSequence.cxx @@ -82,6 +82,37 @@ HistogramDataSequence::~HistogramDataSequence() } } +void HistogramDataSequence::setBinningParameters(sal_Int32 nFrequencyType, double fBinWidth, + sal_Int32 nBinCount, bool bUseUnderflowBin, + double fUnderflowBinValue, bool bUseOverflowBin, + double fOverflowBinValue) +{ + { + osl::MutexGuard aGuard(GetMutex()); + + // Recomputing the bins is expensive, so parameters that repeat the ones already held + // leave the cached bins and the modified state alone. + if (m_nFrequencyType == nFrequencyType && m_fBinWidth == fBinWidth + && m_nBinCount == nBinCount && m_bUseUnderflowBin == bUseUnderflowBin + && m_fUnderflowBinValue == fUnderflowBinValue && m_bUseOverflowBin == bUseOverflowBin + && m_fOverflowBinValue == fOverflowBinValue) + { + return; + } + + m_nFrequencyType = nFrequencyType; + m_fBinWidth = fBinWidth; + m_nBinCount = nBinCount; + m_bUseUnderflowBin = bUseUnderflowBin; + m_fUnderflowBinValue = fUnderflowBinValue; + m_bUseOverflowBin = bUseOverflowBin; + m_fOverflowBinValue = fOverflowBinValue; + m_bIsDirty = true; + } + + m_xModifyEventForwarder->modified(lang::EventObject(static_cast(this))); +} + IMPLEMENT_FORWARD_XINTERFACE2(HistogramDataSequence, HistogramDataSequence_Base, ::comphelper::OPropertyContainer)