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.
Created attachment 129159 [details]
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
It seems to be the same problem as in bug 76456.
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.
Created attachment 138556 [details]
Screenshot of shape properties in Excel
Created attachment 138557 [details]
Screenshot of text properties in Excel
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.
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 )
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.
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.
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.
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.
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
Created attachment 139480 [details]
Created attachment 139483 [details]
source document opened in LO with applied patch and resaved as ods
Look at the shape "rect_90deg".
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.
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.
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.
(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.
Please see bug 115591 too, which is about "upright" text independent from import/export with MS Office formats.
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.
(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.
> 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
> drawingfragment.cxx#261 it is private bug I cant read and I don't know what
> it is.
> 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.
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.
*** This bug has been marked as a duplicate of bug 83593 ***