Re: [REVIEW][3-5] fdo#38215: sw: fix consecutive border line gaps regression

On 20/04/12 14:11, Caolán McNamara wrote:
> On Mon, 2012-04-16 at 17:19 +0200, Michael Stahl wrote:
> > this fixes a regression in painting border lines in LO 3.4, leading to
> > small visible gaps between borders of consecutive paragraphs, and is
> > proposed for libreoffice-3-5.
> >
> > fixed by re-implementing the merging of border lines that are less than
> > 1.5 pixel apart that was done in the old paint implementation.
>
> > http://cgit.freedesktop.org/libreoffice/core/commit/?id=0868a0155a2b57daf7b862d120aead0458372b17
> > http://cgit.freedesktop.org/libreoffice/core/commit/?id=44092833d3a0f0d6074c64bd0e64bbdf11109afe
>
> So, looks reasonable to me, except it doesn't apply cleanly to 3-5, can
> you tweak it to apply to 3-5 and we can push that then to 3-5.

it requires this one first:

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1024c172a5bfb3d85a86fcf7a046aa2b03950edd

but then i still got a trivial conflict, so i've merged the 2 commits
into one that applies on top of the above, see attachment.

> And why
> the change from ::std::swap to a manual push_back of each line to new
> SequenceAsVector followed by clear on the original ?

because the types of the elements are different, the return value has
uno::Reference of XFooPrimitive while the member is changed to contain
rtl::Reference of BorderLinePrimitive so it's not necessary to
dynamic_cast while iterating over it.

From 0919f42d62b658fc39be1be269fdfea0892f8a2c Mon Sep 17 00:00:00 2001
From: Michael Stahl <mstahl@redhat.com>
Date: Mon, 16 Apr 2012 16:12:36 +0200
Subject: [PATCH] fdo#38215: merge consecutive border lines:

This re-implements the merging that was done by SwLineRects::AddLineRect,
SwLineRect::MakeUnion with the drawing layer border lines.
This is used to merge borders of paragraphs and of tables that have the
"separating" border-model, which fixes both the tiny dividing gaps
between successive borders in the second bugdoc and the weird subtly
differently rendered successive borders in the first bugdoc.
(regression from 0f0896c26fb260d1bbf31d7a886df3f61837f0f2)

(cherry-picked from 0868a0155a2b57daf7b862d120aead0458372b17
 and 44092833d3a0f0d6074c64bd0e64bbdf11109afe)

Conflicts:

        sw/source/core/layout/paintfrm.cxx
---
 sw/source/core/layout/paintfrm.cxx |  144 +++++++++++++++++++++++++++++++++---
 1 files changed, 132 insertions(+), 12 deletions(-)

diff --git a/sw/source/core/layout/paintfrm.cxx b/sw/source/core/layout/paintfrm.cxx
index 1fd7013..797f3af 100755
--- a/sw/source/core/layout/paintfrm.cxx
+++ b/sw/source/core/layout/paintfrm.cxx
@@ -132,6 +132,9 @@
 
 using namespace ::editeng;
 using namespace ::com::sun::star;
+using ::drawinglayer::primitive2d::BorderLinePrimitive2D;
+using ::std::pair;
+using ::std::make_pair;
 
 #define GETOBJSHELL()       ((SfxObjectShell*)rSh.GetDoc()->GetDocShell())
 
@@ -223,19 +226,21 @@ public:
 
 class BorderLines
 {
-    ::comphelper::SequenceAsVector<
-         ::drawinglayer::primitive2d::Primitive2DReference> m_Lines;
+    typedef ::comphelper::SequenceAsVector<
+        ::rtl::Reference<BorderLinePrimitive2D> > Lines_t;
+    Lines_t m_Lines;
 public:
-    void AddBorderLine(
-            ::drawinglayer::primitive2d::Primitive2DReference const& xLine)
-    {
-        m_Lines.push_back(xLine);
-    }
+    void AddBorderLine(::rtl::Reference<BorderLinePrimitive2D> const& xLine);
     drawinglayer::primitive2d::Primitive2DSequence GetBorderLines_Clear()
     {
         ::comphelper::SequenceAsVector<
             ::drawinglayer::primitive2d::Primitive2DReference> lines;
-        ::std::swap(m_Lines, lines);
+        for (Lines_t::const_iterator it = m_Lines.begin(); it != m_Lines.end();
+                ++it)
+        {
+            lines.push_back(it->get());
+        }
+        m_Lines.clear();
         return lines.getAsConstList();
     }
 };
