Bug 31814 - EMF regression: failure of displaying ChemDraw objects (line width to fix EMF+, reading EmfPlusBoundaryPointData, EmfPlusRecordTypeComment records, EMF+ DrawString and DrawImage, EMF/EMF+ dual mode)
Summary: EMF regression: failure of displaying ChemDraw objects (line width to fix EMF...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
3.3.0 Beta3
Hardware: Other All
: medium normal
Assignee: Bartosz
QA Contact:
URL:
Whiteboard: target:5.4.0 target:6.0.0
Keywords: preBibisect, regression
: 57032 61083 (view as bug list)
Depends on:
Blocks: EMF-WMF
  Show dependency treegraph
 
Reported: 2010-11-21 06:16 UTC by Andras Timar
Modified: 2017-11-03 12:04 UTC (History)
19 users (show)

See Also:
Crash report or crash signature:


Attachments
File with example OLE objects that are not shown correctly under LibO/Fedora15 64bit (80.50 KB, application/msword)
2011-09-27 15:24 UTC, christian_l
Details
emf extracted from the doc (15.62 KB, image/x-emf)
2013-02-21 02:40 UTC, Valek Filippov
Details
2nd emf extracted from the doc (11.52 KB, image/x-emf)
2013-02-21 02:40 UTC, Valek Filippov
Details
Bug 31814 - LO 4.2 vs Word 2010 (82.88 KB, image/jpeg)
2013-07-24 13:43 UTC, bfoman (inactive)
Details
master screen shot as of today (110.76 KB, image/png)
2015-11-06 13:29 UTC, Caolán McNamara
Details
master screen shot with LO52+ in Windows (32.21 KB, image/png)
2016-04-08 06:59 UTC, Timur
Details
Another example of ole object (56.85 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2016-04-08 09:39 UTC, gvlatyshev
Details
Extracted emf+ image which illustrate wrong rendering of some lines (too thin) (88.99 KB, image/emf)
2017-05-07 00:32 UTC, Bartosz
Details
shaded box (21.98 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2017-05-09 10:02 UTC, gvlatyshev
Details
gradient-filled boxes (39.23 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2017-05-09 10:13 UTC, gvlatyshev
Details
EMF file from Accelrys Draw (11.52 KB, image/x-emf)
2017-09-20 16:58 UTC, Chris Sherlock
Details
cppcanvas logging (83.39 KB, text/plain)
2017-09-20 17:08 UTC, Chris Sherlock
Details
Another EMF+ file with dual mode (19.58 KB, image/emf)
2017-10-02 15:24 UTC, Bartosz
Details
Document view with (first) dual mode patch applied (57.19 KB, application/pdf)
2017-10-04 12:43 UTC, Patrick Jaap
Details
How LO 6.0 master is opening EMF+ attachment 75221 (9.35 KB, image/png)
2017-10-09 14:38 UTC, Bartosz
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andras Timar 2010-11-21 06:16:07 UTC
There are many ChemDraw objects (molecules) in the referenced presentation. These objects are not displayed in LibreOffice 3.3 beta 3. This is a regression, because:
Go-OO 3.2.1 - OK
OOo 3.3RC - OK
LibO 3.3b3 - BUG

http://ftp.fsf.hu/LibreOffice/Heteroarom%e1s%20vegy%fcletek%20k%e9mi%e1ja-V.ppt
Comment 1 Alex Thurgood 2010-11-21 20:22:19 UTC Comment hidden (obsolete)
Comment 2 Don't use this account, use tml@iki.fi 2010-11-25 05:23:27 UTC Comment hidden (obsolete)
Comment 3 Thorsten Behrens (CIB) 2010-11-25 07:04:07 UTC Comment hidden (obsolete)
Comment 4 Thorsten Behrens (CIB) 2010-11-25 07:04:28 UTC Comment hidden (obsolete)
Comment 5 Andras Timar 2011-06-02 04:00:45 UTC Comment hidden (obsolete)
Comment 6 gvlatyshev 2011-06-18 03:00:06 UTC Comment hidden (obsolete)
Comment 7 Phil Hord 2011-07-14 16:37:11 UTC Comment hidden (obsolete)
Comment 8 christian_l 2011-09-27 15:24:57 UTC
Created attachment 51694 [details]
File with example OLE objects that are not shown correctly under LibO/Fedora15 64bit 

Is anybody working on this? This bug is really bothering me, as it does not only affect ChemDraw objects, but apparently /any/ chemical structure objects, both in document and presentation files, that people send me. Not sure if it affects all OLE objects, but I guess there's a chance for that, too. For me the problem exists since Fedora switched from OpenOffice to LibreOffice (Fedora 14 -> 15, 64bit version). 

I also tried with a recent nightly build, and the problem persists. 

I have attached a test file that has objects created with ChemDraw, Accelrys Draw (= formerly ISISDraw/SymyxDraw), and MarvinDraw, none of which are fully visible for me in LibO. I hope this helps for fixing the problem!

Christian
Comment 9 christian_l 2011-10-22 22:02:04 UTC Comment hidden (no-value)
Comment 10 Thorsten Behrens (CIB) 2011-10-24 06:13:35 UTC Comment hidden (obsolete)
Comment 11 Radek Doulik 2012-04-18 06:54:19 UTC Comment hidden (obsolete)
Comment 12 Radek Doulik 2012-04-18 10:11:48 UTC Comment hidden (obsolete)
Comment 13 Lubos Lunak 2012-07-16 15:46:27 UTC Comment hidden (obsolete)
Comment 14 Radek Doulik 2012-07-17 10:35:15 UTC Comment hidden (obsolete)
Comment 15 Radek Doulik 2012-07-17 14:29:13 UTC Comment hidden (obsolete)
Comment 16 Lennard Wasserthal 2013-01-28 17:05:39 UTC
*** Bug 57032 has been marked as a duplicate of this bug. ***
Comment 17 Lennard Wasserthal 2013-01-28 17:15:55 UTC
Fails on Windows Version 3.6.3.2 Too. (32 Bit, 2x Intel Core2 Duo)
Displaying ChemDraw works when setting EMF_PLUS_DISABLE to 1.
strangely, the error occurred obviously with delay.
Comment 18 Valek Filippov 2013-02-21 02:40:04 UTC
Created attachment 75220 [details]
emf extracted from the doc
Comment 19 Valek Filippov 2013-02-21 02:40:39 UTC
Created attachment 75221 [details]
2nd emf extracted from the doc
Comment 20 Andras Timar 2013-05-13 11:43:39 UTC
It is a regression, because the bugdoc opens perfectly in Go-OO 3.2.1 (and even in AOO 3.4.1).
Comment 21 bfoman (inactive) 2013-07-24 13:43:06 UTC Comment hidden (obsolete)
Comment 22 QA Administrators 2014-10-23 17:31:59 UTC Comment hidden (obsolete)
Comment 23 Andras Timar 2014-10-23 17:58:30 UTC
The bug is still there in Version: 4.4.0.0.alpha1+
Build ID: dc6052564600688c7baf2be7f5ae07b8706695aa
I tested it on Windows 7.
Comment 24 Matthew Francis 2014-12-30 03:32:03 UTC
Adding Cc: to chris.sherlock79@gmail.com

Chris, here's another EMF bug that may interest you
Comment 25 Robinson Tryon (qubit) 2015-02-19 00:42:07 UTC
*** Bug 89376 has been marked as a duplicate of this bug. ***
Comment 26 Caolán McNamara 2015-11-06 13:29:53 UTC
Created attachment 120324 [details]
master screen shot as of today
Comment 27 Robinson Tryon (qubit) 2015-12-17 11:00:28 UTC Comment hidden (no-value, obsolete)
Comment 28 Timur 2016-04-05 16:53:56 UTC Comment hidden (obsolete)
Comment 29 Jean-Baptiste Faure 2016-04-08 05:11:51 UTC
(In reply to Timur from comment #28)
> Sorry, I can't see this as fixed in 5.1 or 5.2+. Please check.

Tested today with LO 5.1.3.0.0+ with the same bugdoc as Caolan and I get the same result.
I you disagree with Caolan and me, please attach your own screencopy showing how _this_ bug is still there.

If you see some problem with embedded EMF objects, please, open a new bug report with a detailed description and a test file.

Closing again. As we don't know which commit[s] fixed the bug, the right status is WorksForMe.

Best regards. JBF
Comment 30 Timur 2016-04-08 06:59:08 UTC
Created attachment 124176 [details]
master screen shot with LO52+ in Windows

My comment is for Windows, because bug is marked as "All". So please either mark as Linux or reopen.
Comment 31 Jean-Baptiste Faure 2016-04-08 08:05:02 UTC
(In reply to Timur from comment #30)
> Created attachment 124176 [details]
> master screen shot with LO52+ in Windows
> 
> My comment is for Windows, because bug is marked as "All". So please either
> mark as Linux or reopen.

It is another problem, the report is about OLE objects not displayed.
Please open a new bug report for this bad rendering.

Best regards. JBF
Comment 32 gvlatyshev 2016-04-08 09:39:11 UTC
Created attachment 124180 [details]
Another example of ole object

The content of the document is not displayed at all unless EMF_PLUS_DISABLE=1 variable is set

Fedora 21 x86_64
Libreoffice Version: 5.1.3.0.0+
Build ID: 8aa9fac23bc8f29de5e91293352f96c99418ddfc
CPU Threads: 1; OS Version: Linux 4.1; UI Render: default; 
TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:libreoffice-5-1, Time: 2016-04-07_13:49:34
Locale: ru-RU (ru_RU.UTF-8)
Comment 33 Bartosz 2017-05-05 00:11:21 UTC

*** This bug has been marked as a duplicate of bug 103639 ***
Comment 34 Bartosz 2017-05-05 19:32:58 UTC
A lot of EMF import issues was resolved witg LibreOffice 5.4.
Please download the LibreOffice 5.4 Daily Build from website:
http://dev-builds.libreoffice.org/daily/master/

and check if the problem was resolved for you.

Please check if LibreOffice is importing correctly OLE objects created by ChemDraw application.
Comment 35 Bartosz 2017-05-07 00:32:08 UTC
Created attachment 133111 [details]
Extracted emf+ image which illustrate wrong rendering of some lines (too thin)
Comment 36 Commit Notification 2017-05-07 17:02:31 UTC
Bartosz Kosiorek committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=ff7f5e1bbd4a9a3e3fa3e4ddb349c97605dc8a01

tdf#31814 Introduce minimal value of line width to fix EMF+ import issues

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 37 Commit Notification 2017-05-08 22:06:37 UTC
Bartosz Kosiorek committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=4c40aeeaf37bb3c0b780e7b0c2f9afe8c06091f5

tdf#31814 EMF+ Fix an issue when not all elements were displayed

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 38 Bartosz 2017-05-08 22:44:43 UTC
*** Bug 102321 has been marked as a duplicate of this bug. ***
Comment 39 Bartosz 2017-05-08 22:52:59 UTC
*** Bug 47243 has been marked as a duplicate of this bug. ***
Comment 40 Bartosz 2017-05-09 01:47:46 UTC
*** Bug 61083 has been marked as a duplicate of this bug. ***
Comment 41 gvlatyshev 2017-05-09 10:02:30 UTC
Created attachment 133184 [details]
shaded box
Comment 42 gvlatyshev 2017-05-09 10:04:37 UTC
The box is covered with gray box instead of shading
Comment 43 gvlatyshev 2017-05-09 10:13:43 UTC
Created attachment 133185 [details]
gradient-filled boxes

gradient filling is not displayed on all boxes

Version: 5.4.0.0.alpha1+
Build ID: 2b1737f648024328390bf44c4f2c614e748a92fd
CPU threads: 2; OS: Linux 4.1; UI render: default; VCL: kde4; 
TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2017-05-07_03:08:54
Locale: ru-RU (ru_RU.UTF-8); Calc: group
Comment 44 Bartosz 2017-05-09 13:09:56 UTC
Thank you gvlatyshev.
Unfortunately I don't own ChemDraw. Could you please separate figures into separate EMF files?
It will simplify the investigation process.

It will be also great to create new ticket for every issue you will find.
Please let me know (eg. via email), about any new ticket for EMF import.
Comment 45 Commit Notification 2017-05-12 12:43:58 UTC
Bartosz Kosiorek committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=43f5268c6fa394b0d219f8653ef827bdd531b4e4

EMF+ tdf#31814 Add support of reading EmfPlusBoundaryPointData

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 46 Bartosz 2017-05-18 12:49:59 UTC
*** Bug 97720 has been marked as a duplicate of this bug. ***
Comment 47 Bartosz 2017-05-18 20:21:29 UTC
*** Bug 47240 has been marked as a duplicate of this bug. ***
Comment 48 V Stuart Foote 2017-05-28 23:56:59 UTC
*** Bug 108207 has been marked as a duplicate of this bug. ***
Comment 49 Timur 2017-08-24 16:58:19 UTC
In Windows, I can't see why this was fixed, 5.4 and 6.0+ showed it wrong similarly to 5.3. 
With https://cgit.freedesktop.org/libreoffice/core/commit/?id=ebc11ae0b132eefd3b1b1a837a8d0ad3ba73b460 it's not blurry anymore but still not correct. So I set to Reopen.
Comment 50 V Stuart Foote 2017-08-24 22:49:50 UTC
(In reply to Timur from comment #49)
> In Windows, I can't see why this was fixed, 5.4 and 6.0+ showed it wrong
> similarly to 5.3. 
> With
> https://cgit.freedesktop.org/libreoffice/core/commit/
> ?id=ebc11ae0b132eefd3b1b1a837a8d0ad3ba73b460 it's not blurry anymore but
> still not correct. So I set to Reopen.

Yes, there is still something going on. Converting attachment 51694 [details] .doc with Word 2016 to OOXML, or to ODF, places all four images into a media directory.

The media are 3 EMF and a PNG as found in the document.

The EMF for the "Marvin Sketch" renders fine when the .doc/.docx/.odt is opened in current LO master. But the other two EMF (from Accelrys Draw, and ChemDraw) are only partially rendered and show a low resolution raster of the images lines but no lettering.

Weird as attachment 133111 [details] is pretty well formed (except for the dashed lines around the reaction box.
Comment 51 Chris Sherlock 2017-09-20 14:01:02 UTC
EmfPlusRecordTypeFillRegion - we get:

warn:cppcanvas.emf:49701:3004821:drawinglayer/source/tools/emfphelperdata.cxx:1682: EMF+ TODO unhandled record type: 0x4013

EmfPlusRecordTypeComment - we get:

warn:cppcanvas.emf:49701:3004821:drawinglayer/source/tools/emfphelperdata.cxx:1682: EMF+ TODO unhandled record type: 0x4003

I'm assuming that comment record holds the text.

On the latest build, the lines also aren't antialiased.
Comment 52 Chris Sherlock 2017-09-20 16:58:20 UTC
Created attachment 136406 [details]
EMF file from Accelrys Draw

EMF file from Accelrys Draw (formerly ISISDraw/SymyxDraw).

Note that the Comment record which has a signature of MDLS is proprietary data, we can ignore this.
Comment 53 Chris Sherlock 2017-09-20 17:08:52 UTC
Created attachment 136407 [details]
cppcanvas logging

630188s-iMac:core 5K$ export SAL_LOG=+INFO.cppcanvas
630188s-iMac:core 5K$ instdir/LibreOfficeDev.app/Contents/MacOS/soffice 2> ~/emf.log
Comment 54 Chris Sherlock 2017-09-20 17:15:28 UTC
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/wmfemfhelper.cxx:3051: EMF+ passed to canvas mtf renderer, size: 104
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:774: EMF+ record size: 12 type: EmfPlusRecordTypeSetTextRenderingHint flags: 4 data size: 0
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:1329: EMF+ SetTextRenderingHint
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:1330: EMF+	TODO
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:774: EMF+ record size: 48 type: EmfPlusRecordTypeObject flags: 1537 data size: 36
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:136: EMF+ Object slot: 1 flags: 1536
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfpfont.cxx:49: EMF+	font
EMF+	header: 0xdbc01 version: 0x1001 size: 71 unit: 0x3
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfpfont.cxx:50: EMF+	flags: 0x0 reserved: 0x0 length: 0x5
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfpfont.cxx:63: EMF+	family: ARIAL
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:774: EMF+ record size: 44 type: EmfPlusRecordTypeDrawString flags: 32769 data size: 32
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:1219: EMF+ DrawString
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:1224: EMF+ DrawString brushId: 4278190080 formatId: 4294967295 length: 1
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:1231: EMF+ DrawString layoutRect: 2468.02,3460.2 - 0x0
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:1234: EMF+ DrawString string: N

And before this there is a world transform. But you can see we do actually extract the "N" string, but where on earth is being painted to?!?
Comment 55 Patrick Jaap 2017-09-20 17:27:52 UTC
Hi, this bug looks strange, because the content of attached emf from comment 52 is pretty "simple" ... Since I contributed to the new emf(+) renderer, I will look into this.
Comment 56 Chris Sherlock 2017-09-21 07:11:22 UTC
Could it be world to device coordinate mapping that is causing this? Just a suggestion, I get confused how EMFs do affine transforms so I might not be much help here. 

It would be nice if there was a way of automatically turning on anti-aliasing by default.
Comment 57 Chris Sherlock 2017-09-21 07:13:32 UTC
Oh, also: I have submitted a patch to dump comment records in hex if you turn on INFO debugging in cppcanvas.emf

https://gerrit.libreoffice.org/#/c/42558/1/drawinglayer/source/tools/emfphelperdata.cxx

Comment records are a pain as they include proprietary data, but better for us to have a way of getting out a hex dump of the content than nothing. If you could have a look at that, would be appreciated.
Comment 58 Chris Sherlock 2017-09-21 21:55:33 UTC
Sorry, link should be https://gerrit.libreoffice.org/#/c/42558/
Comment 59 Patrick Jaap 2017-09-26 12:06:09 UTC
Hi, I investigated this bug. 
Attachment 136406 [details] is a strange file. It is basically just an EMF file containing all necessary info: text and lines are given there. But, there is a quite large EMF-Comment in beginning of the file, containing a lot of EMF+ data. Again, there is all info about lines and text given, too. Why is this redundant?

What bothers me: The file is only rendered in a very bad resolution and I don't know why. There are a lot of "DrawImage" records in the EMF+ part, which are crucial, because disabling them leads to a blank white picture... 

For what I saw, there is no bug in the EMF+ related code. My guess is there's a bug in the correspondence between EMF/EMF+ caused by the redundant records.
Comment 60 Bartosz 2017-09-28 15:32:47 UTC
I have take a look at EMF+ header, and the file is identified as dual file (dual: 1).

According to documentation:
Metafiles identified as EMF+ Dual can also contain both EMF+ records and EMF records. The
EMF+ Dual flag indicates that the metafile contains a complete set of EMF records sufficient to
correctly render the entire drawing on a system that cannot playback EMF+ records. This feature
makes it possible to render an image with different levels of graphics support in the operating
system. However, only one or the other type of records is processed. All records are enumerated
sequentially. For EMF playback, the metafile player only uses EMF records and ignores EMF+
records. For EMF+ playback, the metafile is played as if it is EMF+ Only.

There are two issues in Attachment 75221 [details] image display:
1. Low resolution of background image. It could be caused by not implemented image attributes:
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:1136: EMF+	TODO: use image attributes
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:1146: EMF+ DrawImage source rectangle: 4,4 100x98

2. Not visible strings which should be displayed by: 

info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:774: EMF+ record size: 44 type: EmfPlusRecordTypeDrawString flags: 32769 data size: 32
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:1224: EMF+ DrawString brushId: 4278190080 formatId: 4294967295 length: 1
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:1231: EMF+ DrawString layoutRect: 2468.02,3460.2 - 0x0
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:1234: EMF+ DrawString string: N

Maybe it was caused by wrong layoutRect, which is 2468.02,3460.2 - 0x0, but for DrawImage it is just 4,4 100x98
Comment 61 Chris Sherlock 2017-10-01 16:00:22 UTC
If this is a dual EMF/EMF+ record, then perhaps we should have a way of falling back to EMF only for now in a non-debug mode?

However, the world transform is:

info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:774: EMF+ record size: 36 type: EmfPlusRecordTypeSetWorldTransform flags: 0 data size: 24
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:1396: EMF+ SetWorldTransform
info:cppcanvas.emf:72789:3187441:drawinglayer/source/tools/emfphelperdata.cxx:1404: EMF+	m11: 0.133333	m12: -0	m21: -0	m22: 0.133333	dx: -304.02	dy: -424.681

So translate and scale the layout rect from 2468.02,3460.2 - 0x0... 

x coord = (2,486.02 - 304.02) * 0.133333 = 288.532612
y coord = (3,460.20 - 424.681) * 0.133333 = 404.734854827

Unless I'm miscalculating this? but the EMF+ picture frame goes from 0,0 - 1886,2217, so this location looks about right I think...
Comment 62 Commit Notification 2017-10-01 19:12:34 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=6996c65015703b6aaa6d44f76c492371f47b138d

tdf#31814 drawinglayer: dump EmfPlusRecordTypeComment records

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 63 Patrick Jaap 2017-10-02 09:57:49 UTC
Hi! Thanks for the additional info. 
In Comment 61, you made a little mistake: The transformation is done by scaling first and translating second. In your case

x coord = 2,486.02 * 0.133333 - 304.02  = 27.4
y coord = 3,460.20 * 0.133333 - 424.681 = 36.7

But I don't know what to do with this information... I'll go a little bit deeper in this bug. Thank for pointing out the "dual" thing. I never heard about that before. First, I need to study the documentation and then the code.
Comment 64 Patrick Jaap 2017-10-02 13:49:22 UTC
Hey, I found something!

Problem is the following: In dual mode, there the bool "bHaveDC" seems to be never set in the EMF+ Comments. therefore, the statement
(DC = Device context, and it tells the parser precess following EMF records)

else if( !mbEMFPlus || bHaveDC || nRecType == EMR_EOF )

in emfio/source/reader/emfreader.cxx line 700 is never true.

By manually setting it to "true" renders the EMF+ file together with the EMF content pretty fine. But it renders both in one picture, this looks not so nice. 

The EMF+ part looks really good. The question is now, should we precess the EMF or the EMF+ part?
Comment 65 Bartosz 2017-10-02 15:24:43 UTC
Created attachment 136699 [details]
Another EMF+ file with dual mode

I have attached another file with dual mode, when not all EMF+ records are displayed correctly. I spend ours in investigating why it is happening.
 
Generally for LibreOffice in dual mode we should display EMF+ by default (as EMF+ is more advanced than EMF). 

If Dual mode is not set, then EMF should be also displayed in case of EmfPlusGetDC record is set (in our case it is haveDC variable). More information is available at EmfPlusHeader Record. 

(Dual mode) - If set, this flag indicates that this metafile is EMF+ Dual, which means that it contains two sets of records, each of which completely specifies the graphics content. If clear, the graphics content is specified by EMF+ records, and possibly EMF records ([MS-EMF] section
2.3) that are preceded by an EmfPlusGetDC record. If this flag is set, EMF records alone SHOULD suffice to define the graphics content. Note that whether the EMF+ Dual flag is set or not, some EMF records are always present, namely EMF control records and the EMF records that contain EMF+ records.
Comment 66 Patrick Jaap 2017-10-02 15:40:55 UTC
Another update: I have written a small patch resolving this issue. It renders EMF+ records only, if dual mode is set. Like Bartosz just mentioned. 

There is a little trouble with the unit tests because as it turned out, all test files are in dual mode! I will give another update within the next few hours.
Comment 67 Patrick Jaap 2017-10-02 18:07:40 UTC
Last info for today: Sorry, I mixed it up. You can easily write a small patch that suppresses EMF+ recored and renders ONLY EMF records. Actually, this solves the bug and gives a fine output but does not take advantage of the better EMF+ functionality. 

However, while trying to force only EMF+ records to be rendered I noticed something: In the "render loop" in emfhelperdata.cxx, there are only a handful records passed, which is strange. Then I noticed, there is a "DrawImage" record which creates a BitmapsEx:

--> BitmapEx aBmp(image.graphic.GetBitmapEx()); <--

which calls an old VCL routine to parse the source file again. This seems not good and causes the low DPI! I'll start there the next time.
Comment 68 Chris Sherlock 2017-10-03 10:45:57 UTC
In that case, as Bartosz says, we should definitely use EMF+ for dual EMF+ records. :-)

Nice work guys! Every EMF/EMF+ fixes helps our rendering fidelity and helps win over converts to LO, what you two are doing here is really quite awesome.
Comment 69 Chris Sherlock 2017-10-03 11:22:39 UTC
Hi Patrick, is your patch on gerrit?
Comment 70 Patrick Jaap 2017-10-04 09:36:20 UTC
Hi Chris,

https://gerrit.libreoffice.org/#/c/43122

This is the patch I mentioned earlier. It uses the fallback EMF renderer in dual mode. Jenkins will fail, because it does not pass all unit tests since the test files are in dual mode.

Basically, it resolves this bug here! There is a slight shift in the position of the letters to the right. I went through the code were the positioning is done but there everything looks right.

The better way would be to go for the new EMF+ renderer. But as I mentioned before, there is trouble caused by the DrawImage record. So there are essentially two problems left:

 - text position in the EMF renderer
 - DrawImage record in the EMF+ renderer
Comment 71 Patrick Jaap 2017-10-04 12:43:26 UTC
Created attachment 136757 [details]
Document view with (first) dual mode patch applied

This is the document with the patch (43122 on gerrit) applied. There you can see the slight shift to the right of the letters.
Comment 72 Chris Sherlock 2017-10-04 19:24:16 UTC
Looks like it's failing with an off-by-one pixel failure.
Comment 73 Patrick Jaap 2017-10-05 07:55:20 UTC
Actually, it is more: the shift is about 4 logical units in the data and about 20 units in the device space.
Comment 74 Bartosz 2017-10-05 15:10:48 UTC
Hello Patrick.
What is happening when you are using EMF+ and disable DrawImage record?

As I understand DrawImage record is not working correctly. Do you have idea what could be wrong with it?
Comment 75 Patrick Jaap 2017-10-05 16:30:54 UTC
Then it renders blank. Just a white rectangle.
Comment 76 Bartosz 2017-10-09 14:38:42 UTC
Created attachment 136872 [details]
How LO 6.0 master is opening EMF+ attachment 75221 [details]

In attachment you could find how attachment 75221 [details] is opened by LO 6.0 master.


The strings on that EMF+ should be drawed by EmfPlusRecordTypeDrawString. Unfortunately it is not visible at all.

Generally only lines drawed by EmfPlusRecordTypeDrawLines records are visible.
But it has low resolution. Do you know the cause of this issue?
Comment 77 Patrick Jaap 2017-10-13 09:52:29 UTC
Hi, I submitted another patch:

https://gerrit.libreoffice.org/#/c/43367/

Yesterday I noticed, that I missed the case of a 'metafile' in DrawImage, therefore is was always rendered as a bitmap in low resolution, this is fixed with the patch (you can review it if you like)

The missing characters (as Bartosz) mentioned are caused by a missing "stringFormat" attribute in DrawString. Documentation says, it is optional, so I fixed that, too.

What the patch does not offer: There is still both EMF and EMF+ records rendered. I will upload the "dual mode patch" in a minute as a separate patch.

What is still unresolved: Both EMF and EMF+ puts the characters not in the right place (this is annoying!). For EMF it is shifted to the right, EMF+ is shifted to the left... I have no idea why. DrawString positioning is tested well, so there is no error I think. The lines and the strings use the same map/base/world transformation. The misplacement is already found in the raw coordinates in the EMF file. Maybe there is another hidden attribute that tells the letters to be aligned at the center or something.  But we will solve this one too!
Comment 78 Patrick Jaap 2017-10-13 10:32:47 UTC
https://gerrit.libreoffice.org/#/c/43122/2

here is the updated patch for the dual mode. It causes trouble with some unit tests. Maybe you can also take a look where this happens (or there is something completely wrong with the patch?)
Comment 79 Commit Notification 2017-10-15 09:31:15 UTC
Patrick Jaap committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=52a2a0101f71b21876f17d5419132ffa27f6e35d

tdf#31814 Fix for EMF+ DrawString and DrawImage

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 80 Commit Notification 2017-10-16 14:14:51 UTC
Bartosz Kosiorek committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f2410ad4d0f1e93c7f12ee51da9e1a1a90f0f5a4

tdf#31814 Resolve TODO from EMF+ DrawImage and DrawImagePoints

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 81 Commit Notification 2017-10-26 18:19:51 UTC
Patrick Jaap committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e06807f51d157844b357dab1bbf960cc55162330

tdf#31814 EMF/EMF+ implement dual mode

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 82 Timur 2017-11-03 11:49:33 UTC
Bartosz and Bartosz, thank you for your dedication to this issue. 
I have no idea whether it's all done, but I see it OK in 6.0+.
So I'll set as Fixed. 
Surely, if you have some comment or conclusion, you're welcome.
Comment 83 Timur 2017-11-03 11:50:09 UTC
Bartosz and Patrick, of course.
Comment 84 Patrick Jaap 2017-11-03 11:54:41 UTC
Hi! The general rendering is fine now. Thanks for closing this bug. 

But still, the position of the characters is not completely right. I have this in mind and I'm still looking for some solution. Can you open a separate bug for this and put me in CC?