Bug 141652 - FILESAVE: DOCX: Image distorted after RT
Summary: FILESAVE: DOCX: Image distorted after RT
Status: RESOLVED WORKSFORME
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
7.2.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: fixed:7.3.1 reverted:7.3.6
Keywords: bibisected, bisected, regression
Depends on:
Blocks: DOCX-Images
  Show dependency treegraph
 
Reported: 2021-04-12 15:59 UTC by Xisco Faulí
Modified: 2022-11-25 08:19 UTC (History)
9 users (show)

See Also:
Crash report or crash signature:


Attachments
Comparison before and after RT (658.63 KB, image/png)
2021-04-12 15:59 UTC, Xisco Faulí
Details
Screenshot in LO 7.5 (816.03 KB, image/png)
2022-11-16 00:07 UTC, Aron Budea
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Xisco Faulí 2021-04-12 15:59:48 UTC
Created attachment 171131 [details]
Comparison before  and after RT

Steps to reproduce:
1. Open attachment 86229 [details] from bug 64817
2. Save it to DOCX
3. Open the new generated file

-> The sunflower is completely distorted. See comparison

Reproduced in

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

[Bug found by office-interoperability-tools]
Comment 1 Xisco Faulí 2021-04-12 16:00:39 UTC
Regression introduced by:

https://cgit.freedesktop.org/libreoffice/core/commit/?id=b90a67838e189f3aee6a50724c78c0a50d416970

author	Gülşah Köse <gulsah.kose@collabora.com>	2021-02-24 15:05:01 +0300
committer	Gülşah Köse <gulsah.kose@collabora.com>	2021-03-03 13:19:08 +0100
commit b90a67838e189f3aee6a50724c78c0a50d416970 (patch)
tree b8a993e271d5467bde23a3fd046d2be8e9129b0c
parent ca98e505cd69bf95d8ddb9387cf3f8e03ae4577d (diff)
Reset ShapeProperty priority and handle only crop case.

Bisected with: bibisect-linux64-7.2

Adding Cc: to Gülşah Köse
Comment 2 Justin L 2021-11-10 09:56:55 UTC
I thought it was a FILEOPEN problem because if I revert, then all previously round-tripped files opened OK. But in Word 2016 they looked stretched out. So it must be a file-save problem after all.

-      rPropMap.setProperty(ShapeProperty::FillBitmap, xGraphic);
+      if (rPropMap.supportsProperty(ShapeProperty::FillBitmapName))
+          rPropMap.setProperty(ShapeProperty::FillBitmapName, xGraphic);
+      else
+          rPropMap.setProperty(ShapeProperty::FillBitmap, xGraphic);
Comment 3 Justin L 2022-01-10 06:05:39 UTC
Xisco backported the fix to 7.1. Not sure exactly which point release it was, but the time was Mar 10, 2021 10:14.
Comment 4 Justin L 2022-01-10 09:27:48 UTC
All of these import-only changes started with LO 7.1.1 in order to fix bug 134210. Since this is an export bug, probably it has just exposed something else.

The order was initial addition of the entire problematic "if(bIsCustomShape)" clause in commit 2c96bd26ec488d865370fe9d394e7c4e228e05ab on 2021-01-21 10:52:46.

Then it changed to if(bIsCustomShape &&
   ( aGraphCrop.Left != 0 || aGraphCrop.Right != 0 || aGraphCrop.Top != 0 || aGraphCrop.Bottom != 0)) in commit 7400372efb3d279225a966a355644d2a3e7d42c3 on Feb 23 16:41:15 2021

and finally to "if(bIsCustomShape && bHasCropValues && bNeedCrop)" in comment 1's commit on 2021-03-03 13:19:08.


Export seems to be multiplying the crop component in the case of exporting a FillBitmapName, but not if just FillBitmap as indicated in comment 2. 

The addition of FillBitmapName looks like a copy/paste from elsewhere in this file (based on review history). I personally have no idea about shapes, so I don't know why one version should be chosen over another. The easy fix here seems to only use FillBitmap as was done in earlier versions of the patch series.
Comment 5 Justin L 2022-01-10 18:53:33 UTC
At export time, the difference is 
oox/source/export/drawingml.cxx:1763: ::WriteXGraphicStretch [600x401] crop right[-769]

instead of
oox/source/export/drawingml.cxx:1763: ::WriteXGraphicStretch [10160x6790] crop right[-769]