@@ -442,6 +447,121 @@ SwSavePaintStatics::~SwSavePaintStatics()
 
 SV_IMPL_VARARR( SwLRects, SwLineRect );
 
+static pair<bool, pair<double, double> >
+lcl_TryMergeLines(pair<double, double> const mergeA,
+                  pair<double, double> const mergeB)
+{
+    double const fMergeGap(nPixelSzW + nHalfPixelSzW); // NOT static!
+    if (   (mergeA.second + fMergeGap >= mergeB.first )
+        && (mergeA.first  - fMergeGap <= mergeB.second))
+    {
+        return make_pair(true, make_pair(
+                                std::min(mergeA.first, mergeB.first),
+                                std::max(mergeA.second, mergeB.second)));
+    }
+    return make_pair(false, make_pair(0, 0));
+}
+
+static ::rtl::Reference<BorderLinePrimitive2D>
+lcl_MergeBorderLines(
+    BorderLinePrimitive2D const& rLine, BorderLinePrimitive2D const& rOther,
+    basegfx::B2DPoint const& rStart, basegfx::B2DPoint const& rEnd)
+{
+    return new BorderLinePrimitive2D(rStart, rEnd,
+                rLine.getLeftWidth(),
+                rLine.getDistance(),
+                rLine.getRightWidth(),
+                rLine.getExtendLeftStart(),
+                rOther.getExtendLeftEnd(),
+                rLine.getExtendRightStart(),
+                rOther.getExtendRightEnd(),
+                rLine.getRGBColorLeft(),
+                rLine.getRGBColorGap(),
+                rLine.getRGBColorRight(),
+                rLine.hasGapColor(),
+                rLine.getStyle());
+}
+
+static ::rtl::Reference<BorderLinePrimitive2D>
+lcl_TryMergeBorderLine(BorderLinePrimitive2D const& rThis,
+                       BorderLinePrimitive2D const& rOther)
+{
+    assert(rThis.getEnd().getX() >= rThis.getStart().getX());
+    assert(rThis.getEnd().getY() >= rThis.getStart().getY());
+    assert(rOther.getEnd().getX() >= rOther.getStart().getX());
+    assert(rOther.getEnd().getY() >= rOther.getStart().getY());
+    double thisHeight = rThis.getEnd().getY() - rThis.getStart().getY();
+    double thisWidth  = rThis.getEnd().getX() - rThis.getStart().getX();
+    double otherHeight = rOther.getEnd().getY() -  rOther.getStart().getY();
+    double otherWidth  = rOther.getEnd().getX() -  rOther.getStart().getX();
+    // check for same orientation, same line width and matching colors
+    if (    ((thisHeight > thisWidth) == (otherHeight > otherWidth))
+        &&  (rThis.getLeftWidth()     == rOther.getLeftWidth())
+        &&  (rThis.getDistance()      == rOther.getDistance())
+        &&  (rThis.getRightWidth()    == rOther.getRightWidth())
+        &&  (rThis.getRGBColorLeft()  == rOther.getRGBColorLeft())
+        &&  (rThis.getRGBColorRight() == rOther.getRGBColorRight())
+        &&  (rThis.hasGapColor()      == rOther.hasGapColor())
+        &&  (!rThis.hasGapColor() ||
+             (rThis.getRGBColorGap()  == rOther.getRGBColorGap())))
+    {
+        if (thisHeight > thisWidth) // vertical line
+        {
+            if (rThis.getStart().getX() == rOther.getStart().getX())
+            {
+                assert(rThis.getEnd().getX() == rOther.getEnd().getX());
+                pair<bool, pair<double, double> > const res = lcl_TryMergeLines(
+                    make_pair(rThis.getStart().getY(), rThis.getEnd().getY()),
+                    make_pair(rOther.getStart().getY(),rOther.getEnd().getY()));
+                if (res.first) // merge them
+                {
+                    basegfx::B2DPoint const start(
+                            rThis.getStart().getX(), res.second.first);
+                    basegfx::B2DPoint const end(
+                            rThis.getStart().getX(), res.second.second);
+                    return lcl_MergeBorderLines(rThis, rOther, start, end);
+                }
+            }
+        }
+        else // horizontal line
+        {
+            if (rThis.getStart().getY() == rOther.getStart().getY())
+            {
+                assert(rThis.getEnd().getY() == rOther.getEnd().getY());
+                pair<bool, pair<double, double> > const res = lcl_TryMergeLines(
+                    make_pair(rThis.getStart().getX(), rThis.getEnd().getX()),
+                    make_pair(rOther.getStart().getX(),rOther.getEnd().getX()));
+                if (res.first) // merge them
+                {
+                    basegfx::B2DPoint const start(
+                            res.second.first, rThis.getStart().getY());
+                    basegfx::B2DPoint const end(
+                            res.second.second, rThis.getEnd().getY());
+                    return lcl_MergeBorderLines(rThis, rOther, start, end);
+                }
+            }
+        }
+    }
+    return 0;
+}
+
+void BorderLines::AddBorderLine(
+        rtl::Reference<BorderLinePrimitive2D> const& xLine)
+{
+    for (Lines_t::reverse_iterator it = m_Lines.rbegin(); it != m_Lines.rend();
+         ++it)
+    {
+        ::rtl::Reference<BorderLinePrimitive2D> const xMerged =
+            lcl_TryMergeBorderLine(**it, *xLine);
+        if (xMerged.is())
+        {
+            *it = xMerged; // replace existing line with merged
+            return;
+        }
+    }
+    m_Lines.push_back(xLine);
+}
+
 SwLineRect::SwLineRect( const SwRect &rRect, const Color *pCol, const SvxBorderStyle nStyl,
                         const SwTabFrm *pT, const sal_uInt8 nSCol ) :
     SwRect( rRect ),
