image handling in sp-image corrupts EMF/WMF output

Bug #1278645 reported by David Mathog on 2014-02-10
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
Medium
David Mathog

Bug Description

I have noticed for a while that images were being mangled on round trips (EMF/WMF/SVG)->(WMF/EMF) through inkscape (via the testew.sh script in bug #1248753), and finally got around to hunting it down. The problem is that in sp-image.cpp this code:

            int trimwidth = std::min<int>(w, ceil(this->width.computed / vw * w));
            int trimheight = std::min<int>(h, ceil(this->height.computed / vh * h));

changes (w,h,rs) for the pixel map from the original (10,10,40) to (5,5,40). I don't understand what this is supposed to be doing, but what it is doing is sending a bitmap further down the road with

            sp_print_image_R8G8B8A8_N(ctx, px + trimx*pixskip + trimy*rs, trimwidth, trimheight, rs, t, this->style);

 until it hits the EMF/WMF output routines with the wrong information. (The bitmap is not 5,5, but it does have a row stride of 40.) To demonstrate this bug, open the example file, and save as EMF. Then reopen the saved EMF, and the image will be different.

Values when this happens are:
w = 10
this->width.computed = 4.233
vw = 10

which with the ceiling becomes 5. If the two lines above are changed to:

            int trimwidth = w;
            int trimheight = h;

images can round trip through EMF without changing at all. (I will post the patch for this next, but I do not think this is likely to be the root cause of this issue.)

In what is likely a related problem, images can no longer be stretched except by equal amounts in x and y. The image stretches interactively, but when the cursor lets go of the object the image is found inscribed in a rectangular select box of the new dimensions, but the image has the same h/w ratio as before. To reproduce this, open the attached image, select it, grab the lower right corner and pull down about 2 height's worth and right slightly, then release the mouse button. This does not invoke the EMF code sections at all.

David Mathog (mathog) wrote :
David Mathog (mathog) wrote :

This small patch fixes the image output problem for EMF, WMF. It has no affect on the stretching issue. It is presented just to demonstrate that the issue runs through this section, I think the real problem is elsewhere.

su_v (suv-lp) wrote :

As mentioned in bug #1263242 comment 45, these image-related regressions are triggered by changes in r13002:
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/13002>

Based on tests with archived builds on OS X 10.7.5:
1) rev <= 13001 produce expected results
2) rev >= r13002 are affected:
  - incorrectly reproduced bitmap images in EMF round-trip editing
  - non-uniformly scaling of linked or embedded bitmap images on-canvas fails

tags: added: bitmap regression transformations
su_v (suv-lp) wrote :

Wrt bug triage and bug importance, the incorrect rendering of scaled (stretched, squeezed) bitmap images would have a higher priority (it affects all users in very basic workflows, and is not limited to round-trip editing a specific type of file formats). It also seems to be in rendering only (the same SVG file created with r13018 with a stretched bitmap image renders correctly in r13001).

Changed in inkscape:
importance: Undecided → Medium
milestone: none → 0.91
status: New → Confirmed
su_v (suv-lp) wrote :

<off-topic wrt EMF/WMF>
Fixing the rendering "regression" of scaled bitmap images might actually offer an opportunity to improve SVG 1.1 compliance (see bug #924377, bug #461467, bug #616717).
</off-topic>

David Mathog (mathog) wrote :

Observation:

If this is added to the <image> structure in the test file:

     preserveAspectRatio="none"

then image export to EMF works again. So apparently the default value for preserveAspectRatio is doing something odd with the scaling, even when the image is not actually scaled.

David Mathog (mathog) wrote :

2nd observation:

adding

     preserveAspectRatio="none"

just below the <viewbox> line in the initial <svg> tag does NOT resolve the image export issue in EMF/WMF.

David Mathog (mathog) wrote :

3rd observation:

at present (r13024) no preserveAspectRatio is required to maintain ratio, or to allow stretching, when an image is used in a pattern. See the attached examples, first the SVG source, then the EMF and WMF files created from it.

David Mathog (mathog) wrote :
David Mathog (mathog) wrote :
David Mathog (mathog) wrote :

This patch seems to work for EMF and WMF. I would bet $10 it breaks LaTEX output, but I have no experience with that, so somebody else will have to test it.

David Mathog (mathog) wrote :

Updated test case. Hand edited SVG to support two different kinds of preserveAspectRation.
 EMF supports conversion of the rotated images, WMF does not.

David Mathog (mathog) wrote :

Result of reading in imageA.svg and "save as" to EMF. Both types of preserveAspectRatio look right.

David Mathog (mathog) wrote :

Result of reading in imageA.svg and then "save as" to WMF. Both types of preserveAspectRatio look right. The rotated examples have odd image sizing, but WMF doesn't support rotated images in any way shape or form, so there is no "right" image for that part of the test.

I forgot to mention above that the patch adds preserveAspectRatio="none" on EMF and WMF image imports. So afterwards they stretch as an end user would expect.

su_v (suv-lp) wrote :

Patch 'changes_2014_02_11b_aspectonly.patch' tested successfully with r13026 on OS X 10.7.5.
(Caveat: I don't use LaTeX either, and can't verify the status of latex output with the patch applied).

@David - any updates to the patch planned (based on the discussion on the ML), or is it ready to get committed as is?

Changed in inkscape:
assignee: nobody → David Mathog (mathog)
status: Confirmed → In Progress
David Mathog (mathog) wrote :

I think it is OK to apply it to trunk.

In the related thread on the developer's list somebody said that LaTEX doesn't actually do image output. Consequently, this patch cannot break LaTEX. I never use LaTEX, but I was curious to see why it was not implemented. The reason seems to be that LaTEX implements images through a sort of "include" mechanism which operates on external files. It has some primitive placement and scaling options to go with the include. So if Inkscape were to support this it would have to write the LaTEX file and N separate image files in some other format. Not that that would be all that hard to do, just convert them to PNG and save them, making up some sort of UUID like name to apply for each.

su_v (suv-lp) wrote :

Patch committed in r13049.

Changed in inkscape:
status: In Progress → Fix Committed
Changed in inkscape:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers