sw/qa/core/layout/data/vmerge-cell-border.docx |binary
 sw/qa/core/layout/layout.cxx                   |   74 ++++++++++++++++++++++
 sw/source/core/inc/cellfrm.hxx                 |    6 +
 sw/source/core/layout/paintfrm.cxx             |   64 +++++++++++++++++++
 sw/source/core/layout/tabfrm.cxx               |   84 +++++++++++++++++++++++++
 5 files changed, 228 insertions(+)

New commits:
commit 66ac8e60896f6306bed8fbb34606fd14474f19ce
Author:     Miklos Vajna <vmik...@collabora.com>
AuthorDate: Thu Mar 4 11:52:58 2021 +0100
Commit:     Miklos Vajna <vmik...@collabora.com>
CommitDate: Thu Mar 4 12:49:18 2021 +0100

    sw: fix unwanted long vertical border around vertically merged Word cell
    
    In case cells A1 and A2 are vertically merged, they can still specify
    different border properties for the two cells. Word draws the border
    properties of the covered cells, while Writer ignores the formatting of
    A2 in this case and only uses the properties of the covering cell.
    
    Table cell border collapsing rules already differ in Word and Writer, so
    SwTabFramePainter::Insert() knows if the table cell's border is
    Word-style or not. Extend this code to handle vertically merged cells
    better.
    
    In general, this means a cell no longer has a fixed set of 4 borders,
    but each edge may have several sub-borders. This commit is a step in
    that direction, and handles the case when a vertical (left or right)
    border style is set initially, but not set at the end -- as we iterate
    through the list of cells in a vertical merge.
    
    Change-Id: I35a05097ce70243099307ce8066766aef61a2bc5
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111954
    Reviewed-by: Miklos Vajna <vmik...@collabora.com>
    Tested-by: Jenkins

diff --git a/sw/qa/core/layout/data/vmerge-cell-border.docx 
b/sw/qa/core/layout/data/vmerge-cell-border.docx
new file mode 100644
index 000000000000..11d7a76d22ec
Binary files /dev/null and b/sw/qa/core/layout/data/vmerge-cell-border.docx 
differ
diff --git a/sw/qa/core/layout/layout.cxx b/sw/qa/core/layout/layout.cxx
index 59af4962263f..7c899aa31d63 100644
--- a/sw/qa/core/layout/layout.cxx
+++ b/sw/qa/core/layout/layout.cxx
@@ -389,6 +389,80 @@ CPPUNIT_TEST_FIXTURE(SwCoreLayoutTest, 
testGutterMarginPageBorder)
 #endif
 }
 
