sw/qa/core/layout/data/inner-border.docx |binary sw/qa/core/layout/layout.cxx | 53 +++++++++++++++++++ sw/source/core/layout/paintfrm.cxx | 83 +++++++++++++++++++++++-------- 3 files changed, 116 insertions(+), 20 deletions(-)
New commits: commit 526c8bdb54eff942d5213030d1455f97720a1ba7 Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Thu Jan 6 15:45:02 2022 +0100 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Thu Jan 6 18:37:14 2022 +0100 sw: fix too long inner borders intersecting with outer borders for Word cells This is similar to commit 66ac8e60896f6306bed8fbb34606fd14474f19ce (sw: fix unwanted long vertical border around vertically merged Word cell, 2021-03-04), but this one is about how we handle table border painting when an inner border intersects with an outer border. Previously we used to paint the full outer border and the full inner border, which looks silly in case you have e.g. double border outside and a single border inside -- the inner line stops at the edge of the thick outer border in Word. Do the same by limiting the start of a horizontal line if its start would match the X coordinate of a vertical line (and the remaining 3 combinations of hori/vert line start/end). We always limit the inner line, so this needs extending SwLineEntry if the line is an outer one or not. Change-Id: I669a271ce3a4c3c69916779d4f3167208e999f05 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128053 Reviewed-by: Miklos Vajna <vmik...@collabora.com> Tested-by: Jenkins diff --git a/sw/qa/core/layout/data/inner-border.docx b/sw/qa/core/layout/data/inner-border.docx new file mode 100644 index 000000000000..1d8adc9fe818 Binary files /dev/null and b/sw/qa/core/layout/data/inner-border.docx differ diff --git a/sw/qa/core/layout/layout.cxx b/sw/qa/core/layout/layout.cxx index b1cb3136b963..2b1dc8be0b3f 100644 --- a/sw/qa/core/layout/layout.cxx +++ b/sw/qa/core/layout/layout.cxx @@ -525,6 +525,59 @@ CPPUNIT_TEST_FIXTURE(SwCoreLayoutTest, testLinkedBullet) assertXPath(pXmlDoc, "//bmpexscale", 1); } +CPPUNIT_TEST_FIXTURE(SwCoreLayoutTest, testInnerCellBorderIntersect) +{ + // Given a table with both outer and inner borders: + SwDoc* pDoc = createSwDoc(DATA_DIRECTORY, "inner-border.docx"); + SwDocShell* pShell = pDoc->GetDocShell(); + + // When rendering table borders: + std::shared_ptr<GDIMetaFile> xMetaFile = pShell->GetPreviewMetaFile(); + + // Then make sure that that inner and outer borders don't overlap in Word compatibility mode, + // and inner borders are reduced to prevent an overlap: + MetafileXmlDump dumper; + xmlDocUniquePtr pXmlDoc = dumpAndParse(dumper, *xMetaFile); + // Collect start/end (vertical) positions of horizontal borders. + xmlXPathObjectPtr pXmlObj = getXPathNode(pXmlDoc, "//polyline[@style='solid']/point"); + xmlNodeSetPtr pXmlNodes = pXmlObj->nodesetval; + std::vector<std::pair<sal_Int32, sal_Int32>> aBorderStartEnds; + for (int i = 0; i < xmlXPathNodeSetGetLength(pXmlNodes); i += 2) + { + xmlNodePtr pStart = pXmlNodes->nodeTab[i]; + xmlNodePtr pEnd = pXmlNodes->nodeTab[i + 1]; + xmlChar* pStartY = xmlGetProp(pStart, BAD_CAST("y")); + xmlChar* pEndY = xmlGetProp(pEnd, BAD_CAST("y")); + sal_Int32 nStartY = OString(reinterpret_cast<char const*>(pStartY)).toInt32(); + sal_Int32 nEndY = OString(reinterpret_cast<char const*>(pEndY)).toInt32(); + if (nStartY != nEndY) + { + // Vertical border. + continue; + } + xmlChar* pStartX = xmlGetProp(pStart, BAD_CAST("x")); + xmlChar* pEndX = xmlGetProp(pEnd, BAD_CAST("x")); + sal_Int32 nStartX = OString(reinterpret_cast<char const*>(pStartX)).toInt32(); + sal_Int32 nEndX = OString(reinterpret_cast<char const*>(pEndX)).toInt32(); + aBorderStartEnds.emplace_back(nStartX, nEndX); + } + xmlXPathFreeObject(pXmlObj); + // We have 3 lines: top, middle and bottom. The top and the bottom one is a full line, since + // it's an outer border. The middle one has increased start and decreased end to avoid an + // overlap. + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), aBorderStartEnds.size()); + CPPUNIT_ASSERT_EQUAL(aBorderStartEnds[0].first, aBorderStartEnds[2].first); + // Without the accompanying fix in place, this test would have failed with: + // - Expected greater than: 1724 + // - Actual : 1724 + // i.e. the middle line's start was the same as the top line start, while what we want is a + // larger X position, so the start of the middle line doesn't overlap with the thick vertical + // outer border on the left of the table. + CPPUNIT_ASSERT_GREATER(aBorderStartEnds[0].first, aBorderStartEnds[1].first); + CPPUNIT_ASSERT_EQUAL(aBorderStartEnds[0].second, aBorderStartEnds[2].second); + CPPUNIT_ASSERT_LESS(aBorderStartEnds[0].second, aBorderStartEnds[1].second); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/layout/paintfrm.cxx b/sw/source/core/layout/paintfrm.cxx index 2f9ecfb6be2e..7916e3a7e5a1 100644 --- a/sw/source/core/layout/paintfrm.cxx +++ b/sw/source/core/layout/paintfrm.cxx @@ -2243,6 +2243,7 @@ struct SwLineEntry SwTwips mnStartPos; SwTwips mnEndPos; SwTwips mnLimitedEndPos; + bool mbOuter; svx::frame::Style maAttribute; @@ -2254,6 +2255,7 @@ public: SwLineEntry( SwTwips nKey, SwTwips nStartPos, SwTwips nEndPos, + bool bOuter, const svx::frame::Style& rAttribute ); OverlapType Overlaps( const SwLineEntry& rComp ) const; @@ -2270,11 +2272,13 @@ public: SwLineEntry::SwLineEntry( SwTwips nKey, SwTwips nStartPos, SwTwips nEndPos, + bool bOuter, const svx::frame::Style& rAttribute ) : mnKey( nKey ), mnStartPos( nStartPos ), mnEndPos( nEndPos ), mnLimitedEndPos(0), + mbOuter(bOuter), maAttribute( rAttribute ) { } @@ -2385,10 +2389,11 @@ class SwTabFramePainter void Insert( SwLineEntry&, bool bHori ); void Insert(const SwFrame& rFrame, const SvxBoxItem& rBoxItem, const SwRect &rPaintArea); void HandleFrame(const SwLayoutFrame& rFrame, const SwRect& rPaintArea); - void FindStylesForLine( const Point&, - const Point&, + void FindStylesForLine( Point&, + Point&, svx::frame::Style*, - bool bHori ) const; + bool bHori, + bool bOuter ) const; public: explicit SwTabFramePainter( const SwTabFrame& rTabFrame ); @@ -2502,7 +2507,7 @@ void SwTabFramePainter::PaintLines(OutputDevice& rDev, const SwRect& rRect) cons svx::frame::Style aStyles[ 7 ]; aStyles[ 0 ] = rEntryStyle; - FindStylesForLine( aStart, aEnd, aStyles, bHori ); + FindStylesForLine(aStart, aEnd, aStyles, bHori, rEntry.mbOuter); if (!bHori && rEntry.mnLimitedEndPos) { @@ -2676,10 +2681,10 @@ void SwTabFramePainter::PaintLines(OutputDevice& rDev, const SwRect& rRect) cons * StartPoint or Endpoint. The styles of these lines are required for DR's magic * line painting functions */ -void SwTabFramePainter::FindStylesForLine( const Point& rStartPoint, - const Point& rEndPoint, +void SwTabFramePainter::FindStylesForLine( Point& rStartPoint, + Point& rEndPoint, svx::frame::Style* pStyles, - bool bHori ) const + bool bHori, bool bOuter ) const { // For example, aLFromB means: this vertical line intersects my horizontal line at its left end, // from bottom. @@ -2690,6 +2695,14 @@ void SwTabFramePainter::FindStylesForLine( const Point& rStartPoint, // pStyles[ 5 ] = bHori ? aRFromR : BFromB, // pStyles[ 6 ] = bHori ? aRFromB : BFromR, + bool bWordTableCell = false; + SwViewShell* pShell = mrTabFrame.getRootFrame()->GetCurrShell(); + if (pShell) + { + const IDocumentSettingAccess& rIDSA = pShell->GetDoc()->getIDocumentSettingAccess(); + bWordTableCell = rIDSA.get(DocumentSettingId::TABLE_ROW_KEEP); + } + SwLineEntryMap::const_iterator aMapIter = maVertLines.find( rStartPoint.X() ); OSL_ENSURE( aMapIter != maVertLines.end(), "FindStylesForLine: Error" ); const SwLineEntrySet& rVertSet = (*aMapIter).second; @@ -2702,6 +2715,11 @@ void SwTabFramePainter::FindStylesForLine( const Point& rStartPoint, pStyles[ 3 ] = rEntry.maAttribute; else if ( rStartPoint.Y() == rEntry.mnEndPos ) pStyles[ 1 ] = rEntry.maAttribute; + + if (bWordTableCell && rStartPoint.X() == rEntry.mnKey && !bOuter && rEntry.mbOuter) + { + rStartPoint.AdjustX(rEntry.maAttribute.GetWidth()); + } } else { @@ -2731,6 +2749,11 @@ void SwTabFramePainter::FindStylesForLine( const Point& rStartPoint, pStyles[ 1 ] = rEntry.maAttribute; else if ( rStartPoint.X() == rEntry.mnStartPos ) pStyles[ 3 ] = rEntry.maAttribute; + + if (bWordTableCell && rStartPoint.Y() == rEntry.mnKey && !bOuter && rEntry.mbOuter) + { + rStartPoint.AdjustY(rEntry.maAttribute.GetWidth()); + } } } @@ -2746,6 +2769,11 @@ void SwTabFramePainter::FindStylesForLine( const Point& rStartPoint, pStyles[ 6 ] = rEntry.maAttribute; else if ( rEndPoint.Y() == rEntry.mnEndPos ) pStyles[ 4 ] = rEntry.maAttribute; + + if (bWordTableCell && rEndPoint.X() == rEntry.mnKey && !bOuter && rEntry.mbOuter) + { + rEndPoint.AdjustX(-rEntry.maAttribute.GetWidth()); + } } } else @@ -2760,6 +2788,11 @@ void SwTabFramePainter::FindStylesForLine( const Point& rStartPoint, pStyles[ 4 ] = rEntry.maAttribute; else if ( rEndPoint.X() == rEntry.mnStartPos ) pStyles[ 6 ] = rEntry.maAttribute; + + if (bWordTableCell && rEndPoint.Y() == rEntry.mnKey && !bOuter && rEntry.mbOuter) + { + rEndPoint.AdjustY(-rEntry.maAttribute.GetWidth()); + } } } } @@ -2827,22 +2860,32 @@ void SwTabFramePainter::Insert(const SwFrame& rFrame, const SvxBoxItem& rBoxItem aT.SetRefMode( !bVert ? svx::frame::RefMode::Begin : svx::frame::RefMode::End ); aB.SetRefMode( !bVert ? svx::frame::RefMode::Begin : svx::frame::RefMode::End ); - SwLineEntry aLeft (nLeft, nTop, nBottom, + // First cell in a row. + bool bOuter = rFrame.IsCellFrame() && rFrame.GetUpper()->GetLower() == &rFrame; + SwLineEntry aLeft (nLeft, nTop, nBottom, bOuter, bVert ? aB : (bR2L ? aR : aL)); if (bWordTableCell && rBoxItem.GetLeft()) { aLeft.LimitVerticalEndPos(rFrame, SwLineEntry::VerticalType::LEFT); } - SwLineEntry aRight (nRight, nTop, nBottom, + // Last cell in a row. + bOuter = rFrame.IsCellFrame() && rFrame.GetNext() == nullptr; + SwLineEntry aRight (nRight, nTop, nBottom, bOuter, bVert ? (bBottomAsTop ? aB : aT) : (bR2L ? aL : aR)); if (bWordTableCell && rBoxItem.GetRight()) { aRight.LimitVerticalEndPos(rFrame, SwLineEntry::VerticalType::RIGHT); } - SwLineEntry aTop (nTop, nLeft, nRight, + + // First row in a table. + bOuter = rFrame.IsCellFrame() && rFrame.GetUpper()->GetUpper()->GetLower() == rFrame.GetUpper(); + SwLineEntry aTop (nTop, nLeft, nRight, bOuter, bVert ? aL : (bBottomAsTop ? aB : aT)); - SwLineEntry aBottom(nBottom, nLeft, nRight, + + // Last row in a table. + bOuter = rFrame.IsCellFrame() && rFrame.GetUpper()->GetNext() == nullptr; + SwLineEntry aBottom(nBottom, nLeft, nRight, bOuter, bVert ? aR : aB); Insert( aLeft, false ); @@ -2871,7 +2914,7 @@ void SwTabFramePainter::Insert( SwLineEntry& rNew, bool bHori ) { const SwLineEntry& rOld = *aIter; - if (rOld.mnLimitedEndPos) + if (rOld.mnLimitedEndPos || rOld.mbOuter != rNew.mbOuter) { // Don't merge with this line entry as it ends sooner than mnEndPos. ++aIter; @@ -2889,10 +2932,10 @@ void SwTabFramePainter::Insert( SwLineEntry& rNew, bool bHori ) OSL_ENSURE( rNew.mnStartPos >= rOld.mnStartPos, "Overlap type 3? How this?" ); // new left segment - const SwLineEntry aLeft( nKey, rOld.mnStartPos, rNew.mnStartPos, rOldAttr ); + const SwLineEntry aLeft(nKey, rOld.mnStartPos, rNew.mnStartPos, rOld.mbOuter, rOldAttr); // new middle segment - const SwLineEntry aMiddle( nKey, rNew.mnStartPos, rOld.mnEndPos, rCmpAttr ); + const SwLineEntry aMiddle(nKey, rNew.mnStartPos, rOld.mnEndPos, rOld.mbOuter, rCmpAttr); // new right segment rNew.mnStartPos = rOld.mnEndPos; @@ -2909,13 +2952,13 @@ void SwTabFramePainter::Insert( SwLineEntry& rNew, bool bHori ) else if ( SwLineEntry::OVERLAP2 == nOverlapType ) { // new left segment - const SwLineEntry aLeft( nKey, rOld.mnStartPos, rNew.mnStartPos, rOldAttr ); + const SwLineEntry aLeft(nKey, rOld.mnStartPos, rNew.mnStartPos, rOld.mbOuter, rOldAttr); // new middle segment - const SwLineEntry aMiddle( nKey, rNew.mnStartPos, rNew.mnEndPos, rCmpAttr ); + const SwLineEntry aMiddle(nKey, rNew.mnStartPos, rNew.mnEndPos, rOld.mbOuter, rCmpAttr); // new right segment - const SwLineEntry aRight( nKey, rNew.mnEndPos, rOld.mnEndPos, rOldAttr ); + const SwLineEntry aRight(nKey, rNew.mnEndPos, rOld.mnEndPos, rOld.mbOuter, rOldAttr); // update current lines set pLineSet->erase( aIter ); @@ -2930,13 +2973,13 @@ void SwTabFramePainter::Insert( SwLineEntry& rNew, bool bHori ) else if ( SwLineEntry::OVERLAP3 == nOverlapType ) { // new left segment - const SwLineEntry aLeft( nKey, rNew.mnStartPos, rOld.mnStartPos, rNewAttr ); + const SwLineEntry aLeft(nKey, rNew.mnStartPos, rOld.mnStartPos, rOld.mbOuter, rNewAttr); // new middle segment - const SwLineEntry aMiddle( nKey, rOld.mnStartPos, rNew.mnEndPos, rCmpAttr ); + const SwLineEntry aMiddle(nKey, rOld.mnStartPos, rNew.mnEndPos, rOld.mbOuter, rCmpAttr); // new right segment - const SwLineEntry aRight( nKey, rNew.mnEndPos, rOld.mnEndPos, rOldAttr ); + const SwLineEntry aRight(nKey, rNew.mnEndPos, rOld.mnEndPos, rOld.mbOuter, rOldAttr); // update current lines set pLineSet->erase( aIter );