Bug 79679 - EMF: dashed lines in background grid are rendered as solid lines
Summary: EMF: dashed lines in background grid are rendered as solid lines
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
4.3.0.0.beta1
Hardware: x86-64 (AMD64) All
: medium major
Assignee: Chris Sherlock
QA Contact:
URL:
Whiteboard: target:5.2.0 target:5.1.2 target:5.0.6
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2014-06-05 12:17 UTC by Karel Hruska
Modified: 2016-10-25 19:02 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:


Attachments
Rendering in LibreOffice 4.3.0.0 beta1 in comparison to rendering in LibreOffice 4.0.6.2 (170.69 KB, image/png)
2014-06-05 12:17 UTC, Karel Hruska
Details
EMF file generated by Matlab (imported broken) (33.43 KB, image/x-emf)
2014-06-05 12:19 UTC, Karel Hruska
Details
EMF file generated by GNU/Octave (imported OK) (146.41 KB, image/x-emf)
2014-06-05 12:20 UTC, Karel Hruska
Details
QA test document (23.02 KB, application/vnd.oasis.opendocument.text)
2016-03-04 12:07 UTC, Chris Sherlock
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Karel Hruska 2014-06-05 12:17:15 UTC
Created attachment 100457 [details]
Rendering in LibreOffice 4.3.0.0 beta1 in comparison to rendering in LibreOffice 4.0.6.2

Figures saved in Matlab as metafiles (*.emf) and imported into LibreOffice 4.3.0.0 beta 1 show incorrect rendering - dashed lines in background grid are rendered as solid lines (see attachment for comparison of rendering between LibreOffice 4.0.6.2 and LibreOffice 4.3.0.0 beta 1). Tested platforms: Windows 7 64-bit (screenshot), Ubuntu 12.04.0 amd64.