The function calls ::SetFillBitmap and ::SetFillBitmapName are very similar, except the prior having the clause
    if (!maShapePropInfo.mbNamedFillBitmap)
        return setAnyProperty(nPropId, rValue);
which is followed in this case.


Proposed partial revert at http://gerrit.libreoffice.org/c/core/+/128260
Comment 6 Commit Notification 2022-02-01 12:32:11 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/18cc1240565e697859dd7d17058f91d5e01df929

tdf#141652 partial revert "Reset ShapeProperty priority...

It will be available in 7.4.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 7 Commit Notification 2022-02-01 15:14:05 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

https://git.libreoffice.org/core/commit/1f0699415a7fc531a3ef4d1c80649606b6d06ece

tdf#141652 partial revert "Reset ShapeProperty priority...

It will be available in 7.3.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 8 Jean-Baptiste Faure 2022-02-01 17:11:46 UTC
Verified fixed.

Version: 7.3.1.0.0+ / LibreOffice Community
Build ID: 79c7f4378ec38c0e153032cccb437a665b318045
CPU threads: 8; OS: Linux 5.13; UI render: default; VCL: gtk3
Locale: fr-FR (fr_FR.UTF-8); UI: fr-FR
Ubuntu_20.04_x86-64
Calc: threaded

Best regards. JBF
Comment 9 Commit Notification 2022-07-28 22:09:46 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

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

Revert "tdf#141652 partial revert "Reset ShapeProperty priority..."

It will be available in 7.5.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 10 Commit Notification 2022-07-29 08:25:52 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-7-4":

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

Revert "tdf#141652 partial revert "Reset ShapeProperty priority..."

It will be available in 7.4.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 11 Commit Notification 2022-08-03 10:33:50 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

https://git.libreoffice.org/core/commit/5397ff649deadc301d9c3f1187814c91f8e7f77b

Revert "tdf#141652 partial revert "Reset ShapeProperty priority..."

It will be available in 7.3.6.

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 Justin L 2022-08-03 10:37:56 UTC
The bug was re-opened because the (now reverted) fix caused regression bug 150061.
Comment 13 Rajasekaran Karunanithi 2022-11-15 23:43:28 UTC
Can't reproduce in L.O 7.4.2.3 under Windows 10(x64).The generated docx file looks fine. No distorted sunflower image.

Version: 7.4.2.3 (x64) / LibreOffice Community
Build ID: 382eef1f22670f7f4118c8c2dd222ec7ad009daf
CPU threads: 4; OS: Windows 10.0 Build 19044; UI render: Skia/Raster; VCL: win
Locale: ta-IN (en_IN); UI: en-US
Calc: threaded
Comment 14 Aron Budea 2022-11-16 00:07:35 UTC
Created attachment 183620 [details]
Screenshot in LO 7.5

I can still see distortion, but it isn't as extreme, please check the attached screenshot, and compare to what you see.

This is with LO 7.5.0.0.alpha0+ (cfc8a8f5d841b3f84d207196153be67da7f60652) / Ubuntu.
I also checked the state right after the revert, and it gave the same result, perhaps the difference in the distortion comes from a different, preceding commit.
Comment 15 Stéphane Guillou (stragu) 2022-11-25 08:19:35 UTC
(In reply to Aron Budea from comment #14)
> Created attachment 183620 [details]
> Screenshot in LO 7.5
> 
> I can still see distortion, but it isn't as extreme, please check the
> attached screenshot, and compare to what you see.
> 
> This is with LO 7.5.0.0.alpha0+ (cfc8a8f5d841b3f84d207196153be67da7f60652) /
> Ubuntu.
> I also checked the state right after the revert, and it gave the same
> result, perhaps the difference in the distortion comes from a different,
> preceding commit.

Yes, I confirm Aron's less dramatic stretch.

Version: 7.4.3.2 / LibreOffice Community
Build ID: 1048a8393ae2eeec98dff31b5c133c5f1d08b890
CPU threads: 8; OS: Linux 5.15; UI render: default; VCL: gtk3
Locale: de-DE (en_AU.UTF-8); UI: en-US
Calc: threaded

However, with a master build from today, I see the issue fixed. Wondering if it's the same fix as for Bug 152199, by Kendy:

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

Version: 7.5.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 24d7431876e87eba700a9f141dc8e030143a92ad
CPU threads: 8; OS: Linux 5.15; UI render: default; VCL: gtk3
Locale: en-AU (en_AU.UTF-8); UI: en-US
Calc: threaded