Bugzilla – Attachment 157377 Details for
Bug 130150
Broken clipping in PDF export
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Some debug patch including a basegfx test.
0001-WIP-tdf-99680-do-not-merge.patch (text/plain), 11.86 KB, created by
Jan-Marek Glogowski
on 2020-01-23 19:27:07 UTC
(
hide
)
Description:
Some debug patch including a basegfx test.
Filename:
MIME Type:
Creator:
Jan-Marek Glogowski
Created:
2020-01-23 19:27:07 UTC
Size:
11.86 KB
patch
obsolete
>From 08402967a0cb4ce2bad864c5314f23376139948c Mon Sep 17 00:00:00 2001 >From: Jan-Marek Glogowski <jan-marek.glogowski@extern.cib.de> >Date: Wed, 22 Jan 2020 17:19:45 +0000 >Subject: [PATCH] WIP tdf#99680 - do not merge > >I have a clipping problem with a test document. In this case the >clipping of a check boxes text is wrong in the PDF export. I'll provide >a simpler test document. > >This patch reverts the original fix for tdf#99680. That changed the >semantics from m_bClipRegion without an m_aClipRegion to mean no >clipping, while the original meaning was, that nothing is visible >(see comment "NULL clip, i.e. nothing visible"). > >After adding debug to all functions modifying m_aClipRegion, it >became clear, that intersectClipRegion has a problem, presumly with >rounding. > >Therefore, with this patch applied, you can get the following output >using the test document from the original bug: > https://bugs.documentfoundation.org/attachment.cgi?id=133268 > >so --convert-to pdf --outdir ../tdf#99680/ ../tdf#99680/test.odt > >In the output look for: > >debug:8010:8010: intersectClipRegion in 1 1 1 >debug:8010:8010: 1 1 [1:<4:(810700,70500)--(810700,484400)--(31209.4,484400)--(31209.4,70500)>] >debug:8010:8010: 1 1 [1:<4:(31209.4,70486.3)--(810737,70486.3)--(810737,484501)--(31209.4,484501)>] >debug:8010:8010: stripDispensablePolygons 779491x413900@(31209.4,70500) >debug:8010:8010: stripDispensablePolygons 779528x414015@(31209.4,70486.3) >debug:8010:8010: stripDispensablePolygons a: 0 b: 1 bAInBrange 0 bAInButil 1 bBInArange 0 bBInAutil 0 >debug:8010:8010: stripDispensablePolygons a: 0 b: 1 bAInB: 0 bBInA: 0 >debug:8010:8010: [0:] >debug:8010:8010: updateGraphicsState 0 > >What fails is the "bAInBrange 0", which is "wrong", eventually due >to some rounding errors. As a result, this also fails as "bAInB: 0". > >Now > >make basegfx.check > >In workdir/CppunitTest/basegfx.test.log search for testPolyIntersection: > >debug:8291:8291: 1 1 [1:<4:(810700,70500)--(810700,484400)--(31209.4,484400)--(31209.4,70500)>] >debug:8291:8291: 1 1 [1:<4:(31209.4,70486.3)--(810737,70486.3)--(810737,484501)--(31209.4,484501)>] >debug:8291:8291: stripDispensablePolygons 779491x413900@(31209.4,70500) >debug:8291:8291: stripDispensablePolygons 779528x414015@(31209.4,70486.3) >debug:8291:8291: stripDispensablePolygons a: 0 b: 1 bAInBrange 1 bAInButil 1 bBInArange 0 bBInAutil 0 >debug:8291:8291: stripDispensablePolygons a: 0 b: 1 bAInB: 1 bBInA: 0 >debug:8291:8291: [1:<4:(810700,70500)--(810700,484400)--(31209.4,484400)--(31209.4,70500)>] > >Here "bAInBrange 1" is true and so it correctly gets "bAInB: 1". > >As you can see, both polypolygons seem to be the same, but just >the unit test correctly sees the first one to be inside the second >one (which is correct). This looks very much like a rounding error. > >Problem is I don't know who and when somebody should round. > >Later on mst__ mentioned tdf#113449, which is the same problem, just >in that / his case basegfx::utils::isInside was failing instead of >B2DRange::isInside for this / mine. > >And my GDB is segfaulting when I try to bt and dump the real values, >just to be really sure. > >Change-Id: I60900962e4ed6da2dad0cced4761c59f5a54e770 >--- > .../source/polygon/b2dpolypolygoncutter.cxx | 12 ++++++++++ > basegfx/test/B2DPolyPolygonTest.cxx | 24 +++++++++++++++++++ > vcl/source/gdi/pdfwriter_impl.cxx | 21 +++++++++++----- > vcl/source/gdi/pdfwriter_impl.hxx | 1 + > 4 files changed, 52 insertions(+), 6 deletions(-) > >diff --git a/basegfx/source/polygon/b2dpolypolygoncutter.cxx b/basegfx/source/polygon/b2dpolypolygoncutter.cxx >index fa0471403d92..15ee18cf5324 100644 >--- a/basegfx/source/polygon/b2dpolypolygoncutter.cxx >+++ b/basegfx/source/polygon/b2dpolypolygoncutter.cxx >@@ -17,6 +17,9 @@ > * the License at http://www.apache.org/licenses/LICENSE-2.0 . > */ > >+#include <sal/config.h> >+#include <sal/log.hxx> >+ > #include <basegfx/numeric/ftools.hxx> > #include <basegfx/polygon/b2dpolypolygoncutter.hxx> > #include <basegfx/point/b2dpoint.hxx> >@@ -830,6 +833,7 @@ namespace basegfx::utils > const B2DPolygon& aCandidate(rCandidate.getB2DPolygon(a)); > StripHelper* pNewHelper = &(aHelpers[a]); > pNewHelper->maRange = utils::getRange(aCandidate); >+ SAL_DEBUG(__FUNCTION__ << " " << pNewHelper->maRange); > pNewHelper->meOrinetation = utils::getOrientation(aCandidate); > pNewHelper->mnDepth = (pNewHelper->meOrinetation == B2VectorOrientation::Negative ? -1 : 0); > } >@@ -843,8 +847,16 @@ namespace basegfx::utils > { > const B2DPolygon& aCandB(rCandidate.getB2DPolygon(b)); > StripHelper& rHelperB = aHelpers[b]; >+ const bool bAInBrange = rHelperB.maRange.isInside(rHelperA.maRange); >+ const bool bBInArange = rHelperA.maRange.isInside(rHelperB.maRange); >+ const bool bAInButil = utils::isInside(aCandB, aCandA, true); >+ const bool bBInAutil = utils::isInside(aCandA, aCandB, true); >+ SAL_DEBUG(__FUNCTION__ << " a: " << a << " b: " << b << " bAInBrange " << bAInBrange << " bAInButil " >+ << bAInButil << " bBInArange " << bBInArange << " bBInAutil " << bBInAutil); >+ > const bool bAInB(rHelperB.maRange.isInside(rHelperA.maRange) && utils::isInside(aCandB, aCandA, true)); > const bool bBInA(rHelperA.maRange.isInside(rHelperB.maRange) && utils::isInside(aCandA, aCandB, true)); >+ SAL_DEBUG(__FUNCTION__ << " a: " << a << " b: " << b << " bAInB: " << bAInB << " bBInA: " << bBInA); > > if(bAInB && bBInA) > { >diff --git a/basegfx/test/B2DPolyPolygonTest.cxx b/basegfx/test/B2DPolyPolygonTest.cxx >index 1fef45328357..65657584cd94 100644 >--- a/basegfx/test/B2DPolyPolygonTest.cxx >+++ b/basegfx/test/B2DPolyPolygonTest.cxx >@@ -17,12 +17,16 @@ > * the License at http://www.apache.org/licenses/LICENSE-2.0 . > */ > >+#include <sal/config.h> >+#include <sal/log.hxx> >+ > #include <cppunit/TestAssert.h> > #include <cppunit/TestFixture.h> > #include <cppunit/extensions/HelperMacros.h> > > #include <basegfx/polygon/b2dpolypolygon.hxx> > #include <basegfx/polygon/b2dtrapezoid.hxx> >+#include <basegfx/polygon/b2dpolypolygoncutter.hxx> > > #include "boxclipper.hxx" > >@@ -65,12 +69,32 @@ public: > CPPUNIT_ASSERT_MESSAGE("more than zero sub-divided trapezoids", !aVector.empty()); > } > >+ void testPolyIntersection() >+ { >+ B2DPolygon aPolygon1{{810700,70500}, {810700,484400}, {31209.4,484400}, {31209.4,70500}}; >+ aPolygon1.setClosed(true); >+ B2DPolyPolygon aPolyPolygon1; >+ aPolyPolygon1.append(aPolygon1); >+ aPolyPolygon1.setClosed(true); >+ >+ B2DPolygon aPolygon2{{31209.4,70486.3}, {810737,70486.3}, {810737,484501}, {31209.4,484501}}; >+ aPolygon2.setClosed(true); >+ B2DPolyPolygon aPolyPolygon2; >+ aPolyPolygon2.append(aPolygon2); >+ aPolyPolygon2.setClosed(true); >+ >+ SAL_DEBUG(aPolyPolygon1.isClosed() << " " << aPolyPolygon1.getB2DPolygon(0).isClosed() << " " << aPolyPolygon1); >+ SAL_DEBUG(aPolyPolygon2.isClosed() << " " << aPolyPolygon2.getB2DPolygon(0).isClosed() << " " << aPolyPolygon2); >+ SAL_DEBUG(basegfx::utils::solvePolygonOperationAnd(aPolyPolygon1, aPolyPolygon2)); >+ } >+ > // Change the following lines only, if you add, remove or rename > // member functions of the current class, > // because these macros are need by auto register mechanism. > > CPPUNIT_TEST_SUITE(b2dpolypolygon); > CPPUNIT_TEST(testTrapezoidHelper); >+ CPPUNIT_TEST(testPolyIntersection); > CPPUNIT_TEST_SUITE_END(); > }; // class b2dpolypolygon > >diff --git a/vcl/source/gdi/pdfwriter_impl.cxx b/vcl/source/gdi/pdfwriter_impl.cxx >index 31daa05a73bd..07f05ea97fdc 100644 >--- a/vcl/source/gdi/pdfwriter_impl.cxx >+++ b/vcl/source/gdi/pdfwriter_impl.cxx >@@ -9620,13 +9620,13 @@ void PDFWriterImpl::updateGraphicsState(Mode const mode) > SetMapMode( rNewState.m_aMapMode ); > m_aCurrentPDFState.m_aMapMode = rNewState.m_aMapMode; > >- aLine.append("q "); >- if ( rNewState.m_aClipRegion.count() ) >- { >+ SAL_DEBUG(__FUNCTION__ << " " << rNewState.m_aClipRegion.count()); >+ aLine.append( "q " ); >+ if( rNewState.m_aClipRegion.count() ) > m_aPages.back().appendPolyPolygon( rNewState.m_aClipRegion, aLine ); >- aLine.append( "W* n\n" ); >- } >- >+ else >+ aLine.append( "0 0 m h " ); // NULL clip, i.e. nothing visible >+ aLine.append( "W* n\n" ); > rNewState.m_aMapMode = aNewMapMode; > SetMapMode( rNewState.m_aMapMode ); > m_aCurrentPDFState.m_aMapMode = rNewState.m_aMapMode; >@@ -9740,6 +9740,7 @@ void PDFWriterImpl::pop() > setMapMode( aState.m_aMapMode ); > if( ! (aState.m_nFlags & PushFlags::CLIPREGION) ) > { >+ SAL_DEBUG(__FUNCTION__); > // do not use setClipRegion here > // it would convert again assuming the current mapmode > rOld.m_aClipRegion = aState.m_aClipRegion; >@@ -9770,6 +9771,7 @@ void PDFWriterImpl::setMapMode( const MapMode& rMapMode ) > > void PDFWriterImpl::setClipRegion( const basegfx::B2DPolyPolygon& rRegion ) > { >+ SAL_DEBUG(__FUNCTION__ << " " << rRegion.count()); > basegfx::B2DPolyPolygon aRegion = LogicToPixel( rRegion, m_aGraphicsStack.front().m_aMapMode ); > aRegion = PixelToLogic( aRegion, m_aMapMode ); > m_aGraphicsStack.front().m_aClipRegion = aRegion; >@@ -9781,6 +9783,7 @@ void PDFWriterImpl::moveClipRegion( sal_Int32 nX, sal_Int32 nY ) > { > if( m_aGraphicsStack.front().m_bClipRegion && m_aGraphicsStack.front().m_aClipRegion.count() ) > { >+ SAL_DEBUG(__FUNCTION__); > Point aPoint( lcl_convert( m_aGraphicsStack.front().m_aMapMode, > m_aMapMode, > this, >@@ -9808,14 +9811,20 @@ void PDFWriterImpl::intersectClipRegion( const basegfx::B2DPolyPolygon& rRegion > basegfx::B2DPolyPolygon aRegion( LogicToPixel( rRegion, m_aGraphicsStack.front().m_aMapMode ) ); > aRegion = PixelToLogic( aRegion, m_aMapMode ); > m_aGraphicsStack.front().m_nUpdateFlags |= GraphicsStateUpdateFlags::ClipRegion; >+ SAL_DEBUG(__FUNCTION__ << " in " << m_aGraphicsStack.front().m_bClipRegion << " " << rRegion.count() << " " << aRegion.count()); > if( m_aGraphicsStack.front().m_bClipRegion ) > { > basegfx::B2DPolyPolygon aOld( basegfx::utils::prepareForPolygonOperation( m_aGraphicsStack.front().m_aClipRegion ) ); > aRegion = basegfx::utils::prepareForPolygonOperation( aRegion ); >+ SAL_DEBUG(aOld.isClosed() << " " << aOld.getB2DPolygon(0).isClosed() << " " << aOld); >+ SAL_DEBUG(aRegion.isClosed() << " " << aRegion.getB2DPolygon(0).isClosed() << " " << aRegion); > m_aGraphicsStack.front().m_aClipRegion = basegfx::utils::solvePolygonOperationAnd( aOld, aRegion ); >+ SAL_DEBUG(m_aGraphicsStack.front().m_aClipRegion); > } > else > { >+ SAL_DEBUG(rRegion); >+ SAL_DEBUG(aRegion); > m_aGraphicsStack.front().m_aClipRegion = aRegion; > m_aGraphicsStack.front().m_bClipRegion = true; > } >diff --git a/vcl/source/gdi/pdfwriter_impl.hxx b/vcl/source/gdi/pdfwriter_impl.hxx >index 82afd47e850e..ef3e1320ddfd 100644 >--- a/vcl/source/gdi/pdfwriter_impl.hxx >+++ b/vcl/source/gdi/pdfwriter_impl.hxx >@@ -1115,6 +1115,7 @@ public: > > void clearClipRegion() > { >+ SAL_DEBUG(__FUNCTION__ << "\n"); > m_aGraphicsStack.front().m_aClipRegion.clear(); > m_aGraphicsStack.front().m_bClipRegion = false; > m_aGraphicsStack.front().m_nUpdateFlags |= GraphicsStateUpdateFlags::ClipRegion; >-- >2.17.1 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=157377">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Actions:
View
Attachments on
bug 130150
:
157375
|
157376
| 157377