ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

Bound the number of histogram bins

Browse Source 
A bin width much smaller than the data range asks for a bin count
with no upper bound. Over a range of 1000 a width of a millionth
resolves to a billion bins. The conversion from double to sal_Int32
had no range check either, so it could overflow.

Cap the count at MAX_HISTOGRAM_BINS. The division stays in double
and saturates at the cap, and a fixed count from a document is
clamped the same way.

A capped count no longer spans the range with the requested width,
so the range is divided evenly instead. Scott's rule gets the same
treatment when it yields no width at all.

Signed-off-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.co.uk>
Change-Id: Ib59ee462edade94799cc58e503b843696743ecfe
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/10257
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins CPCI <releng@collaboraoffice.com>
(cherry picked from commit 93a9df897a14ec12457aa40d328616eb071a9175)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/210337
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Jenkins
This commit is contained in:
Tomaž Vajngerl
2026-09-01 19:49:22 +02:00
committed by Xisco Fauli
parent 92a5acb7e6
commit 3aa0160c31
3 changed files with 58 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
chart2/qa/extras/chart2import2.cxx
+31
View File
@@ -1297,6 +1297,37 @@ CPPUNIT_TEST_FIXTURE(Chart2ImportTest2, testHistogramBinCountLiveUpdate)
CPPUNIT_ASSERT_EQUAL(sal_Int32(3), aUpdatedLabels.getLength());
}
CPPUNIT_TEST_FIXTURE(Chart2ImportTest2, testHistogramBinWidthFarBelowRangeIsBounded)
{
loadFromFile(u"fods/tdf163727_histogram_roundtrip.fods");
uno::Reference<chart2::XChartDocument> xChartDoc = getChartDocFromSheet(0);
CPPUNIT_ASSERT(xChartDoc.is());
Reference<chart2::XChartType> xChartType = getChartTypeFromDoc(xChartDoc, 0, 0);
CPPUNIT_ASSERT(xChartType.is());
Reference<chart2::XAxis> xXAxis = getAxisFromDoc(xChartDoc, 0, 0, 0);
CPPUNIT_ASSERT(xXAxis.is());
chart2::ScaleData aScaleData = xXAxis->getScaleData();
CPPUNIT_ASSERT(aScaleData.Categories.is());
Reference<chart2::data::XTextualDataSequence> xAxisCategories(
aScaleData.Categories->getValues(), uno::UNO_QUERY);
CPPUNIT_ASSERT(xAxisCategories.is());
Reference<beans::XPropertySet> xProperties(xChartType, uno::UNO_QUERY_THROW);
xProperties->setPropertyValue(u"FrequencyType"_ustr, uno::Any(sal_Int32(1)));
// The values span roughly eight units, so bins a millionth wide would number in the
// millions. The count stays within the limit the calculator caps it at.
xProperties->setPropertyValue(u"BinWidth"_ustr, uno::Any(0.000001));
const Sequence<OUString> aLabels = xAxisCategories->getTextualData();
CPPUNIT_ASSERT_EQUAL(sal_Int32(10000), aLabels.getLength());
}
CPPUNIT_TEST_FIXTURE(Chart2ImportTest2, testHistogramBinCountRoundtrip_ODS)
{
// ODF: fixed number of bins survives export/reload at the model level.
chart2/source/inc/HistogramCalculator.hxx
+3
View File
@@ -14,6 +14,9 @@
namespace chart
{
/** Largest number of bins a histogram is partitioned into. Higher counts are capped. */
constexpr sal_Int32 MAX_HISTOGRAM_BINS = 10000;
enum class HistogramBinType
{
Normal,
chart2/source/model/template/HistogramCalculator.cxx
+24 -7
View File
@@ -14,6 +14,23 @@
namespace chart
{
namespace
{
/** Bins of the given width needed to span the given range, capped at MAX_HISTOGRAM_BINS and
at least one. Dividing in double saturates at the cap instead of overflowing the cast. */
sal_Int32 lcl_binCountForWidth(double fRange, double fBinWidth)
{
if (!(fRange > 0.0) || !(fBinWidth > 0.0))
return 1;
const double fBins = std::ceil(fRange / fBinWidth);
if (!(fBins >= 1.0))
return 1;
return sal_Int32(std::min(fBins, double(MAX_HISTOGRAM_BINS)));
}
}
HistogramCalculator::HistogramCalculator() = default;
void HistogramCalculator::computeBinFrequencyHistogram(
@@ -112,13 +129,12 @@ void HistogramCalculator::computeBinFrequencyHistogram(
if (nFrequencyType == 1 && fFixedBinWidth > 0.0)
{
mfBinWidth = fFixedBinWidth;
mnBins
= static_cast<sal_Int32>(std::ceil((fEffectiveMax - fEffectiveMin) / mfBinWidth));
mnBins = lcl_binCountForWidth(fEffectiveMax - fEffectiveMin, mfBinWidth);
bResolved = true;
}
else if (nFrequencyType == 2 && nFixedBinCount > 0)
{
mnBins = nFixedBinCount;
mnBins = std::min<sal_Int32>(nFixedBinCount, MAX_HISTOGRAM_BINS);
mfBinWidth = (fEffectiveMax - fEffectiveMin) / mnBins;
bResolved = true;
}
@@ -131,11 +147,12 @@ void HistogramCalculator::computeBinFrequencyHistogram(
double fStdDev = std::sqrt(fVariance);
mfBinWidth = (3.5 * fStdDev) / std::cbrt(nValidCount);
mnBins
= static_cast<sal_Int32>(std::ceil((fEffectiveMax - fEffectiveMin) / mfBinWidth));
mnBins = lcl_binCountForWidth(fEffectiveMax - fEffectiveMin, mfBinWidth);
}
mnBins = std::max<sal_Int32>(mnBins, 1); // Ensure at least one bin
// A capped count or a zero width no longer spans the range, so divide it evenly.
if (!(mfBinWidth > 0.0) || mnBins == MAX_HISTOGRAM_BINS)
mfBinWidth = (fEffectiveMax - fEffectiveMin) / mnBins;
maBinRanges.reserve(mnBins + (bEffectiveUnderflow ? 1 : 0) + (bEffectiveOverflow ? 1 : 0));
maBinTypes.reserve(mnBins + (bEffectiveUnderflow ? 1 : 0) + (bEffectiveOverflow ? 1 : 0));
@@ -227,4 +244,4 @@ void HistogramCalculator::computeBinFrequencyHistogram(
} // namespace chart
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */