sc/qa/unit/subsequent_export_test2.cxx | 13 +++++++++++++ sc/source/core/data/documen3.cxx | 6 ++---- sc/source/filter/oox/drawingbase.cxx | 5 ++--- 3 files changed, 17 insertions(+), 7 deletions(-)
New commits: commit 71dc2b20cc76d2728cb57a3bf62f10db8384a74b Author: Justin Luth <[email protected]> AuthorDate: Wed Nov 26 11:50:24 2025 -0500 Commit: Justin Luth <[email protected]> CommitDate: Fri Dec 5 22:30:24 2025 +0100 related tdf#166724 sc GetRange: last twip still in cell GetRange indicates which cells are found inside the provided rectangular area. The problem is that providing the BottomRight point of a cell to GetRange was resulting in a different cell being "found". I understand that the end of one cell is the beginning of another, and that GetRange calculates in TWIPs (where 1 twip = 1.7 mm100) so imprecision can affect a point at the very edge. However, if we reduce BottomRight by 1 twip, then certainly GetRange should find the original cell back. These (erroneous) formulas already existed in 'Initial import'. I don't care if this gets reverted. I just added it because it makes my calculation cleaner. The unit test highly suggests that this has been hiding bugs... Change-Id: I06d1c362cd8f80913ee695825260dd5b31b29bfb Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194347 Tested-by: Jenkins Reviewed-by: Justin Luth <[email protected]> diff --git a/sc/qa/unit/subsequent_export_test2.cxx b/sc/qa/unit/subsequent_export_test2.cxx index 637fac6fbde6..235138a544ae 100644 --- a/sc/qa/unit/subsequent_export_test2.cxx +++ b/sc/qa/unit/subsequent_export_test2.cxx @@ -96,6 +96,19 @@ CPPUNIT_TEST_FIXTURE(ScExportTest2, testTdf166724_cellAnchor) CPPUNIT_ASSERT_EQUAL(tools::Long(1058), aRect.Top()); CPPUNIT_ASSERT_EQUAL(tools::Long(4192), aRect.GetWidth()); CPPUNIT_ASSERT_EQUAL(tools::Long(557), aRect.GetHeight()); + + // test implementation of GetRange + tools::Rectangle aB2(rRTDoc.GetMMRect(1, 1, 1, 1, 0)); // B2 from sheet 0 in mm100 + // ask which cell contains the bottom right point: found using TWIP precision + ScRange aFoundCell = rRTDoc.GetRange(0, tools::Rectangle(aB2.Right(), aB2.Bottom())); + // BottomRight of cell B2 is approximately the same as the TopLeft of cell C3 + CPPUNIT_ASSERT_EQUAL(SCCOL(2), aFoundCell.aStart.Col()); // found cell C3 + CPPUNIT_ASSERT_EQUAL(SCROW(2), aFoundCell.aStart.Row()); + tools::Long n1Twip = o3tl::convert(1, o3tl::Length::twip, o3tl::Length::mm100); // 2 mm100 + // reducing by 1 twip must return cell B2 + aFoundCell = rRTDoc.GetRange(0, tools::Rectangle(aB2.Right() - n1Twip, aB2.Bottom() - n1Twip)); + CPPUNIT_ASSERT_EQUAL(SCCOL(1), aFoundCell.aStart.Col()); // found cell B2 + CPPUNIT_ASSERT_EQUAL(SCROW(1), aFoundCell.aStart.Row()); }; CPPUNIT_TEST_FIXTURE(ScExportTest2, testFreezePaneStartCellXLSX) diff --git a/sc/source/core/data/documen3.cxx b/sc/source/core/data/documen3.cxx index 2635cb59f25e..4ad6fef755d5 100644 --- a/sc/source/core/data/documen3.cxx +++ b/sc/source/core/data/documen3.cxx @@ -1832,7 +1832,7 @@ ScRange ScDocument::GetRange( SCTAB nTab, const tools::Rectangle& rMMRect, bool while (!bEnd) { nAdd = pTable->GetColWidth(nX1, bHiddenAsZero); - if (nSize+nAdd <= nTwips+1 && nX1<MaxCol()) + if (nSize + nAdd < nTwips+1 && nX1 < MaxCol()) { nSize += nAdd; ++nX1; @@ -1864,15 +1864,13 @@ ScRange ScDocument::GetRange( SCTAB nTab, const tools::Rectangle& rMMRect, bool nTwips = aPosRect.Top(); SCROW nY1 = 0; - // Was if(nSize+nAdd<=nTwips+1) inside loop => if(nSize+nAdd<nTwips+2) - if (lcl_AddTwipsWhile( nSize, nTwips+2, nY1, MaxRow(), pTable, bHiddenAsZero) && nY1 < MaxRow()) + if (lcl_AddTwipsWhile(nSize, nTwips+1, nY1, MaxRow(), pTable, bHiddenAsZero) && nY1 < MaxRow()) ++nY1; // original loop ended on last matched +1 unless that was rDoc.MaxRow() SCROW nY2 = nY1; if (!aPosRect.IsEmpty()) { nTwips = aPosRect.Bottom(); - // Was if(nSize+nAdd<nTwips) inside loop => if(nSize+nAdd<nTwips) if (lcl_AddTwipsWhile( nSize, nTwips, nY2, MaxRow(), pTable, bHiddenAsZero) && nY2 < MaxRow()) ++nY2; // original loop ended on last matched +1 unless that was rDoc.MaxRow() } diff --git a/sc/source/filter/oox/drawingbase.cxx b/sc/source/filter/oox/drawingbase.cxx index 127cb73d4c7e..af028a3ab1ae 100644 --- a/sc/source/filter/oox/drawingbase.cxx +++ b/sc/source/filter/oox/drawingbase.cxx @@ -275,10 +275,9 @@ EmuPoint ShapeAnchor::calcCellAnchorEmu( const CellAnchorModel& rModel ) const // It is easily possible that the provided Offset is invalid (too large). // Excel seems to limit the offsets to the bottom/left edge of the cell. // Because most calculations are rounded down to TWIPs, reduce cell's right edge by a full twip. - // Reduce by another twip because of the way GetRange calculates which cell this point is in... - sal_Int64 n2TwipInEmu = std::ceil(rUnitConv.scaleValue(2, Unit::Twip, Unit::Emu)); + sal_Int64 n1TwipInEmu = rUnitConv.scaleValue(1, Unit::Twip, Unit::Emu); // 635 emu EmuPoint aEmuMaxOffset( - lclHmmToEmu(aNextCell.X) - n2TwipInEmu, lclHmmToEmu(aNextCell.Y) - n2TwipInEmu); + lclHmmToEmu(aNextCell.X) - n1TwipInEmu, lclHmmToEmu(aNextCell.Y) - n1TwipInEmu); // add the offset inside the cell switch( meCellAnchorType )
