Bug 141269 - FILESAVE: PNG: Incorrect transparency after roundtrip
Summary: FILESAVE: PNG: Incorrect transparency after roundtrip
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
7.2.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.2.0
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2021-03-26 15:53 UTC by Xisco Faulí
Modified: 2021-04-16 08:09 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
Comparison before and after RT (172.06 KB, image/png)
2021-03-26 15:53 UTC, Xisco Faulí
Details
simplify reproducer (194.10 KB, application/vnd.oasis.opendocument.presentation)
2021-04-14 10:07 UTC, Xisco Faulí
Details
How the PNG loooks like after saving it (70.91 KB, image/png)
2021-04-14 10:08 UTC, Xisco Faulí
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Xisco Faulí 2021-03-26 15:53:11 UTC
Created attachment 170768 [details]
Comparison before  and after RT

Steps to reproduce:
1. Open attachment 125967 [details] from bug 100662
2. Save it as PPT
3. Open the new generate document

-> Transparency in many slides is incorrect. See screenshot ( see also the slide sorter )

Reproduced in

Version: 7.2.0.0.alpha0+ / LibreOffice Community
Build ID: 3e4eb070787d4d44b3bdc95046e5b231dbbef42b
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-03-26 15:54:19 UTC
Regression introduced by:

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

author	Noel Grandin <noel@peralex.com>	2021-01-07 09:46:07 +0200
committer	Noel Grandin <noel.grandin@collabora.co.uk>	2021-02-23 11:20:35 +0100
commit c181e510c5f5e74f1f6824b64637849aace9ae63 (patch)
tree 31a658cbe4d8e1de92a1dcd39cd649a93f74685b
parent d042b39c2f30246b51cba65400552c25a6dd5105 (diff)
convert internal bitmap formats transparency->alpha

Bisected with: bibisect-linux64-7.2

Adding Cc: to Noel Grandin
Comment 2 Xisco Faulí 2021-03-26 15:57:20 UTC
Also reproduced with attachment 146067 [details] from bug 120964
Comment 3 Xisco Faulí 2021-03-26 15:59:35 UTC
and attachment 146080 [details] from bug 120974
Comment 4 Xisco Faulí 2021-04-09 09:49:31 UTC
Hi Noel,
I believe the problem is around filter/source/msfilter/escherex.cxx:1682 ? I tried to get rid of the '255 -' but the issue is still reproducible.
Could you please take a look when you have some time ?
Comment 5 Noel Grandin 2021-04-09 10:18:17 UTC
If the bug is in that file, are you sure about the bibiseect? And is this definitely a save issue, or did I mess up loading PPT files?
Comment 6 Xisco Faulí 2021-04-11 20:49:01 UTC
(In reply to Noel Grandin from comment #5)
> If the bug is in that file, are you sure about the bibiseect? And is this
> definitely a save issue, or did I mess up loading PPT files?

Nop, the bug is not just in that file, files mentioned in comment 2 and comment 3 are also affected that I could find.
The bisection is correct, if you revert the commit locally, you will see the problem goes away
Comment 7 Xisco Faulí 2021-04-14 10:07:31 UTC
Created attachment 171183 [details]
simplify reproducer

Hi Noel,
I've investigated this issue a bit, and it turned out the problem is exporting to PNG.

Steps to reproduce:
1. Open attached document
2. Select the image, right click, save
3. Save the image to PNG
Comment 8 Xisco Faulí 2021-04-14 10:08:15 UTC
Created attachment 171184 [details]
How the PNG loooks like after saving it
Comment 9 Xisco Faulí 2021-04-14 10:14:23 UTC
it seems this change fixes the issue

--- a/vcl/source/filter/png/pngwrite.cxx
+++ b/vcl/source/filter/png/pngwrite.cxx
@@ -563,7 +563,7 @@ sal_uLong PNGWriterImpl::ImplGetFilter(sal_uLong nY, sal_uLong nXStart, sal_uLon
                         *pDest++ = rColor.GetRed();
                         *pDest++ = rColor.GetGreen();
                         *pDest++ = rColor.GetBlue();
-                        *pDest++ = 255 - mpMaskAccess->GetIndexFromData(pScanlineMask, nX);
+                        *pDest++ = mpMaskAccess->GetIndexFromData(pScanlineMask, nX);
                     }
                 }
                 else


however, it breaks the transparency of the icons in the sidebar
Comment 10 Noel Grandin 2021-04-14 10:35:13 UTC
good spotting x1sco!

In that case, the problem is likely in
   vcl/source/filter/png/pngwrite.cxx
Comment 11 Xisco Faulí 2021-04-14 19:29:09 UTC
(In reply to Noel Grandin from comment #10)
> good spotting x1sco!
> 
> In that case, the problem is likely in
>    vcl/source/filter/png/pngwrite.cxx

Hi Noel,
I gave it a try but I failed. My change < https://gerrit.libreoffice.org/c/core/+/114082/2 > breaks the icons in the sidebar in GTK3 ( see https://imgur.com/a/BYj3CFE ) and a unittest in sw_ooxmlexport10.
Could you please take a look to this issue whenever you have some time ?
Comment 12 Caolán McNamara 2021-04-14 20:03:17 UTC
The icons in the sidebar in GTK3 are set by passing a block of png data to gtk which them renders the png internally, so those showing broken to me indicates that we didn't give them the correct png data, I feel there are a few separate places not working.

so, currently internally in bitmap data we are supposed to be using opacity and not transparency so a fully transparent pixel has an alpha of 0 ?

vcl/source/filter/png/PngImageReader.cxx:309 has:
  pScanAlpha[iAlpha++] = 0xFF - pRow[i + 3]
is that then an out of date conversion from opacity to transparency.
vcl/source/filter/png/pngwrite.cxx:566 has a matching:
  255 - mpMaskAccess->GetIndexFromData(pScanlineMask, nX)
to go the other direction
vcl/headless/svpgdi.cxx has some "make upper layers use standard alpha" places where it does some toggling which might be wrong now (?)
if BitmapEx::AdjustTransparency input cTrans is transparency then should 
nNewTrans = nTrans + *pAScan convert *pAScan from opacity to transparency before the addition ?
Comment 13 Caolán McNamara 2021-04-15 13:53:37 UTC
vcl/unx/generic/gdi/gdiimpl.cxx X11SalGraphicsImpl::drawAlphaBitmap likewise has a inverting block at // the alpha values need to be inverted for XRender and vcl/source/outdev/bitmap.cxx AlphaBlend // vcl stores transparency, not alpha - invert it
Comment 14 Noel Grandin 2021-04-15 16:29:31 UTC
fix for PNG export at
    https://gerrit.libreoffice.org/c/core/+/114168
Comment 15 Commit Notification 2021-04-15 19:06:48 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

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

tdf#141269 Incorrect transparency after roundtrip

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 16 Caolán McNamara 2021-04-15 20:01:24 UTC
should work again by reverting for now and hopefully revisit again as its a desirable to align alpha with the external world
Comment 17 Commit Notification 2021-04-15 20:11:28 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/016da0cece1f01a36f2417f5fbcab3a506f47303

tdf#141269: sd_export: Add unittest

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 18 Xisco Faulí 2021-04-16 08:09:32 UTC
Verified in

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

@Caolán, thanks for taking care of this issue!