[Libreoffice-commits] core.git: Branch 'distro/lhm/libreoffice-6-1+backports' - basegfx/source vcl/qa vcl/source

Armin.Le.Grand (CIB) (via logerrit) logerrit at kemper.freedesktop.org
Sat Oct 17 08:55:21 UTC 2020


 basegfx/source/polygon/b2dpolygontools.cxx      |   15 +++---
 basegfx/source/polygon/b2dpolypolygoncutter.cxx |   36 +++++++++++++++
 vcl/qa/cppunit/pdfexport/pdfexport.cxx          |   28 ++++++------
 vcl/source/gdi/pdfwriter_impl.cxx               |   56 ++++++++++++++++++------
 4 files changed, 102 insertions(+), 33 deletions(-)

New commits:
commit e48467e85a2f5cb06eb1156f1aa539b5a66b5290
Author:     Armin.Le.Grand (CIB) <Armin.Le.Grand at me.com>
AuthorDate: Tue Mar 3 11:34:45 2020 +0100
Commit:     Thorsten Behrens <Thorsten.Behrens at CIB.de>
CommitDate: Sat Oct 17 10:54:49 2020 +0200

    tdf#130150 Improve clipping in PDF export
    
    For more info and discusson please have a look
    at the task. It reverts the change from tdf#99680
    which did a wrong paradigm change in how clip in
    Region(s) is defined and tries to fix the
    underlying error in a more correct way.
    This includes problems noted in tdf#44388 and
    tdf#113449.
    This is a decent improvement, but - due to dealing
    with numerical problems - not yet the whole healing.
    Still thinking about how to solve this for good.
    Adapted PdfExportTest::testTdf99680() and
    PdfExportTest::testTdf99680_2() as needed, empty
    clip regions are allowed again. Added comments, too.
    Had to change solvePolygonOperationAnd to work
    on ranges if both inputs *are* ranges. The AND-case
    is then completely solvable. Also increased geometry
    for transformations of clip geometries - may help
    later.
    
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/89874
    Tested-by: Jenkins
    Reviewed-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
    
    Conflicts:
            vcl/source/gdi/pdfwriter_impl.cxx
    
    Change-Id: I2370447597faa6efb81d58ee31c63654e304262e
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/104207
    Tested-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
    Reviewed-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>

diff --git a/basegfx/source/polygon/b2dpolygontools.cxx b/basegfx/source/polygon/b2dpolygontools.cxx
index 9b2406074a71..4e40ddc74860 100644
--- a/basegfx/source/polygon/b2dpolygontools.cxx
+++ b/basegfx/source/polygon/b2dpolygontools.cxx
@@ -343,15 +343,15 @@ namespace basegfx
                         const B2DPoint aPreviousPoint(aCurrentPoint);
                         aCurrentPoint = aCandidate.getB2DPoint(a);
 
