EMF SetMiterLimit issues

Bug #1004084 reported by David Mathog
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
Fix Released
Low
David Mathog

Bug Description

There are a couple of problems with miter limits and EMF files.

1. SetMiterLimit, at least the one in mingw on XP accepts the miter limit as a float but actually saves it as 32 bit int.
This is inconsistent with Microsoft's documentation, which says it should be saved as a float. I need to find some other
graphics programs that support miter limit (neither PPT nor Xara does) to see what they do about this issue.

2. Inkscape is doing some odd scaling of the miter limit value. The miter limit is a dimensionless ratio, yet it currently goes through some scaling as if it was in PX or PT units. So that the internal value of 5 is sent to SetMiterLimit as 250. The
250 is then scaled back when read in by Inkscape (but won't be by any other program that reads the EMF).

3. When reading in the EMF the miter limit is subjected to a lower limit test: if <1 then 4. That is illogical. This may have
been an earlier attempt to work around (1), where reading in a small int as a float results in a value very close to zero.
The suggested change is to use 2 as a hard lower limit. This is large enough so that right angles are mitered and small enough so that more acute angles may not be, if that is what an artist intends. The lower limit cannot be 1 because then
right angles could never be mitered (1 < sqrt(2)), and because of issue (1), the limit must be an integer. So 2.

Suggested changes to fix/work around these issues:

in emf-win32-print.cpp create_pen method change the section starting with the next line to:

        if (linejoin == PS_JOIN_MITER) {
            float oldmiterlimit;
            float miterlimit = style->stroke_miterlimit.value; //ratio, not a pt size

            SetMiterLimit(
                hdc,
                miterlimit,
                &oldmiterlimit );
        }

in emf-win32-inout.cpp myEnhMetaFileProc function change the section starting with the next line to:

        case EMR_SETMITERLIMIT:
        {
            dbg_str << "<!-- EMR_SETMITERLIMIT -->\n";

            PEMRSETMITERLIMIT pEmr = (PEMRSETMITERLIMIT) lpEMFR;

            //BUG in SetMiterLimit() on mingw, possibly in underlying Windows(at least on XP?)
            //The function takes a float but saves a 32 bit int in the EMR_SETMITERLIMIT record.
            float miterlimit = *((int32_t *) &(pEmr->eMiterLimit));
            d->dc[d->level].style.stroke_miterlimit.value = miterlimit; //ratio, not a pt size
            if (d->dc[d->level].style.stroke_miterlimit.value < 2)
                d->dc[d->level].style.stroke_miterlimit.value = 2.0;
            break;

su_v (suv-lp)
tags: added: emf exporting win32
Revision history for this message
David Mathog (mathog) wrote :

Regarding issue (1) attached is rect2.emf, produced by Adobe Illustrator (11, I think) on a Mac. It too has the miter limit stored as an integer. Amazingly, for such a simple file (two rectangular paths with different miter limits) it also crashes Inkscape, but that happens some time after it hits EMR_EOF, and debug statements at the miter read section showed that
that data was correctly read. The points also seemed to be read correctly, kind of mysterious why it crashes.

Revision history for this message
David Mathog (mathog) wrote :

Toxicity of that test file is due to the description string. A hex dump of the part that prints is:

3f 3f 3f 3f 3f 20 3f 3f 3f 3f 3f 3f 3f 3f e1 6a 3f 23 3f

and this triggers (not sure what this will look like once pasted in):

Entity: line 2: parser error : xmlParseComment: invalid xmlChar value 28
<!-- ????? ???????∟áj♥#♥ -->
                  ^
Entity: line 2: parser error : Start tag expected, '<' not found
<!-- ????? ???????∟áj♥#♥ -->
                  ^

Emergency save activated!
Emergency save completed. Inkscape will close now.

28 is ASCII file separator symbol, not clear if that would print to a Windows command prompt.
Anyway, the problem is seen right after all of the question marks.

Apparently it isn't safe to stuff just any row of binary data into an XML statement. No big surprise there.

Revision history for this message
David Mathog (mathog) wrote :

Found a reference.

The problem is that in most cases the struct EMRWHATEVER is the same as the record EMR_WHATEVER.
That is NOT the case for this one record. Here is the documentation for EMR_SETMITERLIMIT (the record)

  http://msdn.microsoft.com/en-us/library/cc230584%28v=prot.13%29

which shows that the record is supposed to hold an integer. Whereas the equivalent struct EMRSETMITERLIMIT
described here

 http://msdn.microsoft.com/en-us/library/dd162583%28v=vs.85%29

uses a float.

su_v (suv-lp)
tags: added: importing
Revision history for this message
su_v (suv-lp) wrote :

<off-topic>
> in emf-win32-print.cpp create_pen method change the section
> starting with the next line to:
> (…)
> in emf-win32-inout.cpp myEnhMetaFileProc function change
> the section starting with the next line to:
> (…)

It would be appreciated if you could provide patches or diff files against a current revision of trunk [1]: this would ease the process to compare&review the proposed changes, instead of expecting reviewers to tediously locate & compare the mentioned function manually in order to figure out what changes are actually proposed.

[1] <http://doc.bazaar.canonical.com/bzr.2.5/en/user-reference/diff-help.html>
    <http://wiki.inkscape.org/wiki/index.php/Working_with_Bazaar#Inspecting_changes>
    <http://wiki.inkscape.org/wiki/index.php/Working_with_Bazaar#Working_with_patch_files>
</off-topic>

Revision history for this message
David Mathog (mathog) wrote :

Changes to trunk r11367, as a patch

Revision history for this message
David Mathog (mathog) wrote :

rect2.emf is kind of interesting as it suggests that EMF files are not portable between platforms (or at least the ones made by that version of Adobe Illustrator are not.) rect2.emf was made on an old Power Mac. It uses the original EMF format header so it is missing a few pieces in the EMR_HEADER that are present in current versions:

http://msdn.microsoft.com/en-us/library/cc230635%28v=prot.13%29.aspx.

In addition, the extra header description has the byte order in the 16 bit chars reversed from Windows. (But oddly, not the 4 byte integers) So when GetEnhMetaFileDescription in Inkscape on Windows tries to retrieve the description from this file it comes back with garbage, apparently also stepping on memory structures in the process, since the program usually crashes shortly thereafter.

Revision history for this message
Alvin Penner (apenner) wrote :

thanks for the patch, David!
(I just made a minor change to allow the miterlimit to go as low as 1)

test and committed to bzr rev 11427

Changed in inkscape:
status: New → Fix Committed
Kris (kris-degussem)
Changed in inkscape:
importance: Undecided → Low
assignee: nobody → David Mathog (mathog)
milestone: none → 0.49
Revision history for this message
David Mathog (mathog) wrote :

Regarding lower limit for miterlimit - under what circumstances would a miter limit of 1 be appropriate?

The miter limit is the ratio of the mitered join to the line width. If Theta is the angle in degrees (range 0 to 180) of the interior corner formed by the join, W the width of the line, then the miter length is W/(cos(90 -(Theta/2))) giving a ratio of 1/(cos(90 -(Theta/2))). This value can only be as low as 1 on a straight line (180 degrees), in which case the miter limit is pretty much a moot point. So setting miter limit to 1 is equivalent to forcing "bevel" for all joins - which can be done better by actually setting the join to bevel. The lower limit I used was 2, because that allows miters on 90 degree (ratio sqrt(2) = 1.4) and larger angle joins while tripping over to bevel for more acute angles. It isn't at all clear to me that one would ever want to set it as low as 2 (the default value in windows is 10, in inkscape 4), but at least 2 sets a limit such that some joins are mitered and some are beveled.

Revision history for this message
Alvin Penner (apenner) wrote :

sorry about that, I am not very familiar with typical usage of these parameters. My original motivation for allowing a miter limit of 1 was that the file rect2.emf from comment 1 above has a miterlimit = 1. I wanted to emulate that shape so I produced the attached file miter1.svg which has the properties:
stroke-linejoin = miter
stroke-miterlimit = 1

Revision history for this message
Alvin Penner (apenner) wrote :

When exported to emf this file retains its shape, miter1.emf. And when the emf is re-imported back into Inkscape it retains its original shape, as hoped for.

Bryce Harrington (bryce)
Changed in inkscape:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.