trunk: Incorrect clipPath definition in SVG file based on EMF import

Bug #1294713 reported by su_v on 2014-03-19
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
Medium
David Mathog

Bug Description

Attached SVG file has this clip-path definition (generated by EMF import of an EMF exported with LibreOffice 4.2):

    <clipPath
       clipPathUnits="userSpaceOnUse"
       id="clipEmfPath1"><rect
   x="0"
   y="0"
   width="743.93488"
   height="1052.1365"
   id="rect3286" />

   transform=&quot;matrix(1,0,0,1,0,0)&quot;
</clipPath>

Attempting to release the clip in inkscape trunk results in a crash.

Steps to reproduce:
1) open LibreOffice Draw
2) draw a rectangle
3) export as EMF
4) open EMF in inkscape trunk
5) select the shape and release the clip

--> crash:
Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: 13 at address: 0x0000000000000000
0x000000010002937c in Inkscape::URIReference::getObject (this=0x1000000000000) at uri-references.h:79
79 SPObject *getObject() const { return _obj; }

The issue seems two-fold:
1) the EMF import creates an oddly format (or invalid?) transformation attribute stored as content instead of an attribute of the clip definition element.
2) trunk fails to handle such a clip-path definition and crashes.

First encountered with r13165 on OS X 10.7.5.

Note:
Inkscape 0.48.4 doesn't crash (releasing the clip works as expected), but produces this message on the console:
** (inkscape:42805): CRITICAL **: void sp_item_write_transform(SPItem *, Inkscape::XML::Node *, const Geom::Matrix &, const Geom::Matrix *, bool): assertion 'SP_IS_ITEM(item)' failed

su_v (suv-lp) wrote :
su_v (suv-lp) wrote :
description: updated
su_v (suv-lp) wrote :

@David - would you mind taking a closer look at the attached EMF file, and the resulting SVG file?

David Mathog (mathog) wrote :

Interesting. emf-inout creates the string:

<clipPath
        clipPathUnits="userSpaceOnUse"
id="clipEmfPath1" >
<rect
   x="0"
   y="0"
   width="743.93487"
   height="1052.1365" />
   transform="matrix(1,0,0,1,0,0)"
</clipPath>

but something elsewhere in Inkscape changes the double quotes around matrix to &quot;. If the bad svg is edited back to the preceding form it converts again when it is read in. The transform should be in the rect though, so that's probably the root cause.

David Mathog (mathog) wrote :

On linux this seems to resolve this issue. It moves the transform into the <rect> element.

However, something else is now dreadfully wrong, as the "testew.sh" script is failing. Not just failing, but crashing on the 2nd iteration. (That script opens libuemf_ref.emf saves as EMF, opens again, rinse and repeat 3 times.) gdb is saying that an entire loop around line 1112 in emf-inout.cpp has been "optimized out" when that code _cannot_ be optimized out (the program has no way of knowing a priori how many dash entries there are, the loop must exist). It looks like gdb's internal structures are being corrupted, since before anything happens a breakpoint can be set there and works.

This crash has nothing to do with the present bug, it happens even if the entire U_EMR_INTERSECTCLIPRECT section from roughly line 2148 to 2172 in emf-inout.cpp is commented out.

su_v (suv-lp) wrote :

> This crash has nothing to do with the present bug

Agreed - I'll file that separately (maybe I'll find a better test case for what triggers it).

Changed in inkscape:
assignee: nobody → David Mathog (mathog)
importance: Undecided → Medium
status: New → In Progress
tags: removed: crash
summary: - trunk: Crash when releasing a clip in SVG file based on EMF import
+ trunk: Incorrect clipPath definition in SVG file based on EMF import
su_v (suv-lp) wrote :

Patch tested successfully with r13165 on OS X.

[ Note: when reopening the same EMF file a second or third time in the same inkscape process (launch, open EMF file, close window, open EMF file from recent menu, close window, …) the rectangle is no longer clipped at all (with patched or unpatched Inkscape). ]

su_v (suv-lp) wrote :

Patch committed in r13166 - thx a lot!

Changed in inkscape:
milestone: 0.91 → none
status: In Progress → Fix Released
David Mathog (mathog) wrote :

