Bug 126741 - Dash dot dot line style starts with dots
Summary: Dash dot dot line style starts with dots
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
5.2 all versions
Hardware: All All
: medium normal
Assignee: nd101
URL:
Whiteboard: target:6.4.0
Keywords:
Depends on:
Blocks: Shapes-Line
  Show dependency treegraph
 
Reported: 2019-08-07 06:05 UTC by nd101
Modified: 2019-08-30 07:09 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:
Regression By:


Attachments
PowerPoint document with single line with dash-dot-dot line style (31.68 KB, application/vnd.openxmlformats-officedocument.presentationml.presentation)
2019-08-07 06:06 UTC, nd101
Details
Impress dash dot dot line style (842 bytes, image/png)
2019-08-07 06:09 UTC, nd101
Details
MS PowerPoint dash dot dot line style (572 bytes, image/png)
2019-08-07 06:10 UTC, nd101
Details
In PowerPoint 365 available prstDash (29.19 KB, application/vnd.openxmlformats-officedocument.presentationml.presentation)
2019-08-07 19:02 UTC, Regina Henschel
Details
User defined line style with dash and dots (11.68 KB, application/vnd.oasis.opendocument.presentation)
2019-08-07 19:09 UTC, Regina Henschel
Details
Remarks on line styles and example files (174.18 KB, application/x-zip-compressed)
2019-08-11 18:22 UTC, Regina Henschel
Details
The remarks I have mentioned. (22.91 KB, application/vnd.oasis.opendocument.text)
2019-08-11 18:49 UTC, Regina Henschel
Details
round trip test.zip (72.84 KB, application/x-zip-compressed)
2019-08-12 06:11 UTC, nd101
Details
All prstDash of OOXML (34.29 KB, application/vnd.openxmlformats-officedocument.presentationml.presentation)
2019-08-19 10:11 UTC, Regina Henschel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description nd101 2019-08-07 06:05:54 UTC
Description:
For line style "dash dot dot", on MS Office, it starts with a dash. But with Impress, it starts with two dots. 

Steps to Reproduce:
1.Open the attached PowerPoint document
2.Compare it with MS PowerPoint
3.See the difference

Actual Results:
Line starts with two dots

Expected Results:
Line starts with dash


Reproducible: Always


User Profile Reset: No



Additional Info:
Comment 1 nd101 2019-08-07 06:06:57 UTC
Created attachment 153187 [details]
PowerPoint document with single line with dash-dot-dot line style
Comment 2 nd101 2019-08-07 06:09:38 UTC
Created attachment 153188 [details]
Impress dash dot dot line style
Comment 3 nd101 2019-08-07 06:10:10 UTC
Created attachment 153189 [details]
MS PowerPoint dash dot dot line style
Comment 4 Commit Notification 2019-08-07 09:06:50 UTC
nd101 committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/0b2e052238e87faf5b76fe89ed1e10b20c5a3eaa%5E%21

tdf#126741 - fix dash dot dot line style display

It will be available in 6.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 5 Regina Henschel 2019-08-07 18:55:50 UTC
Have you tested it? It doesn't work for me: Import of pptx creates "solid" for me, and with our own dash styles it has now the wrong order. I have not tested MS ppt import and export.

I think it's the wrong place at all. There not only ooxml styles are affected but all styles. I think the adaptation to our model should be done in oox, directly during import or export.

The identifiers in the code and in the API are a problem. Neither ODF nor OOXML know dots and dashes in the file format. In ODF there are only "dots1" and "dots2" with number and length and "distance" with length. OOXML has only several <ds> elements each with line length and following space length in custDash and some prstDash variants. So there exists no difference between dots and dashes.

The struct LineDash of the API works so, that "Dot" corresponds to "dots1" and "Dash" corresponds to "dots2", regardless of the naming in the UI in the Line dialog.

I have tried to adjust the mapping in oox (see below), but this is not the complete solution. This will give the correct order for the import, but also the export would have to be changed.