@@ -4519,8 +4639,8 @@ void lcl_PaintLeftRightLine( const sal_Bool         _bLeft,
         Color aLeftColor = _bLeft ? pLeftRightBorder->GetColorOut( _bLeft ) : 
pLeftRightBorder->GetColorIn( _bLeft );
         Color aRightColor = _bLeft ? pLeftRightBorder->GetColorIn( _bLeft ) : 
pLeftRightBorder->GetColorOut( _bLeft );
 
-        drawinglayer::primitive2d::Primitive2DReference xLine =
-            new drawinglayer::primitive2d::BorderLinePrimitive2D(
+        ::rtl::Reference<BorderLinePrimitive2D> xLine =
+            new BorderLinePrimitive2D(
                 aStart, aEnd, nLeftWidth, pLeftRightBorder->GetDistance(), nRightWidth,
                 nExtentIS, nExtentIE, nExtentOS, nExtentOE,
                 aLeftColor.getBColor(), aRightColor.getBColor(),
@@ -4601,8 +4721,8 @@ void lcl_PaintTopBottomLine( const sal_Bool         _bTop,
         Color aLeftColor = _bTop ? pTopBottomBorder->GetColorOut( _bTop ) : 
pTopBottomBorder->GetColorIn( _bTop );
         Color aRightColor = _bTop ? pTopBottomBorder->GetColorIn( _bTop ) : 
pTopBottomBorder->GetColorOut( _bTop );
 
-        drawinglayer::primitive2d::Primitive2DReference xLine =
-            new drawinglayer::primitive2d::BorderLinePrimitive2D(
+        ::rtl::Reference<BorderLinePrimitive2D> xLine =
+            new BorderLinePrimitive2D(
                 aStart, aEnd, nLeftWidth, pTopBottomBorder->GetDistance(), nRightWidth,
                 nExtentIS, nExtentIE, nExtentOS, nExtentOE,
                 aLeftColor.getBColor(), aRightColor.getBColor(),
-- 
1.7.7.6