@~suv Regarding opening the same EMF file, did you save on window close? That most definitely will lose the clip, because currently EMF only supports one type of clipping record (INTERSECTCLIP) and then only on input. I tried what you said in post 9, but without saving on closing, and the EMF opened the same each time.

David Mathog (mathog) wrote :

Correction, the one supported EMF record type is "INTERSECTCLIPRECT".

David Mathog (mathog) wrote :

The crash referred to in post 7 is entered as bug #1294840. A patch to fix it is posted there.

su_v (suv-lp) wrote :

> (…) did you save on window close?

No. I used these steps:

1) launch trunk with default (new) prefs
2) open 'Untitled 2.emf' via 'File > Open'
3) select the blue shape and check via message in the status bar where it is clipped or not
--> yes
4) close window (Ctrl+W)
5) open 'Untitled 2.emf' (via 'File > Open' or 'File > Open Recent'
6) select the blue shape and check via message in the status bar where it is clipped or not
--> no

(Same results when checking the content via XML Editor after step 5: no clip-path attribute nor a clip-path definition in the <defs>).

David Mathog (mathog) wrote :

OK, I see it now. There seems to be a problem with the initialization of the clipping region. Will look into it.

David Mathog (mathog) wrote :

This patch seems to fix it.

su_v (suv-lp) wrote :

Patch 'changes_2014_03_19d_emfonly.patch' tested successfully with r13167 on OS X 10.7.5, using 'Untitled 2.emf' attached in comment #1. Any side-effects to watch out for, or other sample files you'd recommend to run a quick test on with the patched build?

David Mathog (mathog) wrote :

Unfortunately I never had a very good test file for that type of EMF record. My test cases come from powerpoint output and from the libUEMF test file. The former never generated that type of record, and the latter only had a single instance of a cliprect record (which I never tried to unclip, obviously). However you constructed "Untitled 2.emf" could you make a test case like it but with multiple rectangular clipped regions? Verify that all of the clipped regions load OK, and that once loaded, if one is unclipped that the others stay clipped. Note that when you unclip one you may see a fill from the underlying now no longer clipped object over the entire page. That big object will likely need to be deleted to check/see the other objects.

su_v (suv-lp) wrote :

> However you constructed "Untitled 2.emf"

See bug description. The original drawing was a stroked rectangle drawn in a default new document in LibreOffice Draw (LO 4.2.2), and then exported to EMF. I didn't add the clip to the object in LO Draw myself - that was apparently done during export.

> could you make a test case like it but with multiple rectangular
> clipped regions?

I don't know how to control or verify this - if I export in LO Draw a simple drawing with multiple rectangles to EMF, I can "inspect" the result only with Inkscape (where just the rectangle drawn first (bottom-most in stack order) has a clip-path applied).

David Mathog (mathog) wrote :

Looked into "Untitled 6.emf" with "reademf" from libUEMF and it shows only a single INTERSECTCLIPRECT record which is the size of the entire drawing. Everything drawn after that falls between the next SAVEDC and RESTOREDC, so it should all be clipped, not just the first one. In this example the clipping does nothing useful, since all 3 rectangles are within the drawing.

Clearly I need to revisit this code. In its present incarnation it will only clip the first graphic element that follows the definition of the rectangular clipping region, but at least it no longer blows up when unclipping, and when it fails, it fails by doing no clipping. Not great, but better than clipping the wrong thing.

su_v (suv-lp) wrote :

> Clearly I need to revisit this code.

What about 'changes_2014_03_19d_emfonly.patch' right now? Would it be better to move the patch to a new report which tracks the refactoring of how clipping in EMF files is handled on import, or would you prefer to have it commit to trunk anyway?

David Mathog (mathog) wrote :

Go ahead and commit it.

The clipping code currently in emf-inout.cpp goes all the way back to bug #383180 and revision 10193. It was very rudimentary but before I can do it right I will first need to code (into the libUEMF test program) the appropriate test cases, so it may be a while before a more complete clipping version is available.

I looked into the loss of clipping information on EMF output and found that was just never implemented. It will take a fair amount of work to add that.

When all of this is ready it will be posted in a new thread.

David Mathog (mathog) wrote :

New clipping code: see bug #1302857

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers