Bug 73547 - FILESAVE: OOXML: Wrong a:custDash value after roundtrip
Summary: FILESAVE: OOXML: Wrong a:custDash value after roundtrip
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: filters and storage (show other bugs)
Version:
(earliest affected)
4.3.0.0.alpha0+ Master
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.2.0
Keywords: filter:docx
Depends on:
Blocks: DOCX-Corrupted
  Show dependency treegraph
 
Reported: 2014-01-13 10:16 UTC by Jacobo Aragunde Pérez
Modified: 2019-09-07 20:23 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
Test case (7.14 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2014-01-13 10:16 UTC, Jacobo Aragunde Pérez
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jacobo Aragunde Pérez 2014-01-13 10:16:55 UTC
Created attachment 91938 [details]
Test case

Steps to reproduce:

1. Open the attached file.
2. Save it again as a .docx file.
3. Try to open the saved file with MS Office. It will fail because of a problem in the content.

The problem in the content seems to be located at the tag <a:ds> inside <a:custDash>. This is the difference between the values in the original document and in the exported one:

Original:
<a:custDash>
  <a:ds d="105000" sp="35000"/>
</a:custDash>

Exported:
<a:custDash>
  <a:ds d="3675000000" sp="1225000000"/>
</a:custDash>
Comment 1 Jorendc 2014-03-19 17:00:37 UTC
I can confirm a corruption, tested using Windows 8.1 with LibreOffice Version: 4.3.0.0.alpha0+
Build ID: 90dd4320de6ace24e464979630a2c9fbab35f64b
TinderBox: Win-x86@39, Branch:master, Time: 2014-03-19_00:22:53

Looks like both values are multiplied wrongly with 3500.

Kind regards,
Joren
Comment 2 Joel Madero 2015-05-02 15:41:15 UTC Comment hidden (obsolete)
Comment 3 Jacobo Aragunde Pérez 2015-05-04 09:02:33 UTC
Still reproducible on:
Version: 5.0.0.0.alpha1+
Build ID: a21a0b6dceaf965673ae601318e77991919c8f6a

The exported file has slightly changed, this is the diff of the files before/after roundtrip:
  <                             <a:ds d="105000" sp="35000" />
  ---
  >                             <a:ds d="100000" sp="100000" />
Comment 4 Jorendc 2015-05-06 19:37:08 UTC
(In reply to Jacobo Aragunde Pérez from comment #3)
> The exported file has slightly changed, this is the diff of the files
> before/after roundtrip:
>   <                             <a:ds d="105000" sp="35000" />
>   ---
>   >                             <a:ds d="100000" sp="100000" />

This is due patch http://cgit.freedesktop.org/libreoffice/core/commit/?id=2211a67cc5e577f8abdcc96c9c63865be5fb988d

We now always read or convert the 'd' and 'sp' to 1000th of a %.
As you can see here: http://opengrok.libreoffice.org/xref/core/oox/source/drawingml/lineproperties.cxx#114

+        nConvertedLen      = aIt->first  / 1000 / 100;
+        nConvertedDistance = aIt->second / 1000 / 100;

we not only divide by 1000, also we now divide by 100 extra.

105000 / 1000 / 100 = rounded to 1
35000 / 1000 / 100 = rounded to 0, but maxed later to 1

Those values are multiplied by the nLineWidth at http://opengrok.libreoffice.org/xref/core/oox/source/drawingml/lineproperties.cxx#387 which is maxed to 35 (first it was 32).

aLineDash.DotLen = 1*35 now
aLineDash.Distance = 1*35 now

At export:
+ XML_d , write1000thOfAPercent( aLineDash.DashLen  > 0 ? aLineDash.DashLen  / nLineWidth * 100 : 100 ),
+ XML_sp, write1000thOfAPercent( aLineDash.Distance > 0 ? aLineDash.Distance / nLineWidth * 100 : 100 ),

d = sp = (35 / 32 * 100) * 1000 = 100000

Following patch exports the right values, but countereffect is that now the line is not shown correctly in LibreOffice (length and distance is factor 100 bigger):

diff --git a/oox/source/drawingml/lineproperties.cxx b/oox/source/drawingml/lineproperties.cxx
index efa1ab8..281fa57 100644
--- a/oox/source/drawingml/lineproperties.cxx
+++ b/oox/source/drawingml/lineproperties.cxx
@@ -111,8 +111,8 @@ void lclConvertCustomDash( LineDash& orLineDash, const LineProperties::DashStopV
     for( LineProperties::DashStopVector::const_iterator aIt = rCustomDash.begin(), aEnd = rCustomDash.end(); aIt != aEnd; ++aIt )
     {
         // Get from "1000th of percent" ==> percent ==> multiplier
-        nConvertedLen      = aIt->first  / 1000 / 100;
-        nConvertedDistance = aIt->second / 1000 / 100;
+        nConvertedLen      = aIt->first  / 1000;
+        nConvertedDistance = aIt->second / 1000;
 
         // Check if it is a dot (100% = dot)
         if( nConvertedLen == 1 )
diff --git a/oox/source/export/drawingml.cxx b/oox/source/export/drawingml.cxx
index 484c90b..3c8b1ee 100644
--- a/oox/source/export/drawingml.cxx
+++ b/oox/source/export/drawingml.cxx
@@ -692,8 +692,8 @@ void DrawingML::WriteOutline( Reference<XPropertySet> rXPropSet )
                     for( i = 0; i < aLineDash.Dashes; i ++ )
                     {
                         mpFS->singleElementNS( XML_a , XML_ds,
-                                               XML_d , write1000thOfAPercent( aLineDash.DashLen  > 0 ? aLineDash.DashLen  / nLineWidth * 100 :
-                                               XML_sp, write1000thOfAPercent( aLineDash.Distance > 0 ? aLineDash.Distance / nLineWidth * 100 :
+                                               XML_d , write1000thOfAPercent( aLineDash.DashLen  > 0 ? aLineDash.DashLen  / ::std::max< sal_Int32 >( nLineWidth, 35 ) : 100 ),
+                                               XML_sp, write1000thOfAPercent( aLineDash.Distance > 0 ? aLineDash.Distance / ::std::max< sal_Int32 >( nLineWidth, 35 ) : 100 ),
                                                FSEND );
                     }
                 }


How to get around this incorrect display in LibreOffice?
(I can't find the correct location where these values are handled in the drawing-backend of LibreOffice).
I see 2 options:
1) devide or factor everything related to this DashLen with 100, so it'll be displayed correctly
2) store these very tiny dots or dashes whom are rounded incorrectly in a kind of grab-bag and call them on export again.

Can someone please help me out and show me the right direction :)?

Thanks!
Joren
Comment 5 Xisco Faulí 2016-09-15 12:01:20 UTC
After the nice comment 4 posted by Jorendc, I guess it deserve some attention, adding 'needAdvice'
Comment 6 Xisco Faulí 2016-09-19 15:29:50 UTC Comment hidden (obsolete)
Comment 7 Xisco Faulí 2017-09-29 08:48:58 UTC Comment hidden (obsolete)
Comment 8 Jacobo Aragunde Pérez 2017-10-11 17:18:31 UTC
Exported values are not terribly wrong now:

Original:
  <a:custDash>
    <a:ds d="105000" sp="35000"/>
  </a:custDash>

Exported:
  <a:custDash>
    <a:ds d="100000" sp="100000" />
  </a:custDash>

They are still different from original values in the document, though. The bug is still valid.

Version: 5.3.6.1
Build ID: 5.3.6.1-5.fc26
CPU Threads: 4; OS Version: Linux 4.12; UI Render: default; VCL: gtk3; Layout Engine: new; 
Locale: es-ES (en_US.UTF-8); Calc: group
Comment 9 Xisco Faulí 2017-10-31 11:59:16 UTC
Corrupted message no longer displayed in MSO 2010 after a RT in

Version: 6.0.0.0.alpha1+
Build ID: 60a03d97bc35c02cb1eff0e4a02b6f37fd1a6a34
CPU threads: 4; OS: Linux 4.10; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group

@Jacobo, I guess it's not fixed yet though, isn't it?
Comment 10 Jacobo Aragunde Pérez 2017-10-31 12:29:37 UTC
@Xisco the situation has improved, there is no corruption message (I think that was caused by values out of range for a:ds attributes) but the original value is still not preserved. The bug is less worrying now, but it should remain open.
Comment 11 Commit Notification 2018-08-24 13:07:46 UTC
Mark Hung committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=34dff8df7b9fc46fed6e6ffbaa8b6847ad6ed1b1

tdf#73547 fix ooxml export / import custom dashes.

It will be available in 6.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 12 Xisco Faulí 2018-10-11 10:15:30 UTC
A polite ping to Mark Hung:
Is this bug fixed? if so, could you please close it as RESOLVED FIXED ? Otherwise, Could you please explain what's missing?
Thanks
Comment 13 Regina Henschel 2019-09-07 20:23:34 UTC
Fixed with commit https://cgit.freedesktop.org/libreoffice/core/commit/?id=57c9bdab377a00649299d1a4c9ed2f9e5e03b84e

Fix is available in Version: 6.4.0.0.alpha0+ (x64)
Build ID: 61b757f31f25cda6d595f64226181c7a82ce8d7f
CPU threads: 8; OS: Windows 10.0; UI render: GL; VCL: win; 
TinderBox: Win-x86_64@62-TDF, Branch:master, Time: 2019-09-07_17:48:35
Locale: en-US (en_US); UI-Language: en-US
Calc: CL