From 32cb4e4fe55e662d2e515e9a6facabffe3684754 Mon Sep 17 00:00:00 2001 From: Bartosz Kosiorek Date: Fri, 29 Apr 2022 13:36:10 +0200 Subject: [PATCH] tdf#89331 EMF/WMF Fix holes in lines created with LINETO If the lines are created with MOVETO, LINETO, LINETO... then Line Join NONE is applied. As a result the charts are looks ugly, with the holes inside it. For example: https://bugs.documentfoundation.org/attachment.cgi?id=179962 and https://bugs.documentfoundation.org/attachment.cgi?id=179837 Additinally commit changed default line join style to miter, as during experimenting with MS Paint and MS Word, it appear that default Join Style is PS_JOIN_MITER and Line Cap is Flat/Butter. The PDF export tests has been updated, as there is less number of PDF object after using joiners. The size of the exported tdf145873.pptx to PDF, was slighltly decreased from 22.8kB to 22.0KB Change-Id: I131cc3c5e90f827d67d2360eb18167eed6315abb Reviewed-on: https://gerrit.libreoffice.org/c/core/+/133624 Tested-by: Jenkins Reviewed-by: Bartosz Kosiorek --- drawinglayer/source/tools/wmfemfhelper.cxx | 11 +- emfio/inc/mtftools.hxx | 79 ++++++++- emfio/qa/cppunit/emf/EmfImportTest.cxx | 19 +++ emfio/qa/cppunit/wmf/data/TestLineTo.wmf | Bin 0 -> 240 bytes emfio/source/reader/emfreader.cxx | 180 ++------------------- emfio/source/reader/wmfreader.cxx | 65 +------- vcl/qa/cppunit/pdfexport/pdfexport.cxx | 12 +- 7 files changed, 120 insertions(+), 246 deletions(-) create mode 100644 emfio/qa/cppunit/wmf/data/TestLineTo.wmf diff --git a/drawinglayer/source/tools/wmfemfhelper.cxx b/drawinglayer/source/tools/wmfemfhelper.cxx index 592e67bab9a5..1f5b8a10fcf2 100644 --- a/drawinglayer/source/tools/wmfemfhelper.cxx +++ b/drawinglayer/source/tools/wmfemfhelper.cxx @@ -1548,7 +1548,6 @@ namespace wmfemfhelper } else { - aLineInfo.SetLineJoin(basegfx::B2DLineJoin::NONE); // It were lines; force to NONE createLinePrimitive(aLinePolygon, aLineInfo, rTargetHolders.Current(), rPropertyHolders.Current()); aLinePolygon.clear(); aLineInfo = pA->GetLineInfo(); @@ -1563,16 +1562,14 @@ namespace wmfemfhelper aLinePolygon.append(aEnd); } - nAction++; if(nAction < nCount) pAction = rMetaFile.GetAction(nAction); + nAction++; + if (nAction < nCount) + pAction = rMetaFile.GetAction(nAction); } nAction--; - - if(aLinePolygon.count()) - { - aLineInfo.SetLineJoin(basegfx::B2DLineJoin::NONE); // It were lines; force to NONE + if (aLinePolygon.count()) createLinePrimitive(aLinePolygon, aLineInfo, rTargetHolders.Current(), rPropertyHolders.Current()); - } } break; diff --git a/emfio/inc/mtftools.hxx b/emfio/inc/mtftools.hxx index 0290c487c574..997f2287f010 100644 --- a/emfio/inc/mtftools.hxx +++ b/emfio/inc/mtftools.hxx @@ -204,8 +204,6 @@ namespace emfio enum PenStyle : sal_uInt32 { PS_COSMETIC = 0x00000000, - PS_ENDCAP_ROUND = 0x00000000, - PS_JOIN_ROUND = 0x00000000, PS_SOLID = 0x00000000, PS_DASH = 0x00000001, PS_DOT = 0x00000002, @@ -216,12 +214,17 @@ namespace emfio PS_USERSTYLE = 0x00000007, PS_ALTERNATE = 0x00000008, PS_STYLE_MASK = 0x0000000F, + + PS_ENDCAP_ROUND = 0x00000000, PS_ENDCAP_SQUARE = 0x00000100, PS_ENDCAP_FLAT = 0x00000200, PS_ENDCAP_STYLE_MASK = 0x00000F00, + + PS_JOIN_ROUND = 0x00000000, PS_JOIN_BEVEL = 0x00001000, PS_JOIN_MITER = 0x00002000, PS_JOIN_STYLE_MASK = 0x0000F000, + PS_GEOMETRIC = 0x00010000 }; @@ -461,11 +464,75 @@ namespace emfio , bTransparent(bTrans) {} - WinMtfLineStyle(const Color& rColor, const LineInfo& rStyle, bool bTrans) + WinMtfLineStyle(const Color& rColor, const sal_uInt32 nStyle, const sal_Int32 nPenWidth) : aLineColor(rColor) - , aLineInfo(rStyle) - , bTransparent(bTrans) - {} + { + // According to documentation: nStyle = PS_COSMETIC = 0x0 - line with a width of one logical unit and a style that is a solid color + // tdf#140271 Based on observed behaviour the line width is not constant with PS_COSMETIC + + // Width 0 means default width for LineInfo (HairLine) with 1 pixel wide + aLineInfo.SetWidth(nPenWidth); + switch (nStyle & PS_STYLE_MASK) + { + case PS_DASHDOTDOT: + aLineInfo.SetStyle(LineStyle::Dash); + aLineInfo.SetDashCount(1); + aLineInfo.SetDotCount(2); + break; + case PS_DASHDOT: + aLineInfo.SetStyle(LineStyle::Dash); + aLineInfo.SetDashCount(1); + aLineInfo.SetDotCount(1); + break; + case PS_DOT: + aLineInfo.SetStyle(LineStyle::Dash); + aLineInfo.SetDashCount(0); + aLineInfo.SetDotCount(1); + break; + case PS_DASH: + aLineInfo.SetStyle(LineStyle::Dash); + aLineInfo.SetDashCount(1); + aLineInfo.SetDotCount(0); + break; + case PS_NULL: + aLineInfo.SetStyle(LineStyle::NONE); + break; + case PS_INSIDEFRAME: // TODO Implement PS_INSIDEFRAME + case PS_SOLID: + default: + aLineInfo.SetStyle(LineStyle::Solid); + } + if (nPenWidth) + switch (nStyle & PS_ENDCAP_STYLE_MASK) + { + case PS_ENDCAP_ROUND: + aLineInfo.SetLineCap(css::drawing::LineCap_ROUND); + break; + case PS_ENDCAP_SQUARE: + aLineInfo.SetLineCap(css::drawing::LineCap_SQUARE); + break; + case PS_ENDCAP_FLAT: + default: + aLineInfo.SetLineCap(css::drawing::LineCap_BUTT); + } + else + aLineInfo.SetLineCap(css::drawing::LineCap_BUTT); + switch (nStyle & PS_JOIN_STYLE_MASK) + { + case PS_JOIN_ROUND: + aLineInfo.SetLineJoin(basegfx::B2DLineJoin::Round); + break; + case PS_JOIN_BEVEL: + aLineInfo.SetLineJoin(basegfx::B2DLineJoin::Bevel); + break; + // Undocumented but based on experiments with MS Paint and MS Word, + // the default Join Style is PS_JOIN_MITER + case PS_JOIN_MITER: + default: + aLineInfo.SetLineJoin(basegfx::B2DLineJoin::Miter); + } + bTransparent = aLineInfo.GetStyle() == LineStyle::NONE; + } bool operator==(const WinMtfLineStyle& rStyle) const { diff --git a/emfio/qa/cppunit/emf/EmfImportTest.cxx b/emfio/qa/cppunit/emf/EmfImportTest.cxx index e8db00e21fcc..5891799dd607 100644 --- a/emfio/qa/cppunit/emf/EmfImportTest.cxx +++ b/emfio/qa/cppunit/emf/EmfImportTest.cxx @@ -74,6 +74,7 @@ class Test : public test::BootstrapFixture, public XmlTestTools, public unotest: void TestRestoreDCWMF(); void TestRoundrectWMF(); void TestStretchDIBWMF(); + void TestMoveToLineToWMF(); void TestPolylinetoCloseStroke(); void TestPolyLineWidth(); @@ -120,6 +121,7 @@ public: CPPUNIT_TEST(TestRestoreDCWMF); CPPUNIT_TEST(TestRoundrectWMF); CPPUNIT_TEST(TestStretchDIBWMF); + CPPUNIT_TEST(TestMoveToLineToWMF); CPPUNIT_TEST(TestPolylinetoCloseStroke); CPPUNIT_TEST(TestPolyLineWidth); CPPUNIT_TEST(TestRestoreDC); @@ -1206,6 +1208,23 @@ void Test::TestStretchDIBWMF() "720000,721c1c,723838,725555,727171,72728d,55728d,39728d,1d728d,00728d"); } +void Test::TestMoveToLineToWMF() +{ + // tdf#89331 WMF records: MOTETO, LINETO, CREATEPENINDIRECT. + Primitive2DSequence aSequence = parseEmf(u"/emfio/qa/cppunit/wmf/data/TestLineTo.wmf"); + CPPUNIT_ASSERT_EQUAL(1, static_cast(aSequence.getLength())); + drawinglayer::Primitive2dXmlDump dumper; + xmlDocUniquePtr pDocument = dumper.dumpAndParse(Primitive2DContainer(aSequence)); + CPPUNIT_ASSERT(pDocument); + + assertXPathContent(pDocument, aXPathPrefix + "polygonstroke/polygon", + "5856,3586 7167,621 8625,3586"); + assertXPath(pDocument, aXPathPrefix + "polygonstroke/line", "color", "#800000"); + assertXPath(pDocument, aXPathPrefix + "polygonstroke/line", "width", "310"); + assertXPath(pDocument, aXPathPrefix + "polygonstroke/line", "linejoin", "Bevel"); + assertXPath(pDocument, aXPathPrefix + "polygonstroke/line", "linecap", "ROUND"); +} + void Test::TestPolyLineWidth() { // EMF import with records: CREATEPEN, ROUNDRECT. diff --git a/emfio/qa/cppunit/wmf/data/TestLineTo.wmf b/emfio/qa/cppunit/wmf/data/TestLineTo.wmf new file mode 100644 index 0000000000000000000000000000000000000000..14ba0fc6eb5a92d92d2618de19b50011a6c3d4ca GIT binary patch literal 240 zcmcb9_Sh^2FxV!aEWdyi#7qfdWZ-0AV9o`y`9UlOaR!hCHxrNmvv`=2<+sVR0C`M| zj0_-}g^`zm14#b@s{t~A03@!<$iTo3r2jAh8DIbrWrVBVfFjNW*9B7dfsqNOP5`8? r0f-^`nZWK4Vfx7ag9qd`VWtGe`}`2rM|LS;h$@I`kSWYSH6Q>0?FJRy literal 0 HcmV?d00001 diff --git a/emfio/source/reader/emfreader.cxx b/emfio/source/reader/emfreader.cxx index 54be65c3bcbf..389414c79b9f 100644 --- a/emfio/source/reader/emfreader.cxx +++ b/emfio/source/reader/emfreader.cxx @@ -1144,187 +1144,41 @@ namespace emfio } break; - case EMR_CREATEPEN : + case EMR_CREATEPEN: { - mpInputStream->ReadUInt32( nIndex ); - if ( ( nIndex & ENHMETA_STOCK_OBJECT ) == 0 ) + mpInputStream->ReadUInt32(nIndex); + if ((nIndex & ENHMETA_STOCK_OBJECT) == 0) { - LineInfo aLineInfo; sal_uInt32 nStyle(0); sal_Int32 nPenWidth(0), nIgnored; - - mpInputStream->ReadUInt32( nStyle ).ReadInt32( nPenWidth ).ReadInt32( nIgnored ); - - SAL_INFO("emfio", "\t\tIndex: " << nIndex << " Style: 0x" << std::hex << nStyle << std::dec << " PenWidth: " << nPenWidth); - // According to documentation: nStyle = PS_COSMETIC = 0x0 - line with a width of one logical unit and a style that is a solid color - // tdf#140271 Based on observed behaviour the line width is not constant with PS_COSMETIC - - // Width 0 means default width for LineInfo (HairLine) with 1 pixel wide - aLineInfo.SetWidth( nPenWidth ); - - bool bTransparent = false; - switch( nStyle & PS_STYLE_MASK ) - { - case PS_DASHDOTDOT : - aLineInfo.SetStyle( LineStyle::Dash ); - aLineInfo.SetDashCount( 1 ); - aLineInfo.SetDotCount( 2 ); - break; - case PS_DASHDOT : - aLineInfo.SetStyle( LineStyle::Dash ); - aLineInfo.SetDashCount( 1 ); - aLineInfo.SetDotCount( 1 ); - break; - case PS_DOT : - aLineInfo.SetStyle( LineStyle::Dash ); - aLineInfo.SetDashCount( 0 ); - aLineInfo.SetDotCount( 1 ); - break; - case PS_DASH : - aLineInfo.SetStyle( LineStyle::Dash ); - aLineInfo.SetDashCount( 1 ); - aLineInfo.SetDotCount( 0 ); - break; - case PS_NULL : - bTransparent = true; - aLineInfo.SetStyle( LineStyle::NONE ); - break; - case PS_INSIDEFRAME : - case PS_SOLID : - default : - aLineInfo.SetStyle( LineStyle::Solid ); - } - switch( nStyle & PS_ENDCAP_STYLE_MASK ) - { - case PS_ENDCAP_ROUND : - if ( nPenWidth ) - { - aLineInfo.SetLineCap( css::drawing::LineCap_ROUND ); - break; - } - [[fallthrough]]; - case PS_ENDCAP_SQUARE : - if ( nPenWidth ) - { - aLineInfo.SetLineCap( css::drawing::LineCap_SQUARE ); - break; - } - [[fallthrough]]; - case PS_ENDCAP_FLAT : - default : - aLineInfo.SetLineCap( css::drawing::LineCap_BUTT ); - } - switch( nStyle & PS_JOIN_STYLE_MASK ) - { - case PS_JOIN_ROUND : - aLineInfo.SetLineJoin ( basegfx::B2DLineJoin::Round ); - break; - case PS_JOIN_MITER : - aLineInfo.SetLineJoin ( basegfx::B2DLineJoin::Miter ); - break; - case PS_JOIN_BEVEL : - aLineInfo.SetLineJoin ( basegfx::B2DLineJoin::Bevel ); - break; - default : - aLineInfo.SetLineJoin ( basegfx::B2DLineJoin::NONE ); - } - CreateObjectIndexed(nIndex, std::make_unique( ReadColor(), aLineInfo, bTransparent )); + mpInputStream->ReadUInt32(nStyle).ReadInt32(nPenWidth).ReadInt32(nIgnored); + SAL_INFO("emfio", "\t\tIndex: " << nIndex << " Style: 0x" << std::hex + << nStyle << std::dec + << " PenWidth: " << nPenWidth); + CreateObjectIndexed(nIndex, std::make_unique(ReadColor(), nStyle, nPenWidth)); } } break; - case EMR_EXTCREATEPEN : + case EMR_EXTCREATEPEN: { - mpInputStream->ReadUInt32( nIndex ); - if ( ( nIndex & ENHMETA_STOCK_OBJECT ) == 0 ) + mpInputStream->ReadUInt32(nIndex); + if ((nIndex & ENHMETA_STOCK_OBJECT) == 0) { - sal_uInt32 offBmi, cbBmi, offBits, cbBits, nStyle, nWidth, nBrushStyle, elpNumEntries; - sal_Int32 elpHatch; - mpInputStream->ReadUInt32( offBmi ).ReadUInt32( cbBmi ).ReadUInt32( offBits ).ReadUInt32( cbBits ). ReadUInt32( nStyle ).ReadUInt32( nWidth ).ReadUInt32( nBrushStyle ); + sal_uInt32 offBmi, cbBmi, offBits, cbBits, nStyle, nWidth, nBrushStyle, elpNumEntries; + sal_Int32 elpHatch; + mpInputStream->ReadUInt32(offBmi).ReadUInt32(cbBmi).ReadUInt32(offBits).ReadUInt32(cbBits); + mpInputStream->ReadUInt32(nStyle).ReadUInt32(nWidth).ReadUInt32(nBrushStyle); SAL_INFO("emfio", "\t\tStyle: 0x" << std::hex << nStyle << std::dec); SAL_INFO("emfio", "\t\tWidth: " << nWidth); Color aColorRef = ReadColor(); - mpInputStream->ReadInt32( elpHatch ).ReadUInt32( elpNumEntries ); + mpInputStream->ReadInt32(elpHatch).ReadUInt32(elpNumEntries); if (!mpInputStream->good()) bStatus = false; else - { - LineInfo aLineInfo; - if ( nWidth ) - aLineInfo.SetWidth( nWidth ); - - bool bTransparent = false; - - switch( nStyle & PS_STYLE_MASK ) - { - case PS_DASHDOTDOT : - aLineInfo.SetStyle( LineStyle::Dash ); - aLineInfo.SetDashCount( 1 ); - aLineInfo.SetDotCount( 2 ); - break; - case PS_DASHDOT : - aLineInfo.SetStyle( LineStyle::Dash ); - aLineInfo.SetDashCount( 1 ); - aLineInfo.SetDotCount( 1 ); - break; - case PS_DOT : - aLineInfo.SetStyle( LineStyle::Dash ); - aLineInfo.SetDashCount( 0 ); - aLineInfo.SetDotCount( 1 ); - break; - case PS_DASH : - aLineInfo.SetStyle( LineStyle::Dash ); - aLineInfo.SetDashCount( 1 ); - aLineInfo.SetDotCount( 0 ); - break; - case PS_NULL : - bTransparent = true; - aLineInfo.SetStyle( LineStyle::NONE ); - break; - - case PS_INSIDEFRAME : - case PS_SOLID : - default : - aLineInfo.SetStyle( LineStyle::Solid ); - } - switch( nStyle & PS_ENDCAP_STYLE_MASK ) - { - case PS_ENDCAP_ROUND : - if ( aLineInfo.GetWidth() ) - { - aLineInfo.SetLineCap( css::drawing::LineCap_ROUND ); - break; - } - [[fallthrough]]; - case PS_ENDCAP_SQUARE : - if ( aLineInfo.GetWidth() ) - { - aLineInfo.SetLineCap( css::drawing::LineCap_SQUARE ); - break; - } - [[fallthrough]]; - case PS_ENDCAP_FLAT : - default : - aLineInfo.SetLineCap( css::drawing::LineCap_BUTT ); - } - switch( nStyle & PS_JOIN_STYLE_MASK ) - { - case PS_JOIN_ROUND : - aLineInfo.SetLineJoin ( basegfx::B2DLineJoin::Round ); - break; - case PS_JOIN_MITER : - aLineInfo.SetLineJoin ( basegfx::B2DLineJoin::Miter ); - break; - case PS_JOIN_BEVEL : - aLineInfo.SetLineJoin ( basegfx::B2DLineJoin::Bevel ); - break; - default : - aLineInfo.SetLineJoin ( basegfx::B2DLineJoin::NONE ); - } - CreateObjectIndexed(nIndex, std::make_unique( aColorRef, aLineInfo, bTransparent )); - } + CreateObjectIndexed(nIndex, std::make_unique(aColorRef, nStyle, nWidth)); } } break; diff --git a/emfio/source/reader/wmfreader.cxx b/emfio/source/reader/wmfreader.cxx index 44615556793f..5f6eeed66757 100644 --- a/emfio/source/reader/wmfreader.cxx +++ b/emfio/source/reader/wmfreader.cxx @@ -1025,70 +1025,7 @@ namespace emfio mpInputStream->ReadUInt16(nStyle); mpInputStream->ReadUInt16(nWidth); mpInputStream->ReadUInt16(nHeight); - - if (nWidth > 0) - aLineInfo.SetWidth(nWidth); - - bool bTransparent = false; - - switch( nStyle & 0xFF ) - { - case PS_DASHDOTDOT : - aLineInfo.SetStyle( LineStyle::Dash ); - aLineInfo.SetDashCount( 1 ); - aLineInfo.SetDotCount( 2 ); - break; - case PS_DASHDOT : - aLineInfo.SetStyle( LineStyle::Dash ); - aLineInfo.SetDashCount( 1 ); - aLineInfo.SetDotCount( 1 ); - break; - case PS_DOT : - aLineInfo.SetStyle( LineStyle::Dash ); - aLineInfo.SetDashCount( 0 ); - aLineInfo.SetDotCount( 1 ); - break; - case PS_DASH : - aLineInfo.SetStyle( LineStyle::Dash ); - aLineInfo.SetDashCount( 1 ); - aLineInfo.SetDotCount( 0 ); - break; - case PS_NULL : - bTransparent = true; - aLineInfo.SetStyle( LineStyle::NONE ); - break; - default : - case PS_INSIDEFRAME : - case PS_SOLID : - aLineInfo.SetStyle( LineStyle::Solid ); - } - switch( nStyle & 0xF00 ) - { - case PS_ENDCAP_ROUND : - aLineInfo.SetLineCap( css::drawing::LineCap_ROUND ); - break; - case PS_ENDCAP_SQUARE : - aLineInfo.SetLineCap( css::drawing::LineCap_SQUARE ); - break; - case PS_ENDCAP_FLAT : - default : - aLineInfo.SetLineCap( css::drawing::LineCap_BUTT ); - } - switch( nStyle & 0xF000 ) - { - case PS_JOIN_ROUND : - aLineInfo.SetLineJoin ( basegfx::B2DLineJoin::Round ); - break; - case PS_JOIN_MITER : - aLineInfo.SetLineJoin ( basegfx::B2DLineJoin::Miter ); - break; - case PS_JOIN_BEVEL : - aLineInfo.SetLineJoin ( basegfx::B2DLineJoin::Bevel ); - break; - default : - aLineInfo.SetLineJoin ( basegfx::B2DLineJoin::NONE ); - } - CreateObject(std::make_unique( ReadColor(), aLineInfo, bTransparent )); + CreateObject(std::make_unique(ReadColor(), nStyle, nWidth)); } break; diff --git a/vcl/qa/cppunit/pdfexport/pdfexport.cxx b/vcl/qa/cppunit/pdfexport/pdfexport.cxx index f8358c6d85b0..9da9e129a253 100644 --- a/vcl/qa/cppunit/pdfexport/pdfexport.cxx +++ b/vcl/qa/cppunit/pdfexport/pdfexport.cxx @@ -2955,22 +2955,22 @@ CPPUNIT_TEST_FIXTURE(PdfExportTest, testTdf145873) int nPageObjectCount = pPdfPage->getObjectCount(); // tdf#145873: Without the fix #1 in place, this test would have failed with - // - Expected: 318 + // - Expected: 66 // - Actual : 3 - CPPUNIT_ASSERT_EQUAL(318, nPageObjectCount); + CPPUNIT_ASSERT_EQUAL(66, nPageObjectCount); auto pObject = pPdfPage->getObject(4); CPPUNIT_ASSERT_MESSAGE("no object", pObject != nullptr); // tdf#145873: Without the fix #2 in place, this test would have failed with - // - Expected: 3.23 + // - Expected: 13.23 // - Actual : 3.57... // - Delta : 0.1 - CPPUNIT_ASSERT_DOUBLES_EQUAL(3.23, pObject->getBounds().getWidth(), 0.1); - // - Expected: 3.49 + CPPUNIT_ASSERT_DOUBLES_EQUAL(13.23, pObject->getBounds().getWidth(), 0.1); + // - Expected: 13.49 // - Actual : 3.74... // - Delta : 0.1 - CPPUNIT_ASSERT_DOUBLES_EQUAL(3.49, pObject->getBounds().getHeight(), 0.1); + CPPUNIT_ASSERT_DOUBLES_EQUAL(13.49, pObject->getBounds().getHeight(), 0.1); } CPPUNIT_TEST_FIXTURE(PdfExportTest, testPdfImageHyperlink)