Bug Hunting Session
Bug 91999 - FILESAVE: Shapes Rotated 180 Moved on Round-Trip
Summary: FILESAVE: Shapes Rotated 180 Moved on Round-Trip
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: filters and storage (show other bugs)
Version:
(earliest affected)
4.2.8.2 release
Hardware: Other All
: medium normal
Assignee: Justin L
URL:
Whiteboard: target:6.2.0
Keywords: bisected, filter:pptx, filter:xlsx, regression
: 110424 115603 119000 (view as bug list)
Depends on:
Blocks: OOXML-Shapes PPTX XLSX 113711
  Show dependency treegraph
 
Reported: 2015-06-11 05:28 UTC by Luke
Modified: 2018-09-09 21:29 UTC (History)
10 users (show)

See Also:
Crash report or crash signature:


Attachments
Example file with the arrow shape rotated 180 deg (29.03 KB, application/vnd.openxmlformats-officedocument.presentationml.presentation)
2015-06-11 05:28 UTC, Luke
Details
Exporting native ODP file to PPTX results in same issue (10.66 KB, application/vnd.oasis.opendocument.presentation)
2015-06-11 06:17 UTC, Luke
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Luke 2015-06-11 05:28:26 UTC
Created attachment 116458 [details]
Example file with the arrow shape rotated 180 deg

Steps to reproduce:
1. Create 2 shapes in MS Office
2. Rotate one of them 180 deg
3. Open in LibreOffice
4. Save 
5. Open the Roundtrip file

Note the rotated shape has been moved relative to the other shape
Comment 1 Luke 2015-06-11 06:17:04 UTC
Created attachment 116459 [details]
Exporting native ODP file to PPTX results in same issue
Comment 2 Buovjaga 2015-06-12 10:06:49 UTC
(In reply to Luke from comment #0)
> Created attachment 116458 [details]
> Example file with the arrow shape rotated 180 deg
> 
> Steps to reproduce:
> 1. Create 2 shapes in MS Office
> 2. Rotate one of them 180 deg
> 3. Open in LibreOffice
> 4. Save 
> 5. Open the Roundtrip file
> 
> Note the rotated shape has been moved relative to the other shape

Reproduced the moving by saving & reloading attachment 116458 [details].

Win 7 Pro 64-bit Version: 5.1.0.0.alpha1+
Build ID: 5fc0cbbc1254223fedf0f78c5e7539219b228697
TinderBox: Win-x86@39, Branch:master, Time: 2015-06-11_04:30:51
Locale: fi-FI (fi_FI)
Comment 3 Luke 2015-06-14 16:21:45 UTC
Adding Andras whose been improving the DrawingML export filters.
Comment 4 Luke 2017-08-02 16:02:53 UTC
*** Bug 110424 has been marked as a duplicate of this bug. ***
Comment 5 Luke 2017-08-02 16:38:04 UTC
Patch on gerrit here:

https://gerrit.libreoffice.org/#/c/40689/

Miklos could you please take a look at this?
Comment 6 Luke 2017-08-02 21:01:44 UTC
The build is failing in Linux because sw/qa/extras/ooxmlexport/data/dml-textshape.docx has a connector that is rotated 180 deg. When I ungroup that connector, it is not shifted on roundtrip. I made some more test cases and found that my patch fails when a group of shapes have members that are rotated 180 deg. 

Should the 180 deg check only be applied to members of a group? Or is there something else going on here?
Comment 7 Luke 2017-08-04 04:00:37 UTC
Version: 4.2.8.2 correctly exports the test file, while 4.3.7.2 suffers from this regression. Marking as bisected since reverting https://cgit.freedesktop.org/libreoffice/core/commit/?id=60635557
fixes this issue.

Miklos, 
I assume the issue is the check for 180 should only apply to members of a group. Any code pointers?
Comment 8 Miklos Vajna 2017-08-07 07:14:03 UTC
You already have access to the SdrObject representing the shape in question. Try using SdrObject::GetUpGroup() to test if it's a standalone shape or it's inside a group.
Comment 9 Tamás Zolnai 2017-09-26 12:22:50 UTC
Hi Luke,

Are you still working on this issue?
Comment 10 Luke 2017-09-26 13:42:44 UTC
Tamás Zolnai,
Not at the moment, so it would be great if you could take a stab at it. 

Using pShape->GetUpGroup() to test group membership didn't produce the correct results, so there's something wrong with my analysis or my code.
Comment 11 Paul Trojahn 2017-11-06 22:56:18 UTC
Seems to work on master. Might be related to bug 113263?

Version: 6.0.0.0.alpha1+
Build ID: 82957288eaf6454f25b91f2e624b44c692f197cc
CPU threads: 8; OS: Linux 4.9; UI render: default; VCL: gtk3; 
Locale: de-DE (de_DE.UTF-8); Calc: group
Comment 12 Luke 2018-07-11 08:19:10 UTC
Justin,
https://cgit.freedesktop.org/libreoffice/core/commit/?id=ee45d881
Has caused attachment 116458 [details] to fail to save correctly again. 


Should I file a new report or you want to track it here?
Comment 13 Justin L 2018-07-11 12:29:27 UTC
NEW: based on comment 12. Looks like it was initially fixed via bug 113263 instead of the suggestion in comment 11.

So, sometimes 180 rotated non-grouped customshapes need to be swapped, and sometimes not. Bug 114845's example has bFlipV - that's the only significant difference I can see so far. There are no 180, bFlipH examples in sd/qa.

Someone else create more regressions, not me...
Comment 14 Luke 2018-07-12 16:59:30 UTC
I just checked and I can confirm that this bug was originally fixed with:

https://cgit.freedesktop.org/libreoffice/core/commit/?id=9ae1e094
PPTX export: correct position of rotated groups

It regressed again with 

https://cgit.freedesktop.org/libreoffice/core/commit/?id=ee45d881
tdf#114845 sd: only shift rotated group items
Comment 15 Justin L 2018-07-31 19:13:03 UTC
*** Bug 119000 has been marked as a duplicate of this bug. ***
Comment 16 Justin L 2018-08-01 07:10:29 UTC
I missed the note in comment 7 that this is clearly a regression from 2014-01-23.

I looked at the import code for inspiration (oox/source/drawingxml/shape.cxx Shape::createAndInsert). I found nothing in there to suggest that there is anything special about rotation == 180. So I think Luke was correct in his comment 5 suggestion.

I promised myself that I would not do any more shape-related commits, but this saga already has my name in it, so I can't resist another try...

Luke, the testing that you did with grouping - was that all with .docx?  It looks like this rotation block should be avoided if DocumentType == DOCX (similar qualifications found elsewhere in the code). But I should test with grouped shapes, custom shapes, and line shapes to see if it is only a subset of DOCX.
Comment 17 Justin L 2018-08-01 16:23:18 UTC
proposed fix: https://gerrit.libreoffice.org/58434

I tested with with most of the example documents from this bug, bug 114845 and duplicates, bug 113711, and bug 113263 with duplicates. There are also lots of existing ooxmlexport examples with rotation, some specifically testing position, and this passes all tests.
Comment 18 Commit Notification 2018-08-10 14:19:12 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

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

tdf#91999 export/drawingml: shape rotate 180 is not special

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 19 Xisco Faulí 2018-08-15 10:49:03 UTC
Verified in

Version: 6.2.0.0.alpha0+
Build ID: 18e20676024baecaf5719139f80f053f5f1e784a
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: threaded

@Justin Luth, Thanks for fixing this!!
Comment 20 Aron Budea 2018-09-09 21:29:07 UTC
*** Bug 115603 has been marked as a duplicate of this bug. ***