Bug 139258 - Rotated image saved as xlsx has wrong width/height ratio and position when opened in MS Excel
Summary: Rotated image saved as xlsx has wrong width/height ratio and position when op...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
6.1.0.3 release
Hardware: x86-64 (AMD64) Windows (All)
: medium normal
Assignee: Szabolcs Toth
URL:
Whiteboard: target:7.2.0 target:7.1.1
Keywords: filter:xlsx
: 91457 (view as bug list)
Depends on:
Blocks: XLSX-Images
  Show dependency treegraph
 
Reported: 2020-12-27 12:10 UTC by snowboard975
Modified: 2021-02-26 14:52 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
Comparision of rotated image saved in xlsx shown in Libreoffice and MS Excel (1.15 MB, image/png)
2020-12-27 12:13 UTC, snowboard975
Details
Rotated image in the original ods format (618.50 KB, application/vnd.oasis.opendocument.spreadsheet)
2020-12-27 12:15 UTC, snowboard975
Details
Rotated image saved in xlsx format (583.63 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2020-12-27 12:16 UTC, snowboard975
Details
proposed patch (1.51 KB, patch)
2020-12-29 15:30 UTC, Regina Henschel
Details
The better example file and its XLSX-saved version in Calc (1.01 MB, image/png)
2020-12-30 14:22 UTC, NISZ LibreOffice Team
Details

Note You need to log in before you can comment on or make changes to this bug.
Description snowboard975 2020-12-27 12:10:29 UTC
Description:
When an image is rotated in Libreoffice Calc and saved as xlsx, the image is shown at a wrong position with a wrong ratio of width/height when opened in MS Excel. 

Steps to Reproduce:
1. In Libreoffice Calc, insert an image that is not in a square shape but a rectangular one.
2. Rotate it by 90°.
2. Save it as xlsx.
3. Open the xlsx file in MS Excel.

Actual Results:
The width and height ratio seems to remain as unrotated image and it makes the image look distorted. The position of image shown in MS Excel is displaced from the correct position in Libreoffice Calc.

Expected Results:
The rotated image shown in MS Excel should have the same width and height as set in Libreoffice Calc and be placed on the same position.


Reproducible: Always


User Profile Reset: Yes



Additional Info:
Liberoffice Calc should save the rotated image on a correct position in an xlsx file format, too.
Comment 1 snowboard975 2020-12-27 12:13:22 UTC
Created attachment 168512 [details]
Comparision of rotated image saved in xlsx shown in Libreoffice and MS Excel
Comment 2 snowboard975 2020-12-27 12:15:50 UTC
Created attachment 168515 [details]
Rotated image in the original ods format
Comment 3 snowboard975 2020-12-27 12:16:18 UTC
Created attachment 168516 [details]
Rotated image saved in xlsx format
Comment 4 Regina Henschel 2020-12-27 17:50:39 UTC
In LO54 the image was not rotated at all.
In a LO60 daily the image is rotated and shows already the wrong position.
Likely no regression but an implementation error.

It is likely mainly an export bug, because opening a file made by Excel has correct rotation, size and position, only the anchor type is wrong.
Import problems are already tracked in bug 129063 and bug 113515.

I have looked for duplicates, but didn't found one.
Comment 5 Regina Henschel 2020-12-29 14:57:04 UTC
The method XclObjAny::WriteFromTo() in https://opengrok.libreoffice.org/xref/core/sc/source/filter/xcl97/xcl97rec.cxx?r=3184cfc4#1087 has special handling for custom shapes. I think, the same is needed for other kind of objects too.

Not only images, but legacy rectangle and ellipse as well as text boxes have the same error. It seems to me, that the decision for special object handling should not be made on the shape type but on rotation angle. So all objects, which are able to rotate, would be covered.
Comment 6 Regina Henschel 2020-12-29 15:30:19 UTC
Created attachment 168557 [details]
proposed patch

Removing the restriction to custom shapes seems to solve the problems.
Comment 7 NISZ LibreOffice Team 2020-12-30 14:22:20 UTC
Created attachment 168580 [details]
The better example file and its XLSX-saved version in Calc

How it looks in:

Version: 7.2.0.0.alpha0+ (x64)
Build ID: c0eee433e079d8e3413f4691607e075b99af92b0
CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: Skia/Raster; VCL: win
Locale: hu-HU (hu_HU); UI: en-US
Calc: CL

The top left corner of the image goes from about A5 to S30, and looks the same in Excel.
Comment 8 Szabolcs Toth 2020-12-30 14:58:47 UTC
Here is a super long comment about what happens with custom shapes on import:
https://opengrok.libreoffice.org/xref/core/sc/source/filter/oox/drawingfragment.cxx?r=3e85b227#268 . (how they are hangles in MSO in general)
It helps to make a shape in excel while reading the comment and color the cells with anchors (green)before and (red)after the rotation.
So make a document, save an unrotated shape, open the xml, write down the anchor positions, open the document, color the cells that you have written down.
Now rotate the shape, and repeat.


These are my first thoughts on this bug; I think all of these kind of bugs come from the difference of how we handle the rotations.
Since LO does not work the way MSO works, we must deal with this at least twice, at import and at export.
Whatever import comes through here https://gerrit.libreoffice.org/c/core/+/94611 is safe, and it seems like most of the shapes and such are going through there.
The export looks more spread.

I will start digging into this particular case very soon.
Comment 9 Szabolcs Toth 2020-12-31 09:53:18 UTC
(In reply to Regina Henschel from comment #6)
> Created attachment 168557 [details]
> proposed patch
> 
> Removing the restriction to custom shapes seems to solve the problems.

You are right Regina. The same thing happens with custom shapes and images.

SdrObj contains two rects:
logic - unrotated shape
snap - rotated shape.
I expected logicRect to contain the correct position of the shape / image too, not only its size. But when I draw the rect on LO (and MSO) axes (y = -y), the logicRect is off by far from where it should be. (but no problem with size or rotation)

(Import, that is not a matter on this bug, but relevant to the general problem:
When I force the "correct" top left point value into the SdrObj's logicRect on import, our representation in the opened document goes off by as much as the export goes off if I do not do this.)

So in my solution I find the good top left value by manipulating the snapRect, which has the correct position but the rotated shape.
In the next if, I rotate the shape, this is because of the things I said in my previous comment.


A note: rounding errors go crazy with larger objects. I am currently allowing 100 000 emus in my unit test. It is barely visible to the eye after one roundtrip, but after 10 roundtrips, it can be a problem. It is the emu-hmm conversion, not related to this bug.


The fix proposed by Regina:
https://gerrit.libreoffice.org/c/core/+/108533

Removing a restriction completely would cause problems in some cases but I will test if weakening it further fixes anything else. Any suggestions are appreciated.
Comment 10 Regina Henschel 2020-12-31 16:23:14 UTC
(In reply to Szabolcs Toth from comment #9)
> (In reply to Regina Henschel from comment #6)
> > Created attachment 168557 [details]
> > proposed patch
> > 
> > Removing the restriction to custom shapes seems to solve the problems.
> 
> You are right Regina. The same thing happens with custom shapes and images.
> 
> SdrObj contains two rects:
> logic - unrotated shape
> snap - rotated shape.
> I expected logicRect to contain the correct position of the shape / image
> too, not only its size. But when I draw the rect on LO (and MSO) axes (y =
> -y), the logicRect is off by far from where it should be. (but no problem
> with size or rotation)

The logic rectangle in LibreOffice has the position so, that applying shear and rotation to the left top corner of the logic rectangle as origin will result in the snap rectangle. [There are some quirks with vertical flipped custom shapes, which are not relevant here.]

But Excel applies the transformation to the center of the shape. It determines the center from the <a:xfrm> element. So we need to provide a rectangle there, that has the center in our snap rectangle and size the same as our logical rectangle. That is done in
const tools::Rectangle& aSnapRect(pObj->GetSnapRect());
aTopLeft.X = aSnapRect.getX() + (aSnapRect.GetWidth() / 2) - nHalfWidth;
aTopLeft.Y = aSnapRect.getY() + (aSnapRect.GetHeight() / 2) - nHalfHeight;

> 
> (Import, that is not a matter on this bug, but relevant to the general
> problem:
> When I force the "correct" top left point value into the SdrObj's logicRect
> on import, our representation in the opened document goes off by as much as
> the export goes off if I do not do this.)

The import would need the reverse conservation, but I see no problem in import, because files generated by Excel open fine.

> 
> So in my solution I find the good top left value by manipulating the
> snapRect, which has the correct position but the rotated shape.

The snapRect has the center, which is needed in Excel. But it does not have a left top corner, that is usable for Excels xfrm element, because our snapRect contains the rotated shape for arbitrary angles. Excel only uses the unrotated shape in case of rotation nearer to x-axis than diagonal and 90° rotated shape (around center of shape!) in case of rotation farer to x-axis than diagonals. The switch to the 90° rotated shape is done later in
aTopLeft.X = aTopLeft.X - nHalfHeight + nHalfWidth;
aTopLeft.Y = aTopLeft.Y - nHalfWidth + nHalfHeight;
std::swap(aSize.Width, aSize.Height);

> In the next if, I rotate the shape, this is because of the things I said in
> my previous comment.
> 
> 
> A note: rounding errors go crazy with larger objects. I am currently
> allowing 100 000 emus in my unit test. It is barely visible to the eye after
> one roundtrip, but after 10 roundtrips, it can be a problem. It is the
> emu-hmm conversion, not related to this bug.

There shouldn't be such large errors due to emu-hmm conversion. 100 000EMu are about 277 hmm.  Are you sure the inaccuracy in round-trip are not introduced by the inaccurate column width, which results in an inaccurate offset?

> 
> 
> The fix proposed by Regina:
> https://gerrit.libreoffice.org/c/core/+/108533
> 
> Removing a restriction completely would cause problems in some cases

Can you give details about the problems you have seen?
Comment 11 Commit Notification 2021-01-15 11:49:24 UTC
Szabolcs Toth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/fddad2ed797f1773ed5be979a0b05d3f976b744e

tdf#139258 XLSX export: fix position of rotated images

It will be available in 7.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 12 snowboard975 2021-01-18 10:55:48 UTC
(In reply to Commit Notification from comment #11)
> Szabolcs Toth committed a patch related to this issue.
> It has been pushed to "master":
> 
> https://git.libreoffice.org/core/commit/
> fddad2ed797f1773ed5be979a0b05d3f976b744e
> 
> tdf#139258 XLSX export: fix position of rotated images
> 
> It will be available in 7.2.0.
> 
> The patch should be included in the daily builds available at
> https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
> information about daily builds can be found at:
> https://wiki.documentfoundation.org/Testing_Daily_Builds
> 
> Affected users are encouraged to test the fix and report feedback.

Great! I tested this version, and the issue that I had was gone.
Thank you very much!
Comment 13 Xisco Faulí 2021-01-19 10:43:17 UTC
Verified in

Version: 7.2.0.0.alpha0+ / LibreOffice Community
Build ID: 4e1294b7d6f8de981147f15d4ca1b4e4853249eb
CPU threads: 4; OS: Linux 5.7; UI render: default; VCL: gtk3
Locale: en-US (en_US.UTF-8); UI: en-US
Calc: threaded

@Szabolcs Toth, thanks for fixing this issue!!
Comment 14 Commit Notification 2021-01-19 10:44:10 UTC
Szabolcs Toth committed a patch related to this issue.
It has been pushed to "libreoffice-7-1":

https://git.libreoffice.org/core/commit/aa23da6b9e935fbfcb0b432d43c72f7a6db50657

tdf#139258 XLSX export: fix position of rotated images

It will be available in 7.1.1.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 15 snowboard975 2021-01-21 10:56:15 UTC
(In reply to Commit Notification from comment #14)
> Szabolcs Toth committed a patch related to this issue.
> It has been pushed to "libreoffice-7-1":
> 
> https://git.libreoffice.org/core/commit/
> aa23da6b9e935fbfcb0b432d43c72f7a6db50657
> 
> tdf#139258 XLSX export: fix position of rotated images
> 
> It will be available in 7.1.1.
> 
> The patch should be included in the daily builds available at
> https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
> information about daily builds can be found at:
> https://wiki.documentfoundation.org/Testing_Daily_Builds
> 
> Affected users are encouraged to test the fix and report feedback.

I checked that it works nice in 7.1.1 as well. Thank you!

Test environment:
Version: 7.1.1.0.0+ (x64) / LibreOffice Community
Build ID: f9f5a92b9612e134994aad5f05a543860c6f098c
CPU threads: 4; OS: Windows 10.0 Build 18363; UI render: Skia/Raster; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: threaded
Comment 16 Xisco Faulí 2021-02-26 14:52:54 UTC
*** Bug 91457 has been marked as a duplicate of this bug. ***