Bug 113494 - FILESAVE: New feature image rotation uses wrong value in transformation attribute
Summary: FILESAVE: New feature image rotation uses wrong value in transformation attri...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
6.0.0.0.alpha1+
Hardware: x86 (IA32) All
: high normal
Assignee: Armin Le Grand (allotropia)
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-28 14:22 UTC by Regina Henschel
Modified: 2018-04-19 18:07 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Regina Henschel 2017-10-28 14:22:04 UTC
Insert an image in a text document and rotate it by 30°. Save document. Unpack it and examine the attribute "transform" of the <draw:frame> element of the image.

LibreOffice has a distinction between an image as SwXTextGraphicObject or as SwXShape. ODF 1.2 has no such distinction. In both cases the file format has a <draw:frame> element with <draw:image> child element and a style of family "graphic".

That means, that from the view of the file format the attributes of a rotated image derived from a SwXshape have to be the same as derived from a SwXTextGraphicObject. Every converting needed for the internal model has to be done outside of those attributes, which are saved/loaded to/from file.

That is not the case in the new feature of rotating a text graphic in Writer.
Example: Using 30deg as rotation angle in the UI results in the attribute draw:transform="rotate(300)".

This is wrong in two aspects:
(1) The rotation angle should be in radians, but it is in 1/10degree.
(2) Rotations are done around coordinate system origin, and to get the rotated object to the correct position an additional translate-transformation is necessary.
Comment 1 Buovjaga 2017-11-09 15:46:24 UTC
Reproduced.

Arch Linux 64-bit, KDE Plasma 5
Version: 6.0.0.0.alpha1+
Build ID: 4b5751dd0b08d5fe55f89513ea1062f059c493c7
CPU threads: 8; OS: Linux 4.13; UI render: default; VCL: kde4; 
Locale: fi-FI (fi_FI.UTF-8); Calc: group
Built on November 8th 2017
Comment 2 Armin Le Grand (allotropia) 2018-01-10 10:52:59 UTC
Already fixed something (see 'RotateFlyFrameFix', cb628728a33a3c6092ab929def747252ecec428c), but need to check if - due to rotation around center - this is sufficient. It may be necessary to add the needed translate(s) to the transformation to make this mathematically correct.
Comment 3 Armin Le Grand (allotropia) 2018-01-10 11:06:30 UTC
Checked unrotated: First svg:x, svg:y get written, then svg:width/height. When we propose that the order of svg commands matters, this is already wrong. When proposing to transform the shape starting from the implied UnitShape, we first need to scale, then translate.
When rotated, it is svg:x/svg:y/svg:width/svg:height/draw:transform. To express rotation as around center we will have to translate minus center, rot, translate center in this configuration when taking order serious.
Alternatively, draw:transform could be first in the order, then to express rotation in transformation could be done using: translate(-0.5, -0.5), rot(...), translate(0.5, 0.5).
Not sure what to do. The order in existing docs already *is* as described in 1st lines, draw:transform is new and could still be decided about where to place it.
Comment 4 Armin Le Grand (allotropia) 2018-01-10 11:13:47 UTC
Checked with draw, it correctly writes width/height/x/y unrotated, so first scale, then rotate.
When rotated it changes to svg:width="7cm" svg:height="4.5cm" draw:transform="rotate (0.349065850398866) translate (2.342cm 12.033cm)". Also correct, first scale, then rotate (consequently top-left), then translate (to the destination of the top-left corner, *not* the BoundRect).
Thus we should try to get closer to this in writer, too. Exchange scale/translate as needed in export? What to do with existing docs, imply 'scale first' for a while...?
Comment 5 Regina Henschel 2018-01-10 12:37:28 UTC
Armin, the attributes in file has to be _exactly the same_ as if it is not a Writer-image but a Draw-image. The differentiation between Writer-image and Draw-image is artificial and has nothing to do with ODF. All needed conversion has to be done during export/import.

LibreOffice uses as trick to be able to distinguish Writer-image from Draw-image the fact, that it does not write the attribute style:parent-style-name in case of a Draw-image. To test the behavior you can add/remove attribute style:parent-style-name="Graphics" in its <style:style> element in <office:automatic-styles> in content.xml
Comment 6 Armin Le Grand (allotropia) 2018-01-10 12:42:01 UTC
Some steps further: Examined input-code from draw/impress. While svg:x/y/width/height define the basic shape, draw:transform is seen as 'apply to this'. Order is thus independent for svg:x/y/width/height, but valid for draw:transform (inside of and when multiples - is that allowed at all..?).
Thus need to add draw:transform(-transCenter, rot, transCenter) after others, need to calculate center, need to check how UnitConverter is used here...
Comment 7 Armin Le Grand (allotropia) 2018-01-10 12:43:15 UTC
Regina, thanks for that, I am about to do exactly that - use _exactly the same_ as far as possible, that's why I compare with draw/impress...
Comment 8 Armin Le Grand (allotropia) 2018-01-10 17:12:14 UTC
Added supposed draw:transform(-transCenter, rot, transCenter) and works as planned. If applied mathematically to the shape given by svg:x/y/w/h all is okay. Preparing to add this ASAP to get to 6.0. This gives us the needed data in the ODF for this case, AFAIK also for future usages.
A second change will be the disable of 'AutoContour', also ASAP.
Comment 9 Armin Le Grand (allotropia) 2018-01-10 17:41:16 UTC
Both on gerrit, test build:
https://gerrit.libreoffice.org/#/c/47719/
https://gerrit.libreoffice.org/#/c/47721/
Comment 10 Armin Le Grand (allotropia) 2018-01-11 15:12:21 UTC
All changes in:

libreoffice-6-0: d45631314cef00008a538900800561b435202917 RotateFlyFrameFix: Add RotCenter info to ODF export 353a736b719167ce1caac9cb20a607e3aeda6dca RotateFlyFrameFix: Disable AutoContour for rotated Flys

master: ced59c16a850a9e4c87dd779b7130e8322d87493 RotateFlyFrameFix: Add RotCenter info to ODF export cba1c400af98081abc63d282d6910c9f58403ae3 RotateFlyFrameFix: Disable AutoContour for rotated Flys

On it to add mathematically more precision, seen when doing manual testing/checking, buils in progress on gerrit.
Comment 11 Armin Le Grand (allotropia) 2018-01-11 22:27:23 UTC
Done, see

libreoffice-6-0: 11991973ba565957cef5f09d424e4e2d25c2e1b6 Corrected precision of imported rotation
master: 11991973ba565957cef5f09d424e4e2d25c2e1b6 Corrected precision of imported rotation