Bug Hunting Session
Bug 84254 - FILESAVE: Drawings corrupted on PPTX roundtrip
Summary: FILESAVE: Drawings corrupted on PPTX roundtrip
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
4.3.1.2 release
Hardware: Other All
: medium normal
Assignee: Andras Timar
URL:
Whiteboard: target:5.1.0 target:5.0.0.0.beta2
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-23 17:04 UTC by Luke
Modified: 2016-10-25 19:23 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
Drawing that gets corrupted on roundtrip (31.79 KB, application/vnd.ms-powerpoint.12)
2014-09-23 17:04 UTC, Luke
Details
Roundtrip generated with 4.4 (32.38 KB, application/vnd.ms-powerpoint.12)
2014-09-23 17:04 UTC, Luke
Details
Powerpoint Error message and comparison (95.78 KB, image/png)
2014-09-23 17:06 UTC, Luke
Details
oox drawing object corruption on LO 4331 export to OOXML and reopen (19.33 KB, image/png)
2014-10-18 19:31 UTC, V Stuart Foote
Details
In LO 4.5 PowerPoint graphics are now grey boxes (178.56 KB, image/png)
2015-03-15 01:43 UTC, Luke
Details
Another example of skewed/warped shape on round-trip (30.32 KB, application/vnd.ms-powerpoint.12)
2015-03-30 09:39 UTC, Luke
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Luke 2014-09-23 17:04:11 UTC
Created attachment 106751 [details]
Drawing that gets corrupted on roundtrip

Impress corrupts both the file and the resulting image when you save slides with drawings as PPTX.  

Steps to reproduce the bug:
1. Open SampleArtwork.pptx in Impress
2. Save as .pptx
3. Open in PowerPoint,OneDrive, or Impress 
4. Compare the documents. 

This bug was observed on LibreOffice v4.4 under Windows.
Comment 1 Luke 2014-09-23 17:04:49 UTC
Created attachment 106752 [details]
Roundtrip generated with 4.4
Comment 2 Luke 2014-09-23 17:06:24 UTC
Created attachment 106753 [details]
Powerpoint Error message and comparison
Comment 3 tommy27 2014-09-28 08:42:19 UTC
I confirm issue under Win7x64 using recent 4.4.0.0 alpha, 4.3.1.2 with same results as in screenshot

at least is better than 4.2.6.2 which shows a grey rectangle in place of the image, so some progress from 4.2.x has been achieved.

changed version field to 4.3.1.2 to reflect current state of corruption
Comment 4 tommy27 2014-09-28 08:43:49 UTC
by the way, no drawing corruption saving as .odp
Comment 5 V Stuart Foote 2014-10-18 19:31:35 UTC
Created attachment 108039 [details]
oox drawing object corruption on LO 4331 export to OOXML and reopen
Comment 6 Luke 2015-03-15 01:43:44 UTC
Created attachment 114095 [details]
In LO 4.5 PowerPoint graphics are now grey boxes

In LO 4.5, this issue has gotten worse. Instead of distorting the graphics, it now replaces them with grey boxes. We should bibisect this regression.
Comment 7 Matthew Francis 2015-03-30 06:12:02 UTC
So the fact that it used to work better is a regression, but of course the fact that it didn't used to work at all isn't - that's been true all the way back to LO 3.3.0, and OOo didn't support pptx at all.

So let's call this report a regression for now, and if we get to the state where the regression part is fixed but the image is as imperfect as before, we can consider opening a new bug to deal with that part.
Comment 8 Matthew Francis 2015-03-30 06:14:38 UTC
The regression part started in the 4.5 dbgutil bibisect repo at 
  2015-02-07: source-hash-3159f59e2c620de14d9570b69a802554a3968af8

It seems pretty likely that's going to be the below commit.
Addinc Cc: to timar74@gmail.com; Could you possibly take a look at this one? Thanks


commit b1751e6ed0fd6d6d26141e4405df92520e3c04cd
Author: Andras Timar <andras.timar@collabora.com>
Date:   Thu Feb 5 22:36:24 2015 +0100

    bnc#637947 improve DrawingML export of custom shapes
Comment 9 Andras Timar 2015-03-30 06:24:49 UTC
(In reply to Matthew Francis from comment #8)
> commit b1751e6ed0fd6d6d26141e4405df92520e3c04cd
> Author: Andras Timar <andras.timar@collabora.com>
> Date:   Thu Feb 5 22:36:24 2015 +0100
> 
>     bnc#637947 improve DrawingML export of custom shapes

This commit is in LibreOffice 4.5 only. It cannot cause problems in LibreOffice 4.4.
Comment 10 Matthew Francis 2015-03-30 06:33:37 UTC
OK, I'll do this the other way round then and raise a new bug
Comment 11 Matthew Francis 2015-03-30 06:43:10 UTC
The recent regression has been split onto bug 90338

Setting this one back to "not a regression"
Comment 12 Luke 2015-03-30 09:39:48 UTC
Created attachment 114460 [details]
Another example of skewed/warped shape on round-trip
Comment 13 Andras Timar 2015-05-21 13:34:34 UTC
(In reply to Luke from comment #12)
> Created attachment 114460 [details]
> Another example of skewed/warped shape on round-trip

This is a different bug, so opened a new bug for it: bug 91429. Let's keep this bug clean. In this bug we need to solve https://bugs.documentfoundation.org/attachment.cgi?id=106753 (in LibreOffice 4.4, because 5.0 is borken in a different way).
Comment 14 Andras Timar 2015-05-25 19:52:14 UTC
I started to work on this on the Cambridge Hackfest. Here are my findings:

1. 4.4 export: the problem is that the shape is put into a PolyPolygon, which contains only one Polygon. The WritePolyPolygon function in DrawingML export code uses the first point of each Polygon as a MoveTo command. In our case we have distinct shapes. We cannot model this shape as single Polygon where only the first point is mapped to MoveTo. 

2. 5.0 export: it uses the GetLineGeometry() function, which uses the custom shape engine to convert the shape into a B2DPolyPolygon. In this case we loose size information, all polygons are fit to the bounding box, thus we get the green rectangle.

I think we should get rid of PolyPolygon representation of shape, we loose information. Rather, we should export the shape directly from the XShape object. This approach seems to be working well, but I still have issues. For example what to write to w and h attributes to a:path element, when the shape does not have SubViewSize property.
Comment 15 Commit Notification 2015-05-27 07:19:53 UTC
Andras Timar committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=481c185e327cb83ffcb29657d5a354eae2c4a5f3

tdf#90338 tdf#84254 DrawingML export fix

It will be available in 5.1.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 16 Luke 2015-05-28 03:05:30 UTC
Andras,
Awesome job! All the Office 2013 SmartArt that was failing before is now round-tripping perfectly. This is a huge interoperability improvement and should probably be added to the release notes.

https://wiki.documentfoundation.org/WikiAction/history/ReleaseNotes/5.0

Thanks!


Verified fixed on
Version: 5.1.0.0.alpha1+
Build ID: be01d68420086fc36ecf26b5f597ba7c6b29b369
Comment 17 tommy27 2015-05-28 05:10:21 UTC
before adding it to 5.0 release notes it has to be backported to 5.0.x branch.
actually the fix is just in the 5.1.x codeline.

@Andras
any plans to do this?
Comment 18 Commit Notification 2015-05-28 07:09:22 UTC
Andras Timar committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

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

tdf#90338 tdf#84254 DrawingML export fix

It will be available in 5.0.0.0.beta2.

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.