WMF/EMF open via GUI misplaces drawing if document window is reused

Changes one line in wmf-inout.cpp and restores the previous behavior on linux - drawing opens correctly aligned with the same sized page.

Related: see comments of bug #1244793 (the fix for the crash introduced the incorrect position).

I'm going to close bug #1244793 as 'Fix Released' (no more crashes as reported) - the remaining issue will be tracked in this report.

1) The problem exists for both, EMF and WMF files. Your bug summary and the title for the attached file mentions WMF; the attached file itself however is EMF.

2) The patch only addresses WMF, and does not fix the import of the attached file (it is an EMF file).

3) The patch reintroduces the crash when opening such a file from the command line (bug #1244793).

Ugh, this issue is worse than I thought.

It also affects EMF but in a strange way. Open an emf file and it looks like it is in the right place, but then save as to "test.emf", and it gets shifted downward by about 1/3, the same way that the wmf file was when it was just opened. That only becomes evident when the new file is read back in though. That problem can be eliminated by making the same change to emf-inout.cpp that the preceding patch made to wmf-inout.cpp.

Unfortunately that is not the end of it. Inkscape used to be round trip stable for simple EMF files, that is, a line segment or a rectangle or something simple like that could be read in, written back out to EMF, and the end points in the EMF file did not change. That seems to have broken somewhere along the line, since the test file attached is off by 1 on Y on most coordinates when it is written back out to disk. (I use reademf from libuemf to dump the EMF files as text, and then use diff for comparison.) There may be other +/-1 coordinate issues, I have not looked very carefully, because there is an even bigger problem - the EMF file headers are wildly different. (This could explain all the +/-1 issues too.)

This is the original header from the test file (just the scale and coordinate values):

   rclBounds: {0,0,14030,9920}
   rclFrame: {0,0,29699,20999}
   szlDevice: {10205,13181}
   szlMillimeters: {216,279}
   szlMicrometers: {216000,279000}

This is what r12792 writes out

   rclBounds: {0,0,1051,743}
   rclFrame: {0,0,29699,20999}
    szlDevice: {1020472,1318110}
   szlMillimeters: {21600,27900}
   szlMicrometers: {21600000,27900000}

It is not OK to change values like that - the drawing went from being A4 to 100X A4!
I suspect that most of the +/-1 coordinate changes are due to the slightly different ratios
of 1051/743 and 14030/9920. (EMF uses integer coordinates.)

Presumably this is all related to the Viewport changes.

Sent the emf test the first time, bad click, they are right next to each other in the file list.

Re #4.

Please ask whoever added the Viewport code to EMf/WMF to revisit that work.

All of these issues seem to be more of a Viewport problem than an EMF/WMF one.

Bug #1248354 is probably also related to the Viewport code.

Clarification: only Bug #1238354 section 4 is likely related to Viewport code, sections 1-3 are not.

David Mathog wrote:
> Re #4.
> Please ask whoever added the Viewport code to EMf/WMF to revisit that work.

Reverting assignment.

For testing purposes, it would help to have a complete list of all EMF/WMF "open" modes, since what works for one does not always work for the other. The ones I know of are:

1. In inkscape GUI, "open" from file menu.
2. In inkscape GUI, "paste" from clipboard.
3. In inkscape GUI, "import" from file menu.
4. From command line: inkscape --file name.[e|w]mf.

Are there any others?

This patch seems to work on all 4 open modes. However, it is more of a workaround than a fix. The real problem is that the fix for the --file open mode crash returns incorrect height values for file menu->open. This patch just avoids that by falling back to the previous viewbox method whenever SP_ACTIVE_DOCUMENT isn't null.

Oddly the file offset problem for the current bug does not seem to occur when the new method is used (for --file).

Will try to hunt down whatever is changing the header settings and causing the +/-1 shifts in coordinates.

Noted this behavior:

src/inkscape --file /usr/local/src/libUEMF/dist/test_libuemf_ref.emf --file /usr/local/src/libUEMF/dist/test_libuemf_ref.wmf

** (inkscape:6300): WARNING **: master 0xc60a920: unable to add object 0xddc1930[DialogFillStroke] to the hash. There already is an item with that name (0xdce4160).

The attached patch replaces the previous one.

As far as I can tell it supports all 4 file open modes, it is still a work around, but it seems to work.

I also hunted down the "round trip" problems discovered while testing and fixed them, those are included in this patch. These were:

1. EMF header fields. These have been modified slightly so that rclBounds is round trip stable. The device size fields were reduced from 100xA4 to just A4. As I vaguely recall the original reason for using the larger device had something to do with increasing the resolution of the original Windows EMF driver version when writing the EMF through the windows library. The larger device doesn't do anything with the libUEMF driver, so the fields can be the more natural A4.

2. The Inkscape::Util::Quantity::convert business broke the output of dashed and dotted lines and I had not noticed. What used to be 3 or 1 are now divided by 3.543307 ( = 90/25.4), stored in an integer, which made them zero. The fix multiplies that factor back in before storing into the integer.

3. The Inkscape::Util::Quantity::convert business caused a problem with worldtransforms used before image writes. These became unstable so that while the Destination for the image was stable, the offset in the transform jumped around by a fraction of a pixel. The fix involves working backwards from the integer Destination coordinates to the floating point transform.

4. Off by 1 values. That seems to have gone away with the workaround. I really do not understand why though.

Will come up with a small test example and post it next.

Because it was scriptable, the round trip testing was pretty thorough using the --export-emf and --export-wmf methods (and the patch from bug #1244749 to enable it). Some manual file->open testing was also done, but since that was all manual, it was only done a few times.

Attached is a test case for the various bugs mentioned above and the round trip testing. Apply the patch from bug #1244749 to get the export option. Get libUEMF from sourceforge and build reademf for the comparisons.

src/inkscape --file=/tmp/round0.emf --export-emf=/tmp/round1.emf
src/inkscape --file=/tmp/round1.emf --export-emf=/tmp/round2.emf
reademf round0.emf >round0_emf.txt
reademf round1.emf >round1_emf.txt
reademf round2.emf >round2_emf.txt
% diff round0_emf.txt round2_emf.txt
13c13
< Desc. B: round0.emf
---
> Desc. B: round2.emf
% diff round0_emf.txt round1_emf.txt
13c13
< Desc. B: round0.emf
---
> Desc. B: round1.emf

That is, the text dump of the result emf files differ only in the internal name field.
The same file can be used form wmf testing.

Ah, I see now why I thought this was happening for WMF and not EMF.

start inkscape
open WMF file (displaced)
open EMF file (not displaced)

which was how I started doing the testing, but

start inkscape
open EMF file (displaced)
open WMF file (not displaced)

So it wasn't WMF vs. EMF, it was the order in which the files were opened.

mathog - What's the status of the bug and the patch? Is it something that can be applied to trunk or would you like to work on it some more?

Somebody should test it on a Mac, see post 13 for one method of doing so. If you don't have a Mac it is probably OK to apply without testing it, since there is nothing particularly platform specific in this patch, and it was tested already on both Linux and Windows.

The sections at 3201 in wmf-inout.cpp and 3516 in emf-inout.cpp are still work arounds rather than fixes. I don't know why getHeight() works in some cases and not in others, but the patch works in all of the tested cases whatever that underlying problem is.

Otherwise, it should be good to go.

Thanks.

Fixes for issue originally reported committed as part of patches for bug #1263242:
- r12968 includes fix for EMF files
- r13008 includes fix for WMF files