Created attachment 157375 [details] Broken clipping in non-form PDF export shows Markierfeld titles When converting the attached ODT to PDF, for example via command line like: soffice --convert-to pdf --outdir . broken_pdf_clipping.odt or in Writer using PDF export *without* selecting "create PDF form", the resulting PDF document looses the clipping of the existing checkbox titles. Reverting the patch from bug 99680 fixes this problem. The history of the problem is something like: - commit 581806182ac7 ("tdf#99680 modified clipping for PDF export") - commit 86b47f5138c0 ("tdf#44388: handle the NULL clip correctly for pdf output") - commit a334752d091f ("vcl109: #i65128# avoid breaking polygonal clip regions into rectangles in PDF export"). The latest patch changed the semantics of the clip region parameters: - m_bClipRegion = true - m_aClipRegion.count() == 0 from "// NULL clip, i.e. nothing visible" to "no clipping applied". This inherently conflicts with the code of PDFWriterImpl::intersectClipRegion, which "intersects clip regions". When intersecting clip regions / polygons, the result might be an empty intersection, but that doesn't mean, no clipping, but nothing visible. In the attached bug document that's the clip of the check box inside the clip of the table. Obviously the revert results in bug 99680 again. Inspecting that original bug shows rounding problems with the simple test document attached to that bug (attachment 133268 [details]), resulting in an empty clip region for the first stack object from calling basegfx::utils::solvePolygonOperationAnd in PDFWriterImpl::intersectClipRegion. And in the end it's also the origin of bug 113449, which is also about rounding problems in intersectClipRegion, but in an other place in the call stack. Both bugs AFAIK fail in stripDispensablePolygons. This one in B2DRange::isInside, the other one in basegfx::utils::isInside, used to detect the case of polygons inside each other. Since these fail, no intersection is detected and the result is empty, or - with tdf#99680 applied - completely unclipped / visible. Bug 113449 claims "there is no problem with the commit [581806182ac7]", but from my analysis, the patch is wrong, or I misunderstood that analysis. Still I currently also have no good idea where to fix it and do the rounding to fix the comparison. This is some subtle rounding problem in the basegfx based stack. And it's definitely no easy hack from the analysis point of view, even when a real fix might be as simple as a single round or an adapted comparison.
Created attachment 157376 [details] Wrongly clipped check boxes in exported PDF.
Created attachment 157377 [details] Some debug patch including a basegfx test.
> Created attachment 157375 [details] > Broken clipping in non-form PDF export shows Markierfeld titles Note: The text that appears in the PDF export is the label of the checkboxes. To see that in Writer, switch "Form->Design Mode" on, then resize the checkbox to make it larger. LibreOffice 3.4.0 already had that bug.
Taking a look...
Would've been nice to mention reverting which commit of bug 99680 helps here, but seems the one from comment 43 ( 581806182ac7da81115c9675b4e828688178aa04). Also needed to check what 'Q'/'q' op's in PDF - do, see 4.4.3 'Cipping Path Operators' in https://www.adobe.com/content/dam/acom/en/devnet/pdf/pdfs/pdf_reference_archives/PDFReference.pdf Will now check tdf#99680 - seems indeed that that's the one that was fixed wrong...?
During debugging tried to reduce bugdoc further, but crashes when ending design mode, select one of the contriols & using ControlProperties... from context menu. Still reduced, checking source of the clip region...
(In reply to Armin Le Grand from comment #6) > During debugging tried to reduce bugdoc further, but crashes when ending > design mode, select one of the contriols & using ControlProperties... from > context menu. Still reduced, checking source of the clip region... Crash not reproduced in Version: 7.0.0.0.alpha0+ Build ID: 950e1aec0a984ce40a5038331f491272b51d41fa CPU threads: 4; OS: Linux 4.19; UI render: default; VCL: x11; Locale: en-US (en_US.UTF-8); UI-Language: en-US Calc: threaded
Checking PDF creator - 1st clip is PageRect (PDFExport::ImplExportPage). creates PDFWriterImpl::updateGraphicsState indirect by action#17 (of 85) -> as expected - 2nd, 3rd, 4th are MetaISectRegionClipRegionAction, action #22, #46, #50 - 5th, 6th MetaISectRectClipRegionAction, #61, #69 - PDFWriterImpl::updateGraphicsState indirect by action#70 (MetaTextArrayAction) - PDFWriterImpl::updateGraphicsState indirect by action#76 (MetaRectAction) Very strange stuff - we have five different ClipRegio-actions in a 88-action metafile. The (MetaTextArrayAction) trigger indeed has an empty clip-PolyPolyon what together with having a clip region *should* mean to clip everyting -> paint nothing. Seems correct and like a proof that tdf#99680 fix was more a shot in the dark, hoping it works...
Checked creations: 6x ObjectContactOfPageView::DoProcessDisplay/SdrPageView::DrawLayer 3x CheckBox::ImplDraw 4x OutputDevice::ImplDrawText/Control::DrawControlText Last one is dubious: Always the same rect, but right < left - old Rect problem (that this is even possible). Not corrected, goes in that form in the clip action. Strange, but never touched, so might be unimportant. OTOH this *will* influence the orientation of the polyPolygon when used for And/Or clipping stuff - so might be a hint on the problem...
Note for ::Region: There is IsEmpty() aka all gets clipped, and there is IsNull() for nothing gets clipped - should usually be represented by setting no Region at all.
Jmux (see description)was pretty good aleady. Conclusion: - Need to revert tdf#99680 - Fix it differently, including e.g. tdf#113449 -> Numerical stuff - sigh... Deep-diving into tdf#113449 findings right now...
Have now reconstructed the case described in tdf#113449 - some numbers were missing - or better: The dumped deltas in the two regions are shortened by dumping, so not too useful. Alternaitives indeed are: (1) Make isPointOnPolygon trigger earier/unsharp (2) Make isInside decisions for bCompXA/bCompXB (and also bCompYA/bCompYB) less unsharp by *not* using fTools::more which internally uses rtl::math::approxEqual. The involved numbers approxEqual(3075.0236220472434, 3075.0236220472439) -> true trigger - just in the *last* condition of it which is return (d < a * e48 && d < b * e48); so good at e44 precision, but *not* on e48 precision... Have to think about it - changes here may have deep impact...
Tested some alternatives for basegfx::utils::isPointOnLine: - cross product direct & with normed vectors - area calculation -> close to zero -> point on line - edge length diff -> close to zero -> point on line - use hypot()/sqrt, use squared distances (also goes to zero, but in squared param spaces) 2nd thought is to use isPointOnLine additionally in 2nd half of isInside 'somehow' - more as reference for trigger than anything else, thus another name but alow check if would trigger...
Checked bugdoc from tdf#99680 - much better results, but still *some* missing graphics - need to check deeper...
Checking bugdocs/descriptions from tdf#44388...
All cases from tdf#44388 work well. Still rare bad cases (much rarer than before, but happens). Trying to find/construct a reproducible case...
No reliably reproducable case yet - numerical stuff :-( Preparing changes for commit, it's a solution working in most cases...
Current decent but not complete solution at https://gerrit.libreoffice.org/c/core/+/89874, see there. Thinking about it, esp. I had started to replace that stuff with two big changes - (1) not relying on orientation and thus speeding up sorting out significantly and (2) improved and completely overhauled cutsAndTouches stuff - the backbone for clipping PolyPolygons. That part wopuld probably fix this - the basic reason is that And-Op with PolyPolygons is not always reliable for numerical reasons - and improve it. I would guess I would need 2-3 days to complete that and - due to this being relevant and needs care - some days for testing and to ensure stability...
CppunitTest_vcl_pdfexport fails - sure, it checks for no empty clipping regions which is now wrong. There are no empty clipping regions allowed (in principle the output might also be blocked when this is the case, but that would be a bigger change). Have to adapt that as needed, will comment there: PdfExportTest::testTdf99680_2()
... and PdfExportTest::testTdf99680(), too...
Have now nailed a case where context is lost, it's a writer doc with an embedded OLE where the OLE is a draw document. As many now know writer MapMode is Twip, Draw is 100thMM. Indeed numerical inconsistencies trigger, the MapModes are: {meUnit=MapPoint (8) maOrigin=X:0, Y:0 maScaleX={mnNumerator=1 mnDenominator=1000 mbValid=true } ...} and {mpImplMapMode={m_pimpl=0x0000025d2f08f490 {m_value={meUnit=MapTwip (9) maOrigin=X:1506, Y:848 maScaleX=...} ...} } } The involved Clip-PolyPolygons (which are rectangles due to being handed in from PDFWriterImpl::intersectClipRegion which converts to PolyPolygon) are slightly different - both say true to basegfx::utils::isRectangle and are slightly different: last: {maRangeX={mnMinimum=70850.000000000000 mnMaximum=464300.00000000000 } maRangeY={mnMinimum=61050.000000000000 ...} } {maRangeX={mnMinimum=70894.488188976378 mnMaximum=464371.65354330710 } maRangeY={mnMinimum=61102.068028298498 ...} } Pretty similar - but numerical stuff - sigh - makes it fail. Additionally to various MapModes is the fact that the used Metafile is on integer, too - (ARGH! I proposed MANY times to change the PDF exporter to primitives which would provide double precision values...) Or in short: same same but different :-)
At least I improved now the numerical precision of transformations (locally) using transformations instead of OutputDevice::LogicToPixel/PixelToLogic, but that's not enough. We will somehow need to detect that these coordinates are 'the same'. BUT: Need to be careful, we only want to trigger on real cases - sigh...
Had an idea and it works: If both sources for PDFWriterImpl::intersectClipRegion are ranges (using basegfx::utils::isRectangle) the basegfx::utils::solvePolygonOperationAnd is able to solve the AND-operation always based on these ranges. Adapted ::solvePolygonOperationAnd to do exactly that and it works, checked with quite some cases... Also improved precision for transformations, that may make future detections of 'same-but-different' geometry simpler. Checked quite some test cases from the mentioned tasks, looks good. Doing some more last tests...
Looks good - in this state I can give my recommendation to use this
Test look good, but not to forget - this is a workaround. Comment 18 stays true, the refactor woluld be needed e.g. when someone rotates those embedded draws-in-writer or similar - that would fail and could only be solved by a real AND-PolyPolygon solver (!) Just to make that clear...
Armin.Le.Grand (CIB) committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/6dff631f8f4b964b48aadde52a1e1b8b04b9ba53 tdf#130150 Improve clipping in PDF export It will be available in 7.0.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Armin.Le.Grand (CIB) committed a patch related to this issue. It has been pushed to "libreoffice-6-4": https://git.libreoffice.org/core/commit/3bd5de707a361fc23d53dd5ad76c147722dfa2e9 tdf#130150 Improve clipping in PDF export It will be available in 6.4.3. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.