From d25906087918c085239aac30fd72cb65aa7b9eb4 Mon Sep 17 00:00:00 2001 From: Hossein Date: Thu, 2 Sep 2021 18:29:08 +0200 Subject: tdf#88163 Fix WMF font size The problems in tdf#88163 can be categorized into two parts: 1. A regression which is caused by the commit a9020e461803964a206d5551884b70717eed165c. The font size for text is wrongly calculated for wmf files WITHOUT the placeable header. 2. A long-lasting bug that has existed from LO 3.5 (tested with various LibreOffice versions from 3.5 to master). The font size for text is wrongly calculated for wmf files WITH the placeable header. This patch solves the first part. In this patch the way wmf is scaled is changed using mnUnitsPerInch as a variable that controls the scale. The previous method was dividing the left/right/top/bottom fields of aPlaceableBound which caused the bad scaling of text. The second problem still remains, and possibly can be solved by usign the old scaling method that was previously used here (dividing pos values of aPlaceableBound), but the scaling factor should be calculated, which varies in different wmf files. More study should be done to solve this part. Values used for the example "visio_import_source.wmf" used in the test WmfTest::testNonPlaceableWmf() are slightly changed, but the rendering should not noticeably change, except the fix for the text font size. A new test WmfTest::testTdf88163NonPlaceableWmf() is added which checks for both font height and text boxes' positions. Font height can depend on the exact font that is used for Roman in the platform, so it vary between Linux, Windows and macOS. Thus, a range is used for ensuring that the font height is correct. By visual inspection, it is confirmed that this fix does not affect fdo#74336 and the fix for it still works for attachment 93188. Change-Id: I55bf38ba4345a0aa48c3e522976a2a5c32f7ec8c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121272 Tested-by: Jenkins Reviewed-by: Miklos Vajna --- emfio/qa/cppunit/emf/EmfImportTest.cxx | 10 ++-- .../qa/cppunit/wmf/data/tdf88163-non-placeable.wmf | Bin 0 -> 1268 bytes emfio/qa/cppunit/wmf/wmfimporttest.cxx | 63 +++++++++++++++++---- emfio/source/reader/wmfreader.cxx | 7 +-- 4 files changed, 61 insertions(+), 19 deletions(-) create mode 100644 emfio/qa/cppunit/wmf/data/tdf88163-non-placeable.wmf diff --git a/emfio/qa/cppunit/emf/EmfImportTest.cxx b/emfio/qa/cppunit/emf/EmfImportTest.cxx index 1d5d68cf28f8..7fc03e2262a2 100644 --- a/emfio/qa/cppunit/emf/EmfImportTest.cxx +++ b/emfio/qa/cppunit/emf/EmfImportTest.cxx @@ -921,17 +921,19 @@ void Test::TestExtTextOutOpaqueAndClipWMF() // On some operating systems (Linux on LO Jenkins CI), the `/mask/` string is not added to XPath // As a result tests are failing. On my Ubuntu 20.04 the `/mask/` string was added // I would leave this test case for macOS to make sure there is no regression at least on one platform. + + // These values come from the fix for tdf#88163 assertXPath(pDocument, aXPathPrefix + "mask/polypolygoncolor", 5); assertXPath(pDocument, aXPathPrefix + "mask/polypolygoncolor[1]/polypolygon", "path", - "m7219 1825h319v3608h-319z"); + "m7257 1836h320v3628h-320z"); assertXPath(pDocument, aXPathPrefix + "mask/polypolygoncolor[1]", "color", "#ff0000"); assertXPath(pDocument, aXPathPrefix + "mask/polypolygoncolor[2]/polypolygon", "path", - "m7219 5942h319v318h-319z"); + "m7257 5976h320v321h-320z"); assertXPath(pDocument, aXPathPrefix + "mask/polypolygoncolor[2]", "color", "#00ff00"); assertXPath(pDocument, aXPathPrefix + "mask/polypolygoncolor[3]/polypolygon", "path", - "m10149 5942h319v318h-319z"); + "m10203 5976h320v321h-320z"); assertXPath(pDocument, aXPathPrefix + "mask/polypolygoncolor[3]", "color", "#8080ff"); assertXPath(pDocument, aXPathPrefix + "mask/group", 5); @@ -948,7 +950,7 @@ void Test::TestExtTextOutOpaqueAndClipWMF() assertXPath(pDocument, aXPathPrefix + "mask/group[3]/mask/group/polypolygoncolor", "color", "#ff8000"); assertXPath(pDocument, aXPathPrefix + "mask/group[3]/mask/group/polypolygoncolor/polypolygon", - "path", "m1062 1061h1270v473h-1270z"); + "path", "m1067 1067h1270v473h-1270z"); assertXPath(pDocument, aXPathPrefix + "mask/group[3]/mask/group/textsimpleportion", "text", "OOOO"); assertXPath(pDocument, aXPathPrefix + "mask/group[3]/mask/group/textsimpleportion", "fontcolor", diff --git a/emfio/qa/cppunit/wmf/data/tdf88163-non-placeable.wmf b/emfio/qa/cppunit/wmf/data/tdf88163-non-placeable.wmf new file mode 100644 index 000000000000..b39514bd1b5c Binary files /dev/null and b/emfio/qa/cppunit/wmf/data/tdf88163-non-placeable.wmf differ diff --git a/emfio/qa/cppunit/wmf/wmfimporttest.cxx b/emfio/qa/cppunit/wmf/wmfimporttest.cxx index 62a669fe3676..00da6f14ad84 100644 --- a/emfio/qa/cppunit/wmf/wmfimporttest.cxx +++ b/emfio/qa/cppunit/wmf/wmfimporttest.cxx @@ -46,6 +46,7 @@ public: } void testNonPlaceableWmf(); + void testTdf88163NonPlaceableWmf(); void testSine(); void testEmfProblem(); void testEmfLineStyles(); @@ -57,6 +58,7 @@ public: CPPUNIT_TEST_SUITE(WmfTest); CPPUNIT_TEST(testNonPlaceableWmf); + CPPUNIT_TEST(testTdf88163NonPlaceableWmf); CPPUNIT_TEST(testSine); CPPUNIT_TEST(testEmfProblem); CPPUNIT_TEST(testEmfLineStyles); @@ -77,24 +79,65 @@ void WmfTest::testNonPlaceableWmf() MetafileXmlDump dumper; dumper.filterAllActionTypes(); dumper.filterActionType(MetaActionType::POLYLINE, false); + xmlDocUniquePtr pDoc = dumpAndParse(dumper, aGDIMetaFile); CPPUNIT_ASSERT(pDoc); - assertXPath(pDoc, "/metafile/polyline[1]/point[1]", "x", "16798"); - assertXPath(pDoc, "/metafile/polyline[1]/point[1]", "y", "1003"); + // These values come from changes done in tdf#88163 + assertXPath(pDoc, "/metafile/polyline[1]/point[1]", "x", "16813"); + assertXPath(pDoc, "/metafile/polyline[1]/point[1]", "y", "1004"); + + assertXPath(pDoc, "/metafile/polyline[1]/point[2]", "x", "16813"); + assertXPath(pDoc, "/metafile/polyline[1]/point[2]", "y", "7514"); + + assertXPath(pDoc, "/metafile/polyline[1]/point[3]", "x", "26112"); + assertXPath(pDoc, "/metafile/polyline[1]/point[3]", "y", "7514"); + + assertXPath(pDoc, "/metafile/polyline[1]/point[4]", "x", "26112"); + assertXPath(pDoc, "/metafile/polyline[1]/point[4]", "y", "1004"); + + assertXPath(pDoc, "/metafile/polyline[1]/point[5]", "x", "16813"); + assertXPath(pDoc, "/metafile/polyline[1]/point[5]", "y", "1004"); +} - assertXPath(pDoc, "/metafile/polyline[1]/point[2]", "x", "16798"); - assertXPath(pDoc, "/metafile/polyline[1]/point[2]", "y", "7507"); +void WmfTest::testTdf88163NonPlaceableWmf() +{ + OUString fileName(u"tdf88163-non-placeable.wmf"); + SvFileStream aFileStream(getFullUrl(fileName), StreamMode::READ); + GDIMetaFile aGDIMetaFile; + ReadWindowMetafile(aFileStream, aGDIMetaFile); - assertXPath(pDoc, "/metafile/polyline[1]/point[3]", "x", "26090"); - assertXPath(pDoc, "/metafile/polyline[1]/point[3]", "y", "7507"); + MetafileXmlDump dumper; + xmlDocUniquePtr pDoc = dumpAndParse(dumper, aGDIMetaFile); - assertXPath(pDoc, "/metafile/polyline[1]/point[4]", "x", "26090"); - assertXPath(pDoc, "/metafile/polyline[1]/point[4]", "y", "1003"); + CPPUNIT_ASSERT(pDoc); - assertXPath(pDoc, "/metafile/polyline[1]/point[5]", "x", "16798"); - assertXPath(pDoc, "/metafile/polyline[1]/point[5]", "y", "1003"); + // These values come from the fix for tdf#88163 + + // Font 'Roman' and its height can vary according to the platform + // Fails without the fix + // Linux: With fix: 3136, without fix: ~ 8000 + // Mac: With fix: 3230, without fix: ~ 8000 + // Windows: With fix: 3303, without fix: ~ 8000 + auto x = getXPath(pDoc, "/metafile/push[2]/font[1]", "height"); + CPPUNIT_ASSERT_MESSAGE(fileName.toUtf8().getStr(), x.toInt32() > 3000); + CPPUNIT_ASSERT_MESSAGE(fileName.toUtf8().getStr(), x.toInt32() < 3500); + + // Fails without the fix: Expected: 7359, Actual: 7336 + assertXPath(pDoc, "/metafile/push[2]/textarray[1]", "x", "7359"); + // Fails without the fix: Expected: 4118, Actual: 4104 + assertXPath(pDoc, "/metafile/push[2]/textarray[1]", "y", "4118"); + + // Fails without the fix: Expected: 5989, Actual: 5971 + assertXPath(pDoc, "/metafile/push[2]/textarray[2]", "x", "5989"); + // Fails without the fix: Expected: 16264, Actual: 16208 + assertXPath(pDoc, "/metafile/push[2]/textarray[2]", "y", "16264"); + + // Fails without the fix: Expected: 20769, Actual: 20705 + assertXPath(pDoc, "/metafile/push[2]/textarray[3]", "x", "20769"); + // Fails without the fix: Expected: 4077, Actual: 4062 + assertXPath(pDoc, "/metafile/push[2]/textarray[3]", "y", "4077"); } void WmfTest::testSine() diff --git a/emfio/source/reader/wmfreader.cxx b/emfio/source/reader/wmfreader.cxx index 82c6210acbbf..62f03fc8df25 100644 --- a/emfio/source/reader/wmfreader.cxx +++ b/emfio/source/reader/wmfreader.cxx @@ -1470,11 +1470,8 @@ namespace emfio const double fMaxWidth = static_cast(aMaxWidth); double fRatio = aPlaceableBound.GetWidth() / fMaxWidth; - aPlaceableBound = tools::Rectangle( - aPlaceableBound.Left() / fRatio, - aPlaceableBound.Top() / fRatio, - aPlaceableBound.Right() / fRatio, - aPlaceableBound.Bottom() / fRatio); + // changing mnUnitsPerInch as a tool to scale wmf + mnUnitsPerInch *= fRatio; SAL_INFO("emfio", "Placeable bounds " " left: " << aPlaceableBound.Left() << " top: " << aPlaceableBound.Top() -- cgit v1.2.3