+CPPUNIT_TEST_FIXTURE(SwCoreLayoutTest, testVerticallyMergedCellBorder)
+{
+    // Given a document with a table: 2 columns, 5 rows. B2 -> B5 is merged:
+    SwDoc* pDoc = createSwDoc(DATA_DIRECTORY, "vmerge-cell-border.docx");
+    SwDocShell* pShell = pDoc->GetDocShell();
+
+    // When rendering the table:
+    std::shared_ptr<GDIMetaFile> xMetaFile = pShell->GetPreviewMetaFile();
+
+    // Make sure that B4->B5 has no borders.
+    MetafileXmlDump dumper;
+    xmlDocUniquePtr pXmlDoc = dumpAndParse(dumper, *xMetaFile);
+    // Collect vertical positions of all border points.
+    xmlXPathObjectPtr pXmlObj = getXPathNode(pXmlDoc, 
"//polyline[@style='solid']/point");
+    xmlNodeSetPtr pXmlNodes = pXmlObj->nodesetval;
+    std::vector<sal_Int32> aBorderPositions;
+    for (int i = 0; i < xmlXPathNodeSetGetLength(pXmlNodes); ++i)
+    {
+        xmlNodePtr pXmlNode = pXmlNodes->nodeTab[i];
+        xmlChar* pValue = xmlGetProp(pXmlNode, BAD_CAST("y"));
+        sal_Int32 nValue = OString(reinterpret_cast<char 
const*>(pValue)).toInt32();
+        aBorderPositions.push_back(nValue);
+    }
+    xmlXPathFreeObject(pXmlObj);
+    // Collect top and bottom of the B1->B3 rows.
+    xmlDocUniquePtr pLayout = parseLayoutDump();
+    pXmlObj = getXPathNode(pLayout, "//tab/row/infos/bounds");
+    pXmlNodes = pXmlObj->nodesetval;
+    std::vector<sal_Int32> aLayoutPositions;
+    for (int i = 0; i < 3; ++i)
+    {
+        xmlNodePtr pXmlNode = pXmlNodes->nodeTab[i];
+        if (i == 0)
+        {
+            xmlChar* pValue = xmlGetProp(pXmlNode, BAD_CAST("top"));
+            sal_Int32 nValue = OString(reinterpret_cast<char 
const*>(pValue)).toInt32();
+            aLayoutPositions.push_back(nValue);
+        }
+        xmlChar* pValue = xmlGetProp(pXmlNode, BAD_CAST("bottom"));
+        sal_Int32 nValue = OString(reinterpret_cast<char 
const*>(pValue)).toInt32();
+        aLayoutPositions.push_back(nValue);
+    }
+    xmlXPathFreeObject(pXmlObj);
+    // Check if any border is outside the B1->B3 range.
+    for (const auto nBorderPosition : aBorderPositions)
+    {
+        bool bFound = false;
+        for (const auto nLayoutPosition : aLayoutPositions)
+        {
+            if (std::abs(nBorderPosition - nLayoutPosition) <= 15)
+            {
+                bFound = true;
+                break;
+            }
+        }
+        std::stringstream ss;
+        ss << "Bad vertical position for border point: " << nBorderPosition;
+        ss << " Expected positions: ";
+        for (size_t i = 0; i < aLayoutPositions.size(); ++i)
+        {
+            if (i > 0)
+            {
+                ss << ", ";
+            }
+            ss << aLayoutPositions[i];
+        }
+
+        // Without the accompanying fix in place, this test would have failed 
with:
+        // - Bad vertical position for border point: 5624 Expected positions: 
3022, 3540, 4059, 4578
+        // i.e. the middle vertical border end was the bottom of B5, not 
bottom of B3.
+        CPPUNIT_ASSERT_MESSAGE(ss.str(), bFound);
+    }
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/inc/cellfrm.hxx b/sw/source/core/inc/cellfrm.hxx
index d2f8730691ed..e8f894af3058 100644
--- a/sw/source/core/inc/cellfrm.hxx
+++ b/sw/source/core/inc/cellfrm.hxx
@@ -62,6 +62,12 @@ public:
     const SwCellFrame& FindStartEndOfRowSpanCell( bool bStart ) const;
     tools::Long GetLayoutRowSpan() const;
 
+    /// If this is a vertically merged cell, then looks up its covered cell in 
rRow.
+    const SwCellFrame* GetCoveredCellInRow(const SwRowFrame& rRow) const;
+
+    /// If this is a vertically merged cell, then looks up its covered cells.
+    std::vector<const SwCellFrame*> GetCoveredCells() const;
+
     void dumpAsXmlAttributes(xmlTextWriterPtr writer) const override;
 };
 
diff --git a/sw/source/core/layout/paintfrm.cxx 
b/sw/source/core/layout/paintfrm.cxx
index c549b5ffeb46..e90ab0bad378 100644
--- a/sw/source/core/layout/paintfrm.cxx
+++ b/sw/source/core/layout/paintfrm.cxx
@@ -2241,11 +2241,14 @@ struct SwLineEntry
     SwTwips mnKey;
     SwTwips mnStartPos;
     SwTwips mnEndPos;
+    SwTwips mnLimitedEndPos;
 
     svx::frame::Style maAttribute;
 
     enum OverlapType { NO_OVERLAP, OVERLAP1, OVERLAP2, OVERLAP3 };
 