diff --git a/oox/source/drawingml/lineproperties.cxx b/oox/source/drawingml/lineproperties.cxx
index 2b5de7a1f8c0..e6aa1308dc3c 100644
--- a/oox/source/drawingml/lineproperties.cxx
+++ b/oox/source/drawingml/lineproperties.cxx
@@ -61,17 +61,17 @@ void lclConvertPresetDash(LineDash& orLineDash, sal_Int32 nPresetDash, sal_Int32
     switch( nPresetDash )
     {
         case XML_dot:           lclSetDashData( orLineDash, 1, 1, 0, 0, 3 );    break;
-        case XML_dash:          lclSetDashData( orLineDash, 0, 0, 1, 4, 3 );    break;
-        case XML_dashDot:       lclSetDashData( orLineDash, 1, 1, 1, 4, 3 );    break;
+        case XML_dash:          lclSetDashData( orLineDash, 1, 4, 0, 0, 3 );    break;
+        case XML_dashDot:       lclSetDashData( orLineDash, 1, 4, 1, 1, 3 );    break;
 
-        case XML_lgDash:        lclSetDashData( orLineDash, 0, 0, 1, 8, 3 );    break;
-        case XML_lgDashDot:     lclSetDashData( orLineDash, 1, 1, 1, 8, 3 );    break;
-        case XML_lgDashDotDot:  lclSetDashData( orLineDash, 2, 1, 1, 8, 3 );    break;
+        case XML_lgDash:        lclSetDashData( orLineDash, 1, 8, 0, 0, 3 );    break;
+        case XML_lgDashDot:     lclSetDashData( orLineDash, 1, 8, 1, 1, 3 );    break;
+        case XML_lgDashDotDot:  lclSetDashData( orLineDash, 1, 8, 2, 0, 3 );    break;
 
         case XML_sysDot:        lclSetDashData( orLineDash, 1, 1, 0, 0, 1 );    break;
-        case XML_sysDash:       lclSetDashData( orLineDash, 0, 0, 1, 3, 1 );    break;
-        case XML_sysDashDot:    lclSetDashData( orLineDash, 1, 1, 1, 3, 1 );    break;
-        case XML_sysDashDotDot: lclSetDashData( orLineDash, 2, 1, 1, 3, 1 );    break;
+        case XML_sysDash:       lclSetDashData( orLineDash, 1, 3, 0, 0, 1 );    break;
+        case XML_sysDashDot:    lclSetDashData( orLineDash, 1, 3, 1, 1, 1 );    break;
+        case XML_sysDashDotDot: lclSetDashData( orLineDash, 1, 3, 2, 1, 1 );    break;
 
         default:
             OSL_FAIL( "lclConvertPresetDash - unsupported preset dash" );
Comment 6 Regina Henschel 2019-08-07 19:02:49 UTC
Created attachment 153206 [details]
In PowerPoint 365 available prstDash

PowerPoint does not support all prstDash variants, which are defined in OOXML. I have not fould a <xml> description of the prstDash variants. But you can count the length, when you copy the illustration from section "L.4.8.5.2 Line Dash Properties" in ISO/IEC 29500-1:2016(E) and display it in monospaced font.
Comment 7 Regina Henschel 2019-08-07 19:09:27 UTC
Created attachment 153207 [details]
User defined line style with dash and dots

This document contains two user defined line styles. One starts with "dashes", the other with "dots". You can use it to examine, how these styles are shown in the API and to test, whether they are still in the correct order after your fix.

Of cause an export to pptx should generate the corresponding custDash elements.
Comment 8 nd101 2019-08-08 07:59:43 UTC
Yes, I have done some test. With the document "In PowerPoint 365 available prstDash" you provided, set the zoom level 100%, the latest build with the patch did fix the problem and make the lines look visually almost indistinguishable from what MSO shows. But for the user defined styles, it fails to do the job, and it shows 3 dots instead of two dots. We may need to look into what you have suggested to make further changes in the imp/exp area. I hope this is still considered a valid fix, as the MSO document default dash-dot-dot line styles are much more predominant than user defined edge cases. We can undo the patch if a true fix come along to cover all cases.
Comment 9 Regina Henschel 2019-08-08 10:41:38 UTC
(In reply to nd101 from comment #8)
> Yes, I have done some test. With the document "In PowerPoint 365 available
> prstDash" you provided, set the zoom level 100%, the latest build with the
> patch did fix the problem and make the lines look visually almost
> indistinguishable from what MSO shows.

There might be an error in my build. Unfortunately there is no up-to-date of daily master available.

 But for the user defined styles, it
> fails to do the job, and it shows 3 dots instead of two dots. We may need to
> look into what you have suggested to make further changes in the imp/exp
> area. I hope this is still considered a valid fix, as the MSO document
> default dash-dot-dot line styles are much more predominant than user defined
> edge cases. We can undo the patch if a true fix come along to cover all
> cases.

I disagree, that OOXML documents are more important. ODF is the native format of LibreOffice and that should be treated correctly. LibreOffice is not a clone of MS Office.
Comment 10 Commit Notification 2019-08-08 14:58:41 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/8387a6db641b29e6ff3c2f4cdc4688f538cbe35f%5E%21

Revert "tdf#126741 - fix dash dot dot line style display"

It will be available in 6.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 nd101 2019-08-09 01:21:09 UTC
> I have tried to adjust the mapping in oox (see below), but this is not the
> complete solution. This will give the correct order for the import, but also
> the export would have to be changed.
> 

Your approach seems more promising to get to the bottom of the problem. Do you feel like take over the bug? You seems to be half way there. Or give me some pointers on the export part, I can give it a try.

Users around here care a lot of interoperability issue due to the fact that MSO is predominant in the marketplace.
Comment 12 Regina Henschel 2019-08-09 13:21:39 UTC
(In reply to nd101 from comment #11)
> 
> Your approach seems more promising to get to the bottom of the problem. Do
> you feel like take over the bug? You seems to be half way there.

No, I will not work on this bug.

 Or give me
> some pointers on the export part, I can give it a try.

Please try IRC and dev-mailing list to get assistant. My answer might not be qualified enough. I work on LibreOffice in my spare time and not as job.
https://lists.freedesktop.org/mailman/listinfo/libreoffice
irc://chat.freenode.net/libreoffice-dev

Searching for XML_ds in path oox leads me to https://opengrok.libreoffice.org/xref/core/oox/source/export/drawingml.cxx?r=5c19af48#956

XML_ds corresponds to the element <a:ds>, that contains one dash description in OOXML. And oox is the module, which has most parts of the OOXML import/export code. It is worth to read the README parts of the modules.

> 
> Users around here care a lot of interoperability issue due to the fact that
> MSO is predominant in the marketplace.

And they do not realize, that some things cannot be written in OOXML format but in ODF and the other way round.
Comment 13 Regina Henschel 2019-08-11 18:22:55 UTC
Created attachment 153301 [details]
Remarks on line styles and example files

The zip-container contains some remarks on line styles, how they are realized in file format and API, and remarks on interoperability with MS Office. And it contains files for testing.
Comment 14 Regina Henschel 2019-08-11 18:35:34 UTC
@nd101,  Bartosz is working in the same area, so please be aware of his work.
Comment 15 Regina Henschel 2019-08-11 18:49:16 UTC
Created attachment 153302 [details]
The remarks I have mentioned.

I have forgotten to include the text file into the package. Here it is.
Comment 16 nd101 2019-08-12 06:10:17 UTC
void lclConvertPresetDash(LineDash& orLineDash, sal_Int32 nPresetDash, sal_Int32 nLineWidth)
{
    switch( nPresetDash )
    {
        case XML_dot:           lclSetDashData( orLineDash, 1, 1, 0, 0, 3 );    break;
        case XML_dash:          lclSetDashData( orLineDash, 1, 4, 0, 0, 3 );    break;
        case XML_dashDot:       lclSetDashData( orLineDash, 1, 4, 1, 1, 3 );    break;

        case XML_lgDash:        lclSetDashData( orLineDash, 1, 8, 0, 0, 3 );    break;
        case XML_lgDashDot:     lclSetDashData( orLineDash, 1, 8, 1, 1, 3 );    break;
        case XML_lgDashDotDot:  lclSetDashData( orLineDash, 1, 8, 2, 0, 3 );    break;

        case XML_sysDot:        lclSetDashData( orLineDash, 1, 1, 0, 0, 1 );    break;
        case XML_sysDash:       lclSetDashData( orLineDash, 1, 3, 0, 0, 1 );    break;
        case XML_sysDashDot:    lclSetDashData( orLineDash, 1, 3, 1, 1, 1 );    break;
        case XML_sysDashDotDot: lclSetDashData( orLineDash, 1, 3, 2, 1, 1 );    break;

        default:
            OSL_FAIL( "lclConvertPresetDash - unsupported preset dash" );
            lclSetDashData( orLineDash, 0, 0, 1, 4, 3 );
==============================

I made the change as you suggest in the comment. I did fix the import problem. However, with the attached 1.pptx document, it failed in the following test:

1) Before change, open 1.pptx, save it as 1.63.odp, open 1.63.odp, save it as 1.63.pptx, and it looks the same as 1.pptx

2) After the change, open 1.pptx, save it as 1.lo.odp, open 1.lo.odp, save it as 1.lo.pptx, and the dashdotdot line becomes a solit line.

I make the change with latest code in git repo. I have attached the test results.
Comment 17 nd101 2019-08-12 06:11:19 UTC
Created attachment 153317 [details]
round trip test.zip
Comment 18 nd101 2019-08-12 06:50:02 UTC
I looked into the generated odp files, and content.xml has the difference:

BEFORE CHANGE: draw:stroke-dash="Dashed_20__28_var_29__20_10"
AFTER CHANGE: draw:stroke-dash="Dashed_20__28_var_29__20_4"
Comment 19 Regina Henschel 2019-08-12 15:00:52 UTC
I have said, that the export to pptx needs to be adapted. The export is in file DrawingML::WriteOutline in file /core/oox/source/export/drawingml.cxx.

The example 1.pptx has <prstDash val="lgDashDotDot">. With current situation is becomes Dots=2, DotsLen=35, Dashes=1, DashLen=280, Distance=105 in the model and in .odp file. Length values are in 1/100mm. 35 is ersatz value for to small line width. So with relative values this would be Dots=2 DotsLen=100%, Dashes=1, DashLen=800%, Distance=300%.
If you now look at line #894 in drawingml.cxx you will find, that exactly this pattern is used to detect, that prstDash scheme "lgDashDotDot" has to be written.

If now on import the pattern is changed, then of cause the pattern which is used in export to detect the preset schema has to be changed too.

I don't know, whether this will already be sufficient, or wether other places need to be adapted in addition.
Comment 20 nd101 2019-08-14 08:17:43 UTC
Change submitted, with test case and it works.

https://gerrit.libreoffice.org/#/c/77443
Comment 21 Regina Henschel 2019-08-15 10:58:50 UTC
A description of dashing in VML can be found at https://docs.microsoft.com/en-us/windows/win32/vml/msdn-online-vml-dashstyle-attribute.
Comment 22 Regina Henschel 2019-08-15 12:55:25 UTC
Current values written by Word 365 differ from that description. I do not have a Word from that time (I think around Word 2003) to look what was written.
Comment 23 Regina Henschel 2019-08-19 10:11:17 UTC
Created attachment 153501 [details]
All prstDash of OOXML

The document contains all prstDash defined in OOXML on the first slide. The second slide contains a screenshot. Load with immediate save must keep all prstDash. Non of them may become a custDash. You cannot see it in the visual appearance, but you have to unpack the resaved file and look into slide1.xml.
Comment 24 nd101 2019-08-20 03:35:10 UTC
@Regina

With attached document, 
All prstDash of OOXML slide1.xml examine prstDash values. 
Build LO with submitted patch, open "All prstDash of OOXML" and save it as test.pptx and extract slide1.xml from the pptx file. Compare them with prstDash values with the ones found in All prstDash of OOXML slide1.xml, they are identical. 

It pass the preservation test.
Comment 25 Commit Notification 2019-08-23 11:20:59 UTC
nd101 committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/e2b16c5660a8f3f72c1d1ba74ce565e8628b3dbb%5E%21

tdf#126741 - fix dash dot dot line style import problem, for pptx

It will be available in 6.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.