Bug 104290 - FILEOPEN: XSLX - Shape text incorrectly turned
Summary: FILEOPEN: XSLX - Shape text incorrectly turned
Status: RESOLVED DUPLICATE of bug 83593
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
5.2.3.3 release
Hardware: All All
: medium normal
Assignee: Balázs Regényi
URL:
Whiteboard:
Keywords: filter:xlsx
Depends on:
Blocks: OOXML-Shapes XLSX
  Show dependency treegraph
 
Reported: 2016-11-30 19:36 UTC by Roeland
Modified: 2020-05-29 15:19 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
the original document (45.45 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2016-11-30 19:36 UTC, Roeland
Details
outcomes (20.86 KB, image/png)
2016-11-30 19:37 UTC, Roeland
Details
Screenshot of shape properties in Excel (17.06 KB, image/png)
2017-12-20 21:36 UTC, Regina Henschel
Details
Screenshot of text properties in Excel (30.06 KB, image/png)
2017-12-20 21:37 UTC, Regina Henschel
Details
Example with an arrow (9.36 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2017-12-20 21:50 UTC, Regina Henschel
Details
bad size for special shape rotation angle; anchor type "oneCell" (23.89 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2017-12-26 21:20 UTC, Regina Henschel
Details
bad size for special shape rotation angle; anchor type "twoCell" (23.88 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2017-12-26 21:23 UTC, Regina Henschel
Details
Source document (16.33 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2018-01-31 23:36 UTC, Regina Henschel
Details
source document opened in LO with applied patch and resaved as ods (13.26 KB, application/vnd.oasis.opendocument.spreadsheet)
2018-01-31 23:37 UTC, Regina Henschel
Details
Example, where the "upright" attribute is used. (14.99 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2018-02-05 21:54 UTC, Regina Henschel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Roeland 2016-11-30 19:36:34 UTC
Created attachment 129158 [details]
the original document

When I open the attached document in Excel, the text "problem" is rendered horizontally. When opened in Calc this text has turned vertically.

I also attached a png file to compare both outcomes.
Comment 1 Roeland 2016-11-30 19:37:05 UTC
Created attachment 129159 [details]
outcomes
Comment 2 Telesto 2016-12-01 14:11:19 UTC
Confirming with:
Version: 5.4.0.0.alpha0+
Build ID: 4130c8def811d1dcc87eacaa8ae48ba02738a790
CPU Threads: 4; OS Version: Windows 6.19; UI Render: default; 
TinderBox: Win-x86@42, Branch:master, Time: 2016-11-29_01:03:18
Locale: nl-NL (nl_NL); Calc: CL
Comment 3 Regina Henschel 2017-12-14 16:39:59 UTC
It seems to be the same problem as in bug 76456.
Comment 4 Regina Henschel 2017-12-14 17:06:13 UTC
I'm not sure about your proposal in https://gerrit.libreoffice.org/#/c/46271/1. The problem is not the text rotation, but the missing rotation of the shape. Please test your solution with the attachment of bug 76456 too.
Comment 5 Regina Henschel 2017-12-20 21:36:47 UTC
Created attachment 138556 [details]
Screenshot of shape properties in Excel
Comment 6 Regina Henschel 2017-12-20 21:37:24 UTC
Created attachment 138557 [details]
Screenshot of text properties in Excel
Comment 7 Regina Henschel 2017-12-20 21:50:01 UTC
Created attachment 138558 [details]
Example with an arrow

Please have a look at the screenshots. The rectangle has a shape rotation of 90° and a text rotation of 270° in Excel. Together it looks at if the text has no rotation.
With your patch applied, the shape has a rotation of 0°. That is wrong.

To make the problem obvious, you have to use not a rectangular shape, but a non-symmetric shape e.g. "Down Arrow". Rotate it by 90° and the text by 270° in Excel as in the file the original document, done in attached document

The open it in LO with your patch applied. The shape orientation is wrong.

Open it in LO without your patch applied and now correct the missing rotation, then the text is automatically correct.

The error is not in the text rotation, but in the missing shape rotation.
Comment 8 Regina Henschel 2017-12-26 21:20:24 UTC
Created attachment 138667 [details]
bad size for special shape rotation angle; anchor type "oneCell"

I have removed
259                  // Rotation is decided by orientation of shape determined
260                  // by the anchor position given by 'editAs="oneCell"'
261                  if ( mxAnchor->getEditAs() != ShapeAnchor::ANCHOR_ONECELL )
262                          mxShape->setRotation(0);
in /core/sc/source/filter/oox/drawingfragment.cxx and _not_ applied the patch from https://gerrit.libreoffice.org/#/c/46271/1

Then the rotation of the shape and the rotation of the text is correct. That had been introduced 30.May 2012 by Noel Power for bnc#762542. I don't know what bug it fixes and don't know what side-effects such removal would have. But it shows, that the problem is indeed the shape rotation and not the text rotation.

In addition I see this problem: If 45°<=shape rotation angle <=135° or -135°<=shape rotation angle <=-45° (modulo 360°), then width and height of the shape are exchanged (width and height of the not-rotated shape). For the other angle values the import is correct.

I have not tested grouped shapes.
Comment 9 Regina Henschel 2017-12-26 21:23:04 UTC
Created attachment 138668 [details]
bad size for special shape rotation angle; anchor type "twoCell"

Compare the "oneCell" and the "twoCell" (default) case. This shows, that there is an error with the mentioned lines.
Comment 10 Serdar Oktay TUNÇ 2018-01-31 18:14:43 UTC
I send a patch for just text rotation. In this bug report its only about text rotation inside shape and its working fine by me. For 0,90,180,270 degrees text size is true but other degrees text size is not correct because of not rotated shape size lo tries to fit rotated text inside not rotated shape. My last patch was about this code section(calc shape and text rotation import) and my next patch will be about this code section. I will start working bug 76456 this is just partial fix for calc shape and text rotation import till me or someone fix all problems on calc's shape and text rotation import.
Comment 11 Regina Henschel 2018-01-31 22:49:20 UTC
I have contact Noel Power in the meantime. He only remembers that there had been some problems with customer documents, but he doesn't know any details after the long time. The mentioned bug is not public.

Serdar Oktay TUNÇ: I think, that it is still wrong. For a shape, which has a 90° vertical text and a 90° shape rotation in Excel, you have to get a shape in Calc, which has a draw:transform attribute for the 90° shape rotation and a draw:text-rotate-angle="-90" in its draw:enhanced-geometry for the vertical text. Already in the UI, you see, that the shape is not rotated.

With your patch I get a draw:text-rotate-angle="-180" and no rotation at all. You combine text- and shape-rotation into one value for text rotation and so loose the shape rotation. That will cause trouble at latest when the feature "rotate background with shape" is going to be implemented.

So please postpone this and first fix the shape rotation.
Comment 12 Serdar Oktay TUNÇ 2018-01-31 23:14:26 UTC
If i do that textbox rotation will be wrong. I think Noel Power's patch is about textbox rotation. If textbox is rotated in excel. Calc imports rotated shape's coordinates and rotates shape by rotation angle again. Its kinda partial fix for textboxes. Can you send testing document for text-rotate-angle="-180". If i fix shape rotation why bug 76456 didnt marked as duplicate? I am just saying some people might need that partial fix by the way I assigned bug 76456
Comment 13 Regina Henschel 2018-01-31 23:36:15 UTC
Created attachment 139480 [details]
Source document
Comment 14 Regina Henschel 2018-01-31 23:37:44 UTC
Created attachment 139483 [details]
source document opened in LO with applied patch and resaved as ods

Look at the shape "rect_90deg".
Comment 15 Markus Mohrhard 2018-02-05 12:53:27 UTC
A few comments that will with a perfect solution:

We are currently not handling the upright property of bodyPr which is the problem that Noel tried to fix without realizing that there is such a property.

Additionally there is some mess with the shape transformations in the xlsx import code:

oox::xls::DrawingFragment::onEndElement applies the transformation already that is later applied in oox::drawingml::Shape::addShape. We need to make sure that we are not applying the transformation twice.

Additionally we will need to add a huge number of test cases as there are quite a number of special cases.
Comment 16 Regina Henschel 2018-02-05 21:54:58 UTC
Created attachment 139613 [details]
Example, where the "upright" attribute is used.

The upright attribute is the reason, that in Excel a text is not rotated, although the shape is rotated.
Comment 17 Serdar Oktay TUNÇ 2018-02-05 22:37:30 UTC
I know about upright attribute is the reason but I dont know where is the code section about upright attribute. Is this bug only for Calc or all the components inside Libreoffice? I will try to add some test cases for applying transformation twice. There is many bugs in Calc just like them. I think all of them should mark as duplicate.
Comment 18 Regina Henschel 2018-02-05 23:55:04 UTC
(In reply to Serdar Oktay TUNÇ from comment #17)
> I know about upright attribute is the reason but I dont know where is the
> code section about upright attribute.

Using OpenGrok, I only find XML_UPRIGHT once in /core/oox/source/drawingml/textbodypropertiescontext.cxx. And there it is commented out. That means, that attribute is not read at all, and as result not evaluated. The occurrence in xmloff is not related, because it belongs to ODF import-export.
It seems, that a suitable item for it is missing in struct TextBodyProperties.

 Is this bug only for Calc or all the
> components inside Libreoffice?

I don't know for sure, but from the place in OOX it should be a problem in other applications too. But I have not found, how to generate this attribute in the UI of MS Office. If I set the flag manually for an pptx-document, then I can see, that the text orientation in a shape is wrong in import in Impress.

 I will try to add some test cases for
> applying transformation twice. There is many bugs in Calc just like them. I
> think all of them should mark as duplicate.

Marking duplicate is not argent. If they no longer show their error using a build that has got the fix, then it is clear, that they are duplicates. No need to carry about them now.
Comment 19 Regina Henschel 2018-02-10 15:27:21 UTC
Please see bug 115591 too, which is about "upright" text independent from import/export with MS Office formats.
Comment 20 Serdar Oktay TUNÇ 2018-02-15 22:56:25 UTC
I review the code section of drawingfragment:onEndElement and oox::drawingml::Shape::addShape functions. I could not find second rotation process in onEndElement function. https://opengrok.libreoffice.org/xref/core/sc/source/filter/oox/drawingfragment.cxx#261 this line sets rotation to zero. Because some shape's maybe just rectangular shape's coordinates have been given by the last position after rotation in the drawing.xml file. I think that's the only reason for all of shape and text rotation problems.Upright property is not the problem in my opinion that's all about shape's rotation. If partial fixes on text rotations won't be accepted, my solution is, if the shape is rectangular don't rotate it. But I don't know how to get shape's property has been given <a:prstGeom prst="rect"> label on drawingfragment.cxx file. I need to know what is the bug about https://opengrok.libreoffice.org/xref/core/sc/source/filter/oox/drawingfragment.cxx#261 it is private bug I cant read and I don't know what it is. https://opengrok.libreoffice.org/xref/core/oox/source/drawingml/customshapegeometry.cxx#1180 file has the rectangular shape's code that's all I know. I am sure Markus knows better than me and I need his help.
Comment 21 Markus Mohrhard 2018-02-17 19:02:21 UTC
(In reply to Serdar Oktay TUNÇ from comment #20)
> I review the code section of drawingfragment:onEndElement and
> oox::drawingml::Shape::addShape functions. I could not find second rotation
> process in onEndElement function.
> https://opengrok.libreoffice.org/xref/core/sc/source/filter/oox/
> drawingfragment.cxx#261 this line sets rotation to zero. Because some
> shape's maybe just rectangular shape's coordinates have been given by the
> last position after rotation in the drawing.xml file. I think that's the
> only reason for all of shape and text rotation problems.Upright property is
> not the problem in my opinion that's all about shape's rotation. If partial
> fixes on text rotations won't be accepted, my solution is, if the shape is
> rectangular don't rotate it. But I don't know how to get shape's property
> has been given <a:prstGeom prst="rect"> label on drawingfragment.cxx file. I
> need to know what is the bug about
> https://opengrok.libreoffice.org/xref/core/sc/source/filter/oox/
> drawingfragment.cxx#261 it is private bug I cant read and I don't know what
> it is.
> https://opengrok.libreoffice.org/xref/core/oox/source/drawingml/
> customshapegeometry.cxx#1180 file has the rectangular shape's code that's
> all I know. I am sure Markus knows better than me and I need his help.

Hey,

the upright property and the shape rotation bugs are related. The original fix that caused the bug was incorrectly applied to handle a case where the upright property is set.

Now the correct way for handling all and not just a single problem is to implement the correct text and shape rotation handling to avoid putting hack on  hack until nobody understands the code anymore.

IMHO the way to go is to start by handling the upright property as that will make sure that all documents with that property have correct text rotation. The next step is to make sure that the shape rotation and shape dimension code is correct. There are a number of bugs where sc and oox try to do the same thing and cause issues.

To start I would actually not start with fixing the bugs but by creating a number of test documents that you can use. These should include a document with the upright property, documents with rotated shapes, with rotated shapes + rotated text, ...
Integrate these documents into automatic tests that you can run to see if something breaks, starts to work, ...

And finally start fixing the bugs one by one with the upright property and the conflicts between sc and oox.
Comment 22 Szabolcs Toth 2020-05-29 15:19:49 UTC

*** This bug has been marked as a duplicate of bug 83593 ***