+    enum class VerticalType { LEFT, RIGHT };
+
 public:
     SwLineEntry( SwTwips nKey,
                  SwTwips nStartPos,
@@ -2253,6 +2256,12 @@ public:
                  const svx::frame::Style& rAttribute );
 
     OverlapType Overlaps( const SwLineEntry& rComp ) const;
+
+    /**
+     * Assuming that this entry is for a Word-style covering cell and the 
border matching eType is
+     * set, limit the end position of this border in case covered cells have 
no borders set.
+     */
+    void LimitVerticalEndPos(const SwFrame& rFrame, VerticalType eType);
 };
 
 }
@@ -2264,6 +2273,7 @@ SwLineEntry::SwLineEntry( SwTwips nKey,
     :   mnKey( nKey ),
         mnStartPos( nStartPos ),
         mnEndPos( nEndPos ),
+        mnLimitedEndPos(0),
         maAttribute( rAttribute )
 {
 }
@@ -2317,6 +2327,37 @@ SwLineEntry::OverlapType SwLineEntry::Overlaps( const 
SwLineEntry& rNew )  const
     return eRet;
 }
 
+void SwLineEntry::LimitVerticalEndPos(const SwFrame& rFrame, VerticalType 
eType)
+{
+    if (!rFrame.IsCellFrame())
+    {
+        return;
+    }
+
+    const auto& rCellFrame = static_cast<const SwCellFrame&>(rFrame);
+    std::vector<const SwCellFrame*> aCoveredCells = 
rCellFrame.GetCoveredCells();
+    // Iterate in reverse order, so we can stop at the first cell that has a 
border. This can
+    // determine what is the minimal end position that is safe to use as a 
limit.
+    for (auto it = aCoveredCells.rbegin(); it != aCoveredCells.rend(); ++it)
+    {
+        const SwCellFrame* pCoveredCell = *it;
+        SwBorderAttrAccess aAccess( SwFrame::GetCache(), pCoveredCell );
+        const SwBorderAttrs& rAttrs = *aAccess.Get();
+        const SvxBoxItem& rBox = rAttrs.GetBox();
+        if (eType == VerticalType::LEFT && rBox.GetLeft())
+        {
+            break;
+        }
+
+        if (eType == VerticalType::RIGHT && rBox.GetRight())
+        {
+            break;
+        }
+
+        mnLimitedEndPos = pCoveredCell->getFrameArea().Top();
+    }
+}
+
 namespace {
 
 struct lt_SwLineEntry
@@ -2460,6 +2501,12 @@ void SwTabFramePainter::PaintLines(OutputDevice& rDev, 
const SwRect& rRect) cons
             svx::frame::Style aStyles[ 7 ];
             aStyles[ 0 ] = rEntryStyle;
             FindStylesForLine( aStart, aEnd, aStyles, bHori );
+
+            if (!bHori && rEntry.mnLimitedEndPos)
+            {
+                aEnd.setY(rEntry.mnLimitedEndPos);
+            }
+
             SwRect aRepaintRect( aStart, aEnd );
 
             // the repaint rectangle has to be moved a bit for the centered 
lines:
@@ -2780,8 +2827,17 @@ void SwTabFramePainter::Insert(const SwFrame& rFrame, 
const SvxBoxItem& rBoxItem
 
     SwLineEntry aLeft  (nLeft,   nTop,  nBottom,
             bVert ? aB                         : (bR2L ? aR : aL));
+    if (bWordTableCell && rBoxItem.GetLeft())
+    {
+        aLeft.LimitVerticalEndPos(rFrame, SwLineEntry::VerticalType::LEFT);
+    }
+
     SwLineEntry aRight (nRight,  nTop,  nBottom,
             bVert ? (bBottomAsTop ? aB : aT) : (bR2L ? aL : aR));
+    if (bWordTableCell && rBoxItem.GetRight())
+    {
+        aRight.LimitVerticalEndPos(rFrame, SwLineEntry::VerticalType::RIGHT);
+    }
     SwLineEntry aTop   (nTop,    nLeft, nRight,
             bVert ? aL                         : (bBottomAsTop ? aB : aT));
     SwLineEntry aBottom(nBottom, nLeft, nRight,
@@ -2812,6 +2868,14 @@ void SwTabFramePainter::Insert( SwLineEntry& rNew, bool 
bHori )
     while ( aIter != pLineSet->end() && rNew.mnStartPos < rNew.mnEndPos )
     {
         const SwLineEntry& rOld = *aIter;
+
+        if (rOld.mnLimitedEndPos)
+        {
+            // Don't merge with this line entry as it ends sooner than 
mnEndPos.
+            ++aIter;
+            continue;
+        }
+
         const SwLineEntry::OverlapType nOverlapType = rOld.Overlaps( rNew );
 
         const svx::frame::Style& rOldAttr = rOld.maAttribute;
diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx
index e6c8c2bc1a66..25b69f680b58 100644
--- a/sw/source/core/layout/tabfrm.cxx
+++ b/sw/source/core/layout/tabfrm.cxx
@@ -5497,6 +5497,90 @@ tools::Long SwCellFrame::GetLayoutRowSpan() const
     return  nRet;
 }
 
+const SwCellFrame* SwCellFrame::GetCoveredCellInRow(const SwRowFrame& rRow) 
const
+{
+    if (GetLayoutRowSpan() <= 1)
+    {
+        // Not merged vertically.
+        return nullptr;
+    }
+
+    for (const SwFrame* pCell = rRow.GetLower(); pCell; pCell = 
pCell->GetNext())
+    {
+        if (!pCell->IsCellFrame())
+        {
+            continue;
+        }
+
+        auto pCellFrame = static_cast<const SwCellFrame*>(pCell);
+        if (!pCellFrame->IsCoveredCell())
+        {
+            continue;
+        }
+
+        if (pCellFrame->getFrameArea().Left() != getFrameArea().Left())
+        {
+            continue;
+        }
+
+        if (pCellFrame->getFrameArea().Width() != getFrameArea().Width())
+        {
+            continue;
+        }
+
+        // pCellFrame is covered, there are only covered cell frames between 
"this" and pCellFrame
+        // and the horizontal position/size matches "this".
+        return pCellFrame;
+    }
+
+    return nullptr;
+}
+
+std::vector<const SwCellFrame*> SwCellFrame::GetCoveredCells() const
+{
+    std::vector<const SwCellFrame*> aRet;
+    if (GetLayoutRowSpan() <= 1)
+    {
+        return aRet;
+    }
+
+    if (!GetUpper()->IsRowFrame())
+    {
+        return aRet;
+    }
+
+    auto pFirstRowFrame = static_cast<const SwRowFrame*>(GetUpper());
+    if (!pFirstRowFrame->GetNext())
+    {
+        return aRet;
+    }
+
+    if (!pFirstRowFrame->GetNext()->IsRowFrame())
+    {
+        return aRet;
+    }
+
+    for (const SwFrame* pRow = pFirstRowFrame->GetNext(); pRow; pRow = 
pRow->GetNext())
+    {
+        if (!pRow->IsRowFrame())
+        {
+            continue;
+        }
+
+        auto pRowFrame = static_cast<const SwRowFrame*>(pRow);
+        const SwCellFrame* pCovered = GetCoveredCellInRow(*pRowFrame);
+        if (!pCovered)
+        {
+            continue;
+        }
+
+        // Found a cell in a next row that is covered by "this".
+        aRet.push_back(pCovered);
+    }
+
+    return aRet;
+}
+
 void SwCellFrame::dumpAsXmlAttributes(xmlTextWriterPtr pWriter) const
 {
     SwFrame::dumpAsXmlAttributes(pWriter);
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to