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.gr...@me.com> AuthorDate: Tue Mar 3 11:34:45 2020 +0100 Commit: Thorsten Behrens <thorsten.behr...@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.behr...@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.behr...@cib.de> Reviewed-by: Thorsten Behrens <thorsten.behr...@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 ) ); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits