Fix clipped line renderer.

Fix for a nasty corner case where supposedly clipped pixel were
still rasterized (see polytest.cxx:implTestPolyDrawClip for what
failed previously).

Added much more unit tests while at it, clippedlinerenderer.hxx
should now have 100% coverage.

4 files changed, 173 insertions, 30 deletions

diff --git a/basebmp/inc/basebmp/clippedlinerenderer.hxx b/basebmp/inc/basebmp/clippedlinerenderer.hxx
index d1c06f66e076..01262c0f4056 100644
--- a/basebmp/inc/basebmp/clippedlinerenderer.hxx
+++ b/basebmp/inc/basebmp/clippedlinerenderer.hxx
@@ -65,7 +65,8 @@ inline bool prepareClip( sal_Int32  a1,
                          sal_uInt32 bMinFlag,
                          sal_Int32  bMax,
                          sal_uInt32 bMaxFlag,
-                         bool       bRoundTowardsPt2 )
+                         bool       bRoundTowardsPt2,
+                         bool&      o_bUseAlternateBresenham )
 {
     int ca(0), cb(0);
     if( clipCode1 )
@@ -103,13 +104,13 @@ inline bool prepareClip( sal_Int32  a1,
             {
                 o_bs = b1 + cb;
                 if( o_bs > bMax )
-                    return false;
+                    return false; // fully clipped
             }
             else
             {
                 o_bs = b1 - cb;
                 if( o_bs < bMin )
-                    return false;
+                    return false; // fully clipped
             }
 
             io_rem += ca - 2*da*cb;
@@ -121,13 +122,13 @@ inline bool prepareClip( sal_Int32  a1,
             {
                 o_as = a1 + ca;
                 if( o_as > aMax )
-                    return false;
+                    return false; // fully clipped
             }
             else
             {
                 o_as = a1 - ca;
                 if( o_as < aMin )
-                    return false;
+                    return false; // fully clipped
             }
 
             io_rem += 2*db*ca - cb;
@@ -138,7 +139,6 @@ inline bool prepareClip( sal_Int32  a1,
         o_as = a1; o_bs = b1;
     }
 
-    bool bRetVal = false;
     if( clipCode2 )
     {
         if( clipCount2 == 2 )
@@ -153,13 +153,13 @@ inline bool prepareClip( sal_Int32  a1,
         else
         {
             o_n = (clipCode2 & bMinFlag) ? o_bs - bMin : bMax - o_bs;
-            bRetVal = true;
+            o_bUseAlternateBresenham = true;
         }
     }
     else
         o_n = (a2 >= o_as) ? a2 - o_as : o_as - a2;
 
-    return bRetVal;
+    return true; // at least one pixel to render
 }
 
 
@@ -214,7 +214,7 @@ void renderClippedLine( basegfx::B2IPoint             aPt1,
                                                                        rClipRect);
 
     if( clipCode1 & clipCode2 )
-        return; // line fully clipped away
+        return; // line fully clipped away, both endpoints share a half-plane
 
     sal_uInt32 clipCount1 = basegfx::tools::getNumberOfClipPlanes(clipCode1);
     sal_uInt32 clipCount2 = basegfx::tools::getNumberOfClipPlanes(clipCode2);
@@ -254,19 +254,20 @@ void renderClippedLine( basegfx::B2IPoint             aPt1,
     int n  = 0;
     sal_Int32 xs = x1;
     sal_Int32 ys = y1;
+    bool bUseAlternateBresenham=false;
     if( adx >= ady )
     {
         // semi-horizontal line
         sal_Int32 rem = 2*ady - adx - !bRoundTowardsPt2;
 
-        const bool bUseAlternateBresenham(
-            prepareClip(x1, x2, y1, adx, ady, xs, ys, sx, sy,
-                        rem, n, clipCode1, clipCount1, clipCode2, clipCount2,
-                        rClipRect.getMinX(), basegfx::tools::RectClipFlags::LEFT,
-                        rClipRect.getMaxX()-1, basegfx::tools::RectClipFlags::RIGHT,
-                        rClipRect.getMinY(), basegfx::tools::RectClipFlags::TOP,
-                        rClipRect.getMaxY()-1, basegfx::tools::RectClipFlags::BOTTOM,
-                        bRoundTowardsPt2 ));
+        if( !prepareClip(x1, x2, y1, adx, ady, xs, ys, sx, sy,
+                         rem, n, clipCode1, clipCount1, clipCode2, clipCount2,
+                         rClipRect.getMinX(), basegfx::tools::RectClipFlags::LEFT,
+                         rClipRect.getMaxX()-1, basegfx::tools::RectClipFlags::RIGHT,
+                         rClipRect.getMinY(), basegfx::tools::RectClipFlags::TOP,
+                         rClipRect.getMaxY()-1, basegfx::tools::RectClipFlags::BOTTOM,
+                         bRoundTowardsPt2, bUseAlternateBresenham ) )
+            return; // line fully clipped away, no active pixel inside rect
 
         Iterator currIter( begin + vigra::Diff2D(0,ys) );
         typename vigra::IteratorTraits<Iterator>::row_iterator
@@ -283,6 +284,8 @@ void renderClippedLine( basegfx::B2IPoint             aPt1,
 
                 if( rem >= 0 )
                 {
+                    // this is intended - we clip endpoint against y
+                    // plane, so n here denotes y range to render
                     if( --n < 0 )
                         break;
 
@@ -335,14 +338,14 @@ void renderClippedLine( basegfx::B2IPoint             aPt1,
         // semi-vertical line
         sal_Int32 rem = 2*adx - ady - !bRoundTowardsPt2;
 
-        const bool bUseAlternateBresenham(
-            prepareClip(y1, y2, x1, ady, adx, ys, xs, sy, sx,
-                        rem, n, clipCode1, clipCount1, clipCode2, clipCount2,
-                        rClipRect.getMinY(), basegfx::tools::RectClipFlags::TOP,
-                        rClipRect.getMaxY()-1, basegfx::tools::RectClipFlags::BOTTOM,
-                        rClipRect.getMinX(), basegfx::tools::RectClipFlags::LEFT,
-                        rClipRect.getMaxX()-1, basegfx::tools::RectClipFlags::RIGHT,
-                        bRoundTowardsPt2 ));
+        if( !prepareClip(y1, y2, x1, ady, adx, ys, xs, sy, sx,
+                         rem, n, clipCode1, clipCount1, clipCode2, clipCount2,
+                         rClipRect.getMinY(), basegfx::tools::RectClipFlags::TOP,
+                         rClipRect.getMaxY()-1, basegfx::tools::RectClipFlags::BOTTOM,
+                         rClipRect.getMinX(), basegfx::tools::RectClipFlags::LEFT,
+                         rClipRect.getMaxX()-1, basegfx::tools::RectClipFlags::RIGHT,
+                         bRoundTowardsPt2, bUseAlternateBresenham ) )
+            return; // line fully clipped away, no active pixel inside rect
 
         Iterator currIter( begin + vigra::Diff2D(xs,0) );
         typename vigra::IteratorTraits<Iterator>::column_iterator
@@ -359,6 +362,8 @@ void renderClippedLine( basegfx::B2IPoint             aPt1,
 
                 if( rem >= 0 )
                 {
+                    // this is intended - we clip endpoint against x
+                    // plane, so n here denotes x range to render
                     if( --n < 0 )
                         break;
 
diff --git a/basebmp/inc/basebmp/debug.hxx b/basebmp/inc/basebmp/debug.hxx
index 0193af7e1498..0a8c7219318d 100644
--- a/basebmp/inc/basebmp/debug.hxx
+++ b/basebmp/inc/basebmp/debug.hxx
@@ -46,6 +46,16 @@ namespace basebmp
         Stream to write output to.
 
         Used in vcl/headless/svpgdi.cxx when OSL_DEBUG_LEVEL > 2
+
+        Use like this:
+<pre>
+        #include <basebmp/debug.hxx>
+        #include <iostream>
+        #include <fstream>
+
+        std::ofstream output("/tmp/my_test.dump");
+        debugDump( pMyDevice, output );
+</pre>
     */
     void BASEBMP_DLLPUBLIC debugDump( const boost::shared_ptr< BitmapDevice >& rDevice,
                                       ::std::ostream&                          rOutputStream );
diff --git a/basebmp/test/linetest.cxx b/basebmp/test/linetest.cxx
index 885235d128c3..6e0297848c81 100644
--- a/basebmp/test/linetest.cxx
+++ b/basebmp/test/linetest.cxx
@@ -171,6 +171,38 @@ public:
                                            Format::THIRTYTWO_BIT_TC_MASK );
     }
 
+    void testCornerCases()
+    {
+        const basegfx::B2ISize aSize(1,1);
+        BitmapDeviceSharedPtr pDevice = createBitmapDevice(
+            aSize,
+            true,
+            Format::ONE_BIT_MSB_PAL );
+
+        const basegfx::B2IPoint aPt1(0,0);
+        const basegfx::B2IPoint aPt2(10,10);
+        CPPUNIT_ASSERT_MESSAGE("only pixel cleared",
+                                pDevice->getPixelData(aPt1) == 0);
+
+        const Color aCol(0xFFFFFFFF);
+        pDevice->drawLine( aPt1, aPt2, aCol, DrawMode_PAINT );
+        CPPUNIT_ASSERT_MESSAGE("only pixel set",
+                               pDevice->getPixelData(aPt1) == 1);
+
+        const basegfx::B2ISize aSize2(1,0);
+        pDevice = createBitmapDevice(
+            aSize2,
+            true,
+            Format::ONE_BIT_MSB_PAL );
+
+        CPPUNIT_ASSERT_MESSAGE("only pixel cleared",
+                                pDevice->getPixelData(aPt1) == 0);
+
+        pDevice->drawLine( aPt1, aPt2, aCol, DrawMode_PAINT );
+        CPPUNIT_ASSERT_MESSAGE("only pixel still cleared",
+                               pDevice->getPixelData(aPt1) == 0);
+    }
+
     void testBasicDiagonalLines()
     {
         implTestBasicDiagonalLines( mpDevice1bpp );
@@ -202,6 +234,7 @@ public:
     // because these macros are need by auto register mechanism.
 
     CPPUNIT_TEST_SUITE(LineTest);
+    CPPUNIT_TEST(testCornerCases);
     CPPUNIT_TEST(testBasicDiagonalLines);
     CPPUNIT_TEST(testBasicHorizontalLines);
     CPPUNIT_TEST(testBasicVerticalLines);
diff --git a/basebmp/test/polytest.cxx b/basebmp/test/polytest.cxx
index 8cc51d4921ee..63485000b7f9 100644
--- a/basebmp/test/polytest.cxx
+++ b/basebmp/test/polytest.cxx
@@ -65,7 +65,6 @@ private:
         const Color aBgCol(0);
         rDevice->clear(aBgCol);
         basegfx::B2DPolyPolygon aPoly;
-        ::rtl::OUString aSvg;
 
         basegfx::tools::importFromSvgD(
             aPoly,
@@ -100,7 +99,6 @@ private:
         const Color aBgCol(0);
         rDevice->clear(aBgCol);
         basegfx::B2DPolyPolygon aPoly;
-        ::rtl::OUString aSvg;
 
         basegfx::tools::importFromSvgD(
             aPoly,
@@ -150,7 +148,6 @@ private:
         const Color aBgCol(0);
         rDevice->clear(aBgCol);
         basegfx::B2DPolyPolygon aPoly;
-        ::rtl::OUString aSvg;
 
         basegfx::tools::importFromSvgD( aPoly,
                                         ::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM(
@@ -170,7 +167,6 @@ private:
         const Color aBgCol(0);
         rDevice->clear(aBgCol);
         basegfx::B2DPolyPolygon aPoly;
-        ::rtl::OUString aSvg;
 
         basegfx::tools::importFromSvgD( aPoly,
                                         ::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM(
@@ -204,13 +200,98 @@ private:
                                countPixel( rDevice, aCol ) == 7);
     }
 
+    void implTestLineDrawClip(const BitmapDeviceSharedPtr& rDevice)
+    {
+        const Color aCol(0xFFFFFFFF);
+        const Color aBgCol(0);
+        rDevice->clear(aBgCol);
+
+        // create rectangular subset, such that we can 'see' extra
+        // pixel outside
+        BitmapDeviceSharedPtr pClippedDevice=(
+            subsetBitmapDevice( rDevice,
+                                basegfx::B2IBox(3,3,5,9) ));
+
+        // trigger "alternate bresenham" case in
+        // clippedlinerenderer.hxx, first point not clipped
+        const basegfx::B2IPoint aPt1(3,3);
+        const basegfx::B2IPoint aPt2(4,2);
+        pClippedDevice->drawLine( aPt1, aPt2, aCol, DrawMode_PAINT );
+
+        CPPUNIT_ASSERT_MESSAGE("number of rendered pixel is not 1",
+                               countPixel( rDevice, aCol ) == 1);
+
+        // trigger "alternate bresenham" case in
+        // clippedlinerenderer.hxx, both start and endpoint clipped
+        const basegfx::B2IPoint aPt3(0,4);
+        pClippedDevice->drawLine( aPt3, aPt2, aCol, DrawMode_XOR );
+
+        CPPUNIT_ASSERT_MESSAGE("number of rendered pixel is not 0",
+                               countPixel( rDevice, aCol ) == 0);
+
+        // trigger "standard bresenham" case in
+        // clippedlinerenderer.hxx, first point not clipped
+        const basegfx::B2IPoint aPt4(6,2);
+        pClippedDevice->drawLine( aPt1, aPt4, aCol, DrawMode_PAINT );
+
+        CPPUNIT_ASSERT_MESSAGE("number of rendered pixel is not 2",
+                               countPixel( rDevice, aCol ) == 2);
+
+        // trigger "clipCode1 & aMinFlag/bMinFlag" cases in
+        // clippedlinerenderer.hxx (note1: needs forcing end point to
+        // be clipped as well, otherwise optimisation kicks in. note2:
+        // needs forcing end point to clip on two edges, not only on
+        // one, otherwise swap kicks in)
+        const basegfx::B2IPoint aPt5(1,1);
+        const basegfx::B2IPoint aPt6(6,10);
+        pClippedDevice->drawLine( aPt5, aPt6, aCol, DrawMode_XOR );
+
+        CPPUNIT_ASSERT_MESSAGE("number of rendered pixel is not 6",
+                               countPixel( rDevice, aCol ) == 6);
+
+        // trigger "clipCode1 & (aMinFlag|aMaxFlag)" case in
+        // clippedlinerenderer.hxx that was not taken for the test
+        // above
+        pClippedDevice->drawLine( aPt3, aPt6, aCol, DrawMode_XOR );
+
+        CPPUNIT_ASSERT_MESSAGE("number of rendered pixel is not 8",
+                               countPixel( rDevice, aCol ) == 8);
+
+    }
+
+    void implTestPolyDrawClip(const BitmapDeviceSharedPtr& rDevice)
+    {
+        const Color aCol(0xFFFFFFFF);
+        const Color aBgCol(0);
+        rDevice->clear(aBgCol);
+        basegfx::B2DPolyPolygon aPoly;
+
+        // test all corner-touching lines of our clip rect. note that
+        // *all* of the four two-pixel lines in that polygon do *not*
+        // generate a single pixel, due to the rasterization effect.
+        basegfx::tools::importFromSvgD( aPoly,
+                                        ::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM(
+                                            "M2 3 l1 -1 M4 2 l1 1 M2 8 l1 1 M5 8 l-1 1 M2 5 h4 M3 0 v10" )) );
+        BitmapDeviceSharedPtr pClippedDevice=(
+            subsetBitmapDevice( rDevice,
+                                basegfx::B2IBox(3,3,5,9) ));
+
+        for( unsigned int i=0; i<aPoly.count(); ++i )
+            pClippedDevice->drawPolygon(
+                aPoly.getB2DPolygon(i),
+                aCol,
+                DrawMode_PAINT );
+
+        CPPUNIT_ASSERT_MESSAGE("number of rendered pixel is not 7",
+                               countPixel( rDevice, aCol ) == 7);
+    }
+
     void implTestPolyPolyCrissCross(const BitmapDeviceSharedPtr& rDevice)
     {
         const Color aCol(0xFFFFFFFF);
         const Color aBgCol(0);
         rDevice->clear(aBgCol);
         basegfx::B2DPolyPolygon aPoly;
-        ::rtl::OUString aSvg;
 
         basegfx::tools::importFromSvgD( aPoly,
                                         ::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM(
@@ -264,6 +345,18 @@ public:
         implTestPolyPolyClip(mpDevice32bpp);
     }
 
+    void testLineDrawClip()
+    {
+        implTestLineDrawClip(mpDevice1bpp);
+        implTestLineDrawClip(mpDevice32bpp);
+    }
+
+    void testPolyDrawClip()
+    {
+        implTestPolyDrawClip(mpDevice1bpp);
+        implTestPolyDrawClip(mpDevice32bpp);
+    }
+
     void testPolyPolyCrissCross()
     {
         implTestPolyPolyCrissCross(mpDevice1bpp);
@@ -279,6 +372,8 @@ public:
     CPPUNIT_TEST(testHairline);
     CPPUNIT_TEST(testPolyPoly);
     CPPUNIT_TEST(testPolyPolyClip);
+    CPPUNIT_TEST(testLineDrawClip);
+    CPPUNIT_TEST(testPolyDrawClip);
     CPPUNIT_TEST(testPolyPolyCrissCross);
     CPPUNIT_TEST_SUITE_END();
 };