Steps to reproduction:
1. Open a new document
2. Import (drag&drop) the attached EMF file (attachment #2)

In LibreOffice 4.0.6.2
Dashed lines from Matlab are rendered as dashed lines

In LibreOffice 4.3.0.0 beta 1
Dashed lines from Matlab are rendered as solid lines

I did a bibisection in order to find the suspicious commit with following result:

There are only 'skip'ped commits left to test.
The first bad commit could be any of:
67ae616bc846d2a4e05661a5980287cb38b8a455
bde12e7d6eef3d657ebdf62cb5442490fb90d899
7faf01595aded0c825d2d9e50c62c688e91c1496
We cannot bisect more!

Note: EMF figures imported from the GNU/Octave are rendered correctly (attachment #3).
Comment 1 Karel Hruska 2014-06-05 12:19:05 UTC
Created attachment 100458 [details]
EMF file generated by Matlab (imported broken)
Comment 2 Karel Hruska 2014-06-05 12:20:03 UTC
Created attachment 100459 [details]
EMF file generated by GNU/Octave (imported OK)
Comment 3 Jean-Baptiste Faure 2014-06-09 15:48:22 UTC
Tested with LO 4.3.0.0beta2+, LO 4.2.6.0+, LO 4.1.6 and LO 4.0.6 under Ubuntu 14.04 x86-64:
- EMF Gnu/Octave imported OK in all versions
- EMF Matlab imported OK in LO 4.0.6 (generic Linux version)
- EMF Matlab in LO 4.1.6 and 4.2.6.0+ : I see only the axis and the grid with dashed lines, not the curves.
- EMF Matlab in LO 4.3.0.0beta2+ : I see only the curves but not the axis nor the grid.

So there is obviously a bug and it is a regression because it works in LO 4.0.6

Best regards. JBF
Comment 4 Karel Hruska 2014-06-10 07:08:12 UTC
Jean-Baptiste,
please could you try some zooming of the document with Matlab EMF in 4.3.0.0beta2+? In 4.3.0.0 beta 1 the grid appeared after zoom-out, fresh import showed only curves.

Thank you,
KH

(In reply to comment #3)
> Tested with LO 4.3.0.0beta2+, LO 4.2.6.0+, LO 4.1.6 and LO 4.0.6 under
> Ubuntu 14.04 x86-64:
> - EMF Gnu/Octave imported OK in all versions
> - EMF Matlab imported OK in LO 4.0.6 (generic Linux version)
> - EMF Matlab in LO 4.1.6 and 4.2.6.0+ : I see only the axis and the grid
> with dashed lines, not the curves.
> - EMF Matlab in LO 4.3.0.0beta2+ : I see only the curves but not the axis
> nor the grid.
> 
> So there is obviously a bug and it is a regression because it works in LO
> 4.0.6
> 
> Best regards. JBF
Comment 5 Michael Stahl 2014-06-10 14:48:39 UTC
with current libreoffice-4-3, with zoom levels 80% - 140% the grid disappears,
outside that range grid is visible.  oh at 250%-310% it also disappears.

does not happen in 4.2.4.2.
Comment 6 Jean-Baptiste Faure 2014-06-12 17:12:43 UTC
Indeed if I zoom on the document, the grid appears as said by Michael Stahl.
We have the same problem in the preview.
Tested with LO 4.3.0.0beta2+ under Ubuntu 14.04 x86-64

Best regards. JBF
Comment 7 Karel Hruska 2014-08-05 06:44:44 UTC
The bug is still present in final release of LibreOffice 4.3.0.
Comment 8 Jean-Baptiste Faure 2014-08-05 07:01:37 UTC
Please, do not change the version number which shows the oldest version in which the bug has been seen.

Best regards. JBF
Comment 9 Karel Hruska 2014-08-05 07:09:35 UTC
(In reply to comment #8)
> Please, do not change the version number which shows the oldest version in
> which the bug has been seen.
> 
> Best regards. JBF

I am sorry. Thank you for notification.
Comment 10 Matthew Francis 2015-01-06 07:21:31 UTC
Not sure how relevant this is to the current state of EMF, but the (or at least a) commit which broke this is below

Adding Cc: to chris.sherlock79@gmail.com - enother EMF bug for you


commit 09c722873b2d378d2d155f5f1dd7d8f3fb2012e9
Author: Andras Timar <andras.timar@collabora.com>
Date:   Sun Jan 19 15:12:15 2014 +0100

    EMF/WMF: fix rendering of pen styles (dash, dot, dashdot, dashdotdot)
    
    Change-Id: I226bac370601b75f2589f7a7c5e8830746b31e2e
Comment 11 Xisco Faulí 2015-08-20 10:21:43 UTC
Problem still present in

Version: 5.0.0.5
Build ID: 1b1a90865e348b492231e1c451437d7a15bb262b
Locale: es-ES (es_ES)

on Windows 7 (64-bit)
Comment 12 Robinson Tryon (qubit) 2015-12-13 11:09:21 UTC Comment hidden (obsolete)
Comment 13 Chris Sherlock 2016-02-23 14:23:42 UTC
I *think* the issue might be in the following hunk of that commit identified by Matthew:


diff --git a/vcl/source/filter/wmf/winmtf.cxx b/vcl/source/filter/wmf/winmtf.cxx
index 1b204c5..78530a7 100644
--- a/vcl/source/filter/wmf/winmtf.cxx
+++ b/vcl/source/filter/wmf/winmtf.cxx
@@ -713,14 +713,6 @@ void WinMtfOutput::CreateObject( GDIObjectType eType, void* pStyle )
         {
             Size aSize( ((WinMtfLineStyle*)pStyle)->aLineInfo.GetWidth(), 0 );
             ((WinMtfLineStyle*)pStyle)->aLineInfo.SetWidth( ImplMap( aSize ).Width() );
-            if ( ((WinMtfLineStyle*)pStyle)->aLineInfo.GetStyle() == LINE_DASH )
-            {
-                aSize.Width() += 1;
-                long nDotLen = ImplMap( aSize ).Width();
-                ((WinMtfLineStyle*)pStyle)->aLineInfo.SetDistance( nDotLen );
-                ((WinMtfLineStyle*)pStyle)->aLineInfo.SetDotLen( nDotLen );
-                ((WinMtfLineStyle*)pStyle)->aLineInfo.SetDashLen( nDotLen * 4 );
-            }
         }
     }
     sal_uInt32 nIndex;
@@ -749,14 +741,6 @@ void WinMtfOutput::CreateObject( sal_Int32 nIndex, GDIObjectType eType, void* pS
             {
                 Size aSize( ((WinMtfLineStyle*)pStyle)->aLineInfo.GetWidth(), 0 );
                 ((WinMtfLineStyle*)pStyle)->aLineInfo.SetWidth( ImplMap( aSize ).Width() );
-                if ( ((WinMtfLineStyle*)pStyle)->aLineInfo.GetStyle() == LINE_DASH )
-                {
-                    aSize.Width() += 1;
-                    long nDotLen = ImplMap( aSize ).Width();
-                    ((WinMtfLineStyle*)pStyle)->aLineInfo.SetDistance( nDotLen );
-                    ((WinMtfLineStyle*)pStyle)->aLineInfo.SetDotLen( nDotLen );
-                    ((WinMtfLineStyle*)pStyle)->aLineInfo.SetDashLen( nDotLen * 4 );
-                }
             }
         }
         if ( (sal_uInt32)nIndex >= vGDIObj.size() )
Comment 14 Chris Sherlock 2016-02-23 15:39:30 UTC
Oh dear. I believe that we have a problem when we read an EMT_CREATEPEN record...

We read the "style" (ihPem) into a 32 bit integer. Then we read the LogPen structure. We seem to be reading it wrongly :-(

    895     case EMR_CREATEPEN :
    896     {
    897         pWMF->ReadUInt32( nIndex );
    898         if ( ( nIndex & ENHMETA_STOCK_OBJECT ) == 0 )
    899         {
    900 
    901             LineInfo    aLineInfo;
    902             sal_uInt32      nStyle;
    903             Size        aSize;
    904             // #fdo39428 Remove SvStream operator>>(long&)
    905             sal_Int32 nTmpW(0), nTmpH(0);
    906 
    907             pWMF->ReadUInt32( nStyle ).ReadInt32( nTmpW ).ReadInt32( nTmpH );
    908             aSize.Width() = nTmpW;
    909             aSize.Height() = nTmpH;

That second value we are reading MUST be ignored! We show set it to something default.

See https://msdn.microsoft.com/en-us/library/cc230566.aspx

Anyway, aside from this, we appear to be overriding the width with hard coded values. I'd say it this is where things go wrong:

https://cgit.freedesktop.org/libreoffice/core/commit/vcl/source/filter/wmf/winwmf.cxx?id=09c722873b2d378d2d155f5f1dd7d8f3fb2012e9

Will test this out when I get home from hospital.
Comment 15 Chris Sherlock 2016-02-23 15:41:10 UTC
Steven, I notice you are assigned to this one. Do you want to run with this?
Comment 16 Chris Sherlock 2016-02-23 15:44:58 UTC
(Whoever takes this... Can you change nTmpW to nTmpWidth and nTmpH to nTmpHeight? Driving me insane, there's no reason to shorten a one off variable name like that that... Grrr...)
Comment 17 Chris Sherlock 2016-02-23 15:48:38 UTC
Oh, just realised I didn't say where I got that code snippet from...

vcl/filter/WMF/enhwmf.cxx - function is EnhWMFReader::ReadEnhWMF()

http://opengrok.libreoffice.org/xref/core/vcl/source/filter/wmf/enhwmf.cxx#895
Comment 18 Chris Sherlock 2016-02-27 23:23:49 UTC
Have been ill. Sorry for the delay. I'm actually bisecting this bug now to work out precisely where things fell over.
Comment 19 Chris Sherlock 2016-03-04 11:59:33 UTC
Ah, mjfrancis was entirely correct and there was no need for me to re-bisect this. Only happens when I'm off my head on pain killers after surgery. 

But yes, the issue is a regression in commit 09c722873b2d378d2d155f5f1dd7d8f3fb2012e9.
("EMF/WMF: fix rendering of pen styles (dash, dot, dashdot, dashdotdot").

I've looked at how the latest version of Word on the Mac works, and it
turns out that the spacings for the PenStyle enumerations in the LogPen
objects for all the create pen EMF records are as follows:

* PS_DOT       - ■ □ ■ □ ■ □ ■ □ ■ □ ■
* PS_DASHDOT   - ■ ■ ■ □ ■ □ ■ ■ ■ □ ■
* PS_DASHDOTDOT- ■ ■ ■ □ ■ □ ■ □ ■ ■ ■

(where ■  is the actual filled in area, and □  is the space between the
filled in areas)

In other words, each dash fills in the space of three dots, and there
is the one dot worth of empty space between the dashes and dots. Each
"dot" has a width and height equal to the width specified in the pen.

So basically, we seem to be arbitrarily setting the dot, dash and
distance lengths arbitrarily, which were reasonable guesses but tended
to produce very odd lines at different zoom levels.

I've submitted a fix to gerrit here: 

https://gerrit.libreoffice.org/#/c/22886/
Comment 20 Chris Sherlock 2016-03-04 12:07:54 UTC
Created attachment 123224 [details]
QA test document

The following is a test document for QA.

There are two images, one of which is a test EMF file that we use for one of our EMF unit tests, the other is the image that has a graph that should have dashed grid lines. 

There is, unfortuately, still a bug but it's not related to this one - the bug is that there is actually a thin orange rectangle that encompasses the entire graph (shows up in Word for Mac on OS X, but not in LibreOffice). A new bug will need to be filed to sort this one out.
Comment 21 Commit Notification 2016-03-04 18:25:49 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

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

tdf#79679 vcl: dashed lines show as solid lines when importing EMF files

It will be available in 5.2.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 22 Chris Sherlock 2016-03-05 10:11:01 UTC
I've backported this fix into the 5.0 and 5.1 branches. Two gerrit commits awaiting review:

https://gerrit.libreoffice.org/#/c/22928/ - libreoffice-5-0
https://gerrit.libreoffice.org/#/c/22923/ - libreoffice-5-1
Comment 23 Commit Notification 2016-03-15 23:43:37 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "libreoffice-5-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a05f5238fc3811cf2c56f343a3015d5a58180677&h=libreoffice-5-1

tdf#79679 vcl: dashed lines show as solid lines when importing EMF files

It will be available in 5.1.2.

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 24 Commit Notification 2016-04-26 22:31:09 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e21606f3cce557e5eb84b2e7747c173edf526a55&h=libreoffice-5-0

tdf#79679 vcl: dashed lines show as solid lines when importing EMF files

It will be available in 5.0.7.

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 25 Commit Notification 2016-04-26 23:48:32 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "libreoffice-5-0-6":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d35d361774dc5ddfb9a1b2764b4914cdfac080ae&h=libreoffice-5-0-6

tdf#79679 vcl: dashed lines show as solid lines when importing EMF files

It will be available in 5.0.6.

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.