-                        // cross-over in Y?
-                        const bool bCompYA(fTools::more(aPreviousPoint.getY(), rPoint.getY()));
-                        const bool bCompYB(fTools::more(aCurrentPoint.getY(), rPoint.getY()));
+                        // cross-over in Y? tdf#130150 use full precision, no need for epsilon
+                        const bool bCompYA(aPreviousPoint.getY() > rPoint.getY());
+                        const bool bCompYB(aCurrentPoint.getY() > rPoint.getY());
 
                         if(bCompYA != bCompYB)
                         {
-                            // cross-over in X?
-                            const bool bCompXA(fTools::more(aPreviousPoint.getX(), rPoint.getX()));
-                            const bool bCompXB(fTools::more(aCurrentPoint.getX(), rPoint.getX()));
+                            // cross-over in X? tdf#130150 use full precision, no need for epsilon
+                            const bool bCompXA(aPreviousPoint.getX() > rPoint.getX());
+                            const bool bCompXB(aCurrentPoint.getX() > rPoint.getX());
 
                             if(bCompXA == bCompXB)
                             {
@@ -367,7 +367,8 @@ namespace basegfx
                                     (aPreviousPoint.getX() - aCurrentPoint.getX()) /
                                     (aPreviousPoint.getY() - aCurrentPoint.getY()));
 
-                                if(fTools::more(fCompare, rPoint.getX()))
+                                // tdf#130150 use full precision, no need for epsilon
+                                if(fCompare > rPoint.getX())
                                 {
                                     bRetval = !bRetval;
                                 }
diff --git a/basegfx/source/polygon/b2dpolypolygoncutter.cxx b/basegfx/source/polygon/b2dpolypolygoncutter.cxx
index 4fd7bf369d85..1d2b3efb7363 100644
--- a/basegfx/source/polygon/b2dpolypolygoncutter.cxx
+++ b/basegfx/source/polygon/b2dpolypolygoncutter.cxx
@@ -986,6 +986,42 @@ namespace basegfx
             }
             else
             {
+                // tdf#130150 shortcut & precision: If both are simple ranges,
+                // solve based on ranges
+                if(basegfx::utils::isRectangle(rCandidateA) && basegfx::utils::isRectangle(rCandidateB))
+                {
+                    // *if* both are ranges, AND always can be solved
+                    const basegfx::B2DRange aRangeA(rCandidateA.getB2DRange());
+                    const basegfx::B2DRange aRangeB(rCandidateB.getB2DRange());
+
+                    if(aRangeA.isInside(aRangeB))
+                    {
+                        // 2nd completely inside 1st -> 2nd is result of AND
+                        return rCandidateB;
+                    }
+
+                    if(aRangeB.isInside(aRangeA))
+                    {
+                        // 2nd completely inside 1st -> 2nd is result of AND
+                        return rCandidateA;
+                    }
+
+                    // solve by intersection
+                    basegfx::B2DRange aIntersect(aRangeA);
+                    aIntersect.intersect(aRangeB);
+
+                    if(aIntersect.isEmpty())
+                    {
+                        // no overlap -> empty polygon as result of AND
+                        return B2DPolyPolygon();
+                    }
+
+                    // create polygon result
+                    return B2DPolyPolygon(
+                        basegfx::utils::createPolygonFromRect(
+                            aIntersect));
+                }
+
                 // concatenate polygons, solve crossovers and throw away all sub-polygons
                 // with a depth of < 1. This means to keep all polygons where at least two
                 // polygons do overlap.
diff --git a/vcl/qa/cppunit/pdfexport/pdfexport.cxx b/vcl/qa/cppunit/pdfexport/pdfexport.cxx
index 51234f24a30c..addd30663763 100644
--- a/vcl/qa/cppunit/pdfexport/pdfexport.cxx
+++ b/vcl/qa/cppunit/pdfexport/pdfexport.cxx
@@ -711,15 +711,17 @@ void PdfExportTest::testTdf99680()
     aZCodec.Decompress(rObjectStream, aUncompressed);
     CPPUNIT_ASSERT(aZCodec.EndCompression());
 
-    // Make sure there are no empty clipping regions.
-    OString aEmptyRegion("0 0 m h W* n");
-    auto pStart = static_cast<const char*>(aUncompressed.GetData());
-    const char* pEnd = pStart + aUncompressed.GetSize();
-    auto it = std::search(pStart, pEnd, aEmptyRegion.getStr(), aEmptyRegion.getStr() + aEmptyRegion.getLength());
-    CPPUNIT_ASSERT_EQUAL_MESSAGE("Empty clipping region detected!", it, pEnd);
+    // tdf#130150 See infos in task - short: tdf#99680 was not the
+    // correct fix, so empty clip regions are valid - allow again in tests
+    //      Make sure there are no empty clipping regions.
+    //      OString aEmptyRegion("0 0 m h W* n");
+    //      auto it = std::search(pStart, pEnd, aEmptyRegion.getStr(), aEmptyRegion.getStr() + aEmptyRegion.getLength());
+    //      CPPUNIT_ASSERT_EQUAL_MESSAGE("Empty clipping region detected!", it, pEnd);
 
     // Count save graphic state (q) and restore (Q) operators
     // and ensure their amount is equal
+    auto pStart = static_cast<const char*>(aUncompressed.GetData());
+    const char* pEnd = pStart + aUncompressed.GetSize();
     size_t nSaveCount = std::count(pStart, pEnd, 'q');
     size_t nRestoreCount = std::count(pStart, pEnd, 'Q');
     CPPUNIT_ASSERT_EQUAL_MESSAGE("Save/restore graphic state operators count mismatch!", nSaveCount, nRestoreCount);
@@ -750,15 +752,17 @@ void PdfExportTest::testTdf99680_2()
         aZCodec.Decompress(rObjectStream, aUncompressed);
         CPPUNIT_ASSERT(aZCodec.EndCompression());
 
-        // Make sure there are no empty clipping regions.
-        OString aEmptyRegion("0 0 m h W* n");
-        auto pStart = static_cast<const char*>(aUncompressed.GetData());
-        const char* pEnd = pStart + aUncompressed.GetSize();
-        auto it = std::search(pStart, pEnd, aEmptyRegion.getStr(), aEmptyRegion.getStr() + aEmptyRegion.getLength());
-        CPPUNIT_ASSERT_EQUAL_MESSAGE("Empty clipping region detected!", it, pEnd);
+        // tdf#130150 See infos in task - short: tdf#99680 was not the
+        // correct fix, so empty clip regions are valid - allow again in tests
+        //      Make sure there are no empty clipping regions.
+        //      OString aEmptyRegion("0 0 m h W* n");
+        //      auto it = std::search(pStart, pEnd, aEmptyRegion.getStr(), aEmptyRegion.getStr() + aEmptyRegion.getLength());
+        //      CPPUNIT_ASSERT_EQUAL_MESSAGE("Empty clipping region detected!", it, pEnd);
 
         // Count save graphic state (q) and restore (Q) operators
         // and ensure their amount is equal
+        auto pStart = static_cast<const char*>(aUncompressed.GetData());
+        const char* pEnd = pStart + aUncompressed.GetSize();
         size_t nSaveCount = std::count(pStart, pEnd, 'q');
         size_t nRestoreCount = std::count(pStart, pEnd, 'Q');
         CPPUNIT_ASSERT_EQUAL_MESSAGE("Save/restore graphic state operators count mismatch!", nSaveCount, nRestoreCount);
diff --git a/vcl/source/gdi/pdfwriter_impl.cxx b/vcl/source/gdi/pdfwriter_impl.cxx
index 7b098aa4fdf8..7834a8a0f733 100644
--- a/vcl/source/gdi/pdfwriter_impl.cxx
+++ b/vcl/source/gdi/pdfwriter_impl.cxx
@@ -10255,8 +10255,15 @@ void PDFWriterImpl::updateGraphicsState(Mode const mode)
                 if ( rNewState.m_aClipRegion.count() )
                 {
                     m_aPages.back().appendPolyPolygon( rNewState.m_aClipRegion, aLine );
-                    aLine.append( "W* n\n" );
                 }
+                else
+                {
+                    // tdf#130150 Need to revert tdf#99680, that breaks the
+                    // rule that an set but empty clip-region clips everything
+                    // aka draws nothing -> nothing is in an empty clip-region
+                    aLine.append( "0 0 m h " ); // NULL clip, i.e. nothing visible
+                }
+                aLine.append( "W* n\n" );
 
                 rNewState.m_aMapMode = aNewMapMode;
                 getReferenceDevice()->SetMapMode( rNewState.m_aMapMode );
@@ -10402,8 +10409,13 @@ void PDFWriterImpl::setMapMode( const MapMode& rMapMode )
 
 void PDFWriterImpl::setClipRegion( const basegfx::B2DPolyPolygon& rRegion )
 {
-    basegfx::B2DPolyPolygon aRegion = getReferenceDevice()->LogicToPixel( rRegion, m_aGraphicsStack.front().m_aMapMode );
-    aRegion = getReferenceDevice()->PixelToLogic( aRegion, m_aMapMode );
+    // tdf#130150 improve coordinate manipulations to double precision transformations
+    const basegfx::B2DHomMatrix aCurrentTransform(
+        getReferenceDevice()->GetInverseViewTransformation(m_aMapMode) *
+        getReferenceDevice()->GetViewTransformation(m_aGraphicsStack.front().m_aMapMode));
+    basegfx::B2DPolyPolygon aRegion(rRegion);
+
+    aRegion.transform(aCurrentTransform);
     m_aGraphicsStack.front().m_aClipRegion = aRegion;
     m_aGraphicsStack.front().m_bClipRegion = true;
     m_aGraphicsStack.front().m_nUpdateFlags |= GraphicsStateUpdateFlags::ClipRegion;
@@ -10413,16 +10425,26 @@ void PDFWriterImpl::moveClipRegion( sal_Int32 nX, sal_Int32 nY )
 {
     if( m_aGraphicsStack.front().m_bClipRegion && m_aGraphicsStack.front().m_aClipRegion.count() )
     {
-        Point aPoint( lcl_convert( m_aGraphicsStack.front().m_aMapMode,
-                                   m_aMapMode,
-                                   getReferenceDevice(),
-                                   Point( nX, nY ) ) );
-        aPoint -= lcl_convert( m_aGraphicsStack.front().m_aMapMode,
-                               m_aMapMode,
-                               getReferenceDevice(),
-                               Point() );
+        // tdf#130150 improve coordinate manipulations to double precision transformations
+        basegfx::B2DHomMatrix aConvertA;
+
+        if(MapUnit::MapPixel == m_aGraphicsStack.front().m_aMapMode.GetMapUnit())
+        {
+            aConvertA = getReferenceDevice()->GetInverseViewTransformation(m_aMapMode);
+        }
+        else
+        {
+            aConvertA = getReferenceDevice()->LogicToLogic(m_aGraphicsStack.front().m_aMapMode, m_aMapMode);
+        }
+
+        basegfx::B2DPoint aB2DPointA(nX, nY);
+        basegfx::B2DPoint aB2DPointB(0.0, 0.0);
+        aB2DPointA *= aConvertA;
+        aB2DPointB *= aConvertA;
+        aB2DPointA -= aB2DPointB;
         basegfx::B2DHomMatrix aMat;
-        aMat.translate( aPoint.X(), aPoint.Y() );
+
+        aMat.translate(aB2DPointA.getX(), aB2DPointA.getY());
         m_aGraphicsStack.front().m_aClipRegion.transform( aMat );
         m_aGraphicsStack.front().m_nUpdateFlags |= GraphicsStateUpdateFlags::ClipRegion;
     }
@@ -10437,9 +10459,15 @@ void PDFWriterImpl::intersectClipRegion( const tools::Rectangle& rRect )
 
 void PDFWriterImpl::intersectClipRegion( const basegfx::B2DPolyPolygon& rRegion )
 {
-    basegfx::B2DPolyPolygon aRegion( getReferenceDevice()->LogicToPixel( rRegion, m_aGraphicsStack.front().m_aMapMode ) );
-    aRegion = getReferenceDevice()->PixelToLogic( aRegion, m_aMapMode );
+    // tdf#130150 improve coordinate manipulations to double precision transformations
+    const basegfx::B2DHomMatrix aCurrentTransform(
+        getReferenceDevice()->GetInverseViewTransformation(m_aMapMode) *
+        getReferenceDevice()->GetViewTransformation(m_aGraphicsStack.front().m_aMapMode));
+    basegfx::B2DPolyPolygon aRegion(rRegion);
+
+    aRegion.transform(aCurrentTransform);
     m_aGraphicsStack.front().m_nUpdateFlags |= GraphicsStateUpdateFlags::ClipRegion;
+
     if( m_aGraphicsStack.front().m_bClipRegion )
     {
         basegfx::B2DPolyPolygon aOld( basegfx::utils::prepareForPolygonOperation( m_aGraphicsStack.front().m_aClipRegion ) );


More information about the Libreoffice-commits mailing list