Description: With Skia/Metal or Skia/Raster enabled on macOS, the LibreOffice "about" image added in a presentation document with 50% transparency will fail to render (i.e. leave white areas) when running in a slideshow or printing. This behavior is in the libreoffice-7-6 and master branches. But it is not in LibreOffice 7.5.4. Steps to Reproduce: 1. Open the attached Impress document 2. Run a slideshow or print Actual Results: Several unexpected white areas are in the central part of the image. Expected Results: No white areas within the image. Reproducible: Always User Profile Reset: No Additional Info: This only occurs with Skia.
Created attachment 188447 [details] LibreOffice "about" logo with 50% transparency
Created attachment 188448 [details] Patch that disables Skia alpha blending and falls through to the non-Skia slow path
I haven't found what commit caused this change, but it appears that somewhere in the libreoffice-7-6 and master branches the code in vcl/source/outdev/bitmap.cxx:345 is getting called more frequently. The patch in https://bugs.documentfoundation.org/attachment.cgi?id=188448 disables one Skia alpha blending case and allows that case to fall through to the non-Skia slow alpha blending code. With this patch, the bug does *not* occur so it appears that the bug is in one or both of the following: SkiaSalGraphicsImpl::blendAlphaBitmap() SkiaSalGraphicsImpl::blendBitmap()
Random idea: if I understand how we are supposed to draw to mpAlphaVDev, I'd replace the following at source/outdev/bitmap.cxx:349: mpAlphaVDev->BlendBitmap(aTR, rAlpha); with the following pseudo-code: // Make copy of pSalAlphaBmp2 pSalAlphaBmp3 = pSalAlphaBmp2->Clone(); // not sure what the real code is here // Convert transparency to alpha pSalAlphaBmp2->Invert(); // Use the uninverted copy of pSalAlphaBmp2 for blending and, instead of // blending with the drawn bitmap, blend with opaque white. Assuming // mpAlphaVDev is grayscale already, blending the following should overwrite // mpAlphaVDev with grayscale. The SK_ColorWHITE should be darkened by the // two alpha masks so in the end we should have the new, inverted alpha mask. mpAlphaVDev->mpGraphics->BlendAlphaBitmap(aTR, SK_ColorWHITE, *pSalAlphaBmp, *pSalAlphaBmp3, *this)) // Convert alpha to transparency pSalAlphaBmp2->Invert(); return; I'll see if I can get something working in the next couple of days.
So after thinking about this more, I think I now know what is happening. Note: masks on master and libreoffice-7-6 branches so 0 is black (opaque) and 1 is white (transparent) in the examples below. Also, alpha masks are filled with opaque grayscale colors so we don't need to factor in any blending of alpha channels. In the case of the white areas in the semi-transparent .svg image, the white areas are VirtualDevices with their own AlphaMask drawn on top of an overlapping stack of other VirtualDevices with their own separate AlphaMask. Then, the bottom most VirtualDevice is drawn with a gray (50% transparency) alpha mask onto a white background. Blending the alpha masks works fine for the bottom most VirtualDevice because its alpha mask is gray (0.5) and the VirtualDevice it is being drawn to has an opaque (0.0) alpha mask. Applying SkBlendMode::kMultiply blends these two into a dark gray (0.25) alpha mask. So far so good. The problem arises when the destination VirtualDevice (as is the case in the white shapes) is filled with gray and/or white pixels (greater than 0.0 up to 1.0) blending will darken (i.e. make more opaque) the destination's alpha mask darken. For a really simplistic example, say we have a source VirtualDevice that has a 50% transparent (0.5) alpha mask. But the destination VirtualDevice's alpha mask we want to draw to already has a 100% transparent (1.0) alpha mask. In theory, after blending with SkBlendMode::kMultiply, the destination's alpha mask is filled with gray (1.0 * 0.5) but should be filled with white (1.0) because if the destination's alpha mask is fully transparent, nothing will be drawn regardless of the alpha mask of the source. To summarize the issue: mpGraphics->BlendBitmap() makes the destination's alpha mask more opaque than it should be when the destination's alpha bitmap is semi-transparent or fully transparent. So, I think mpGraphics->BlendBitmap() needs to use the following blending algorithm: r = 1 - ((1 - d) + ((1 - d) * (1 - s))) This should prevent the source alpha mask passed into mpGraphics->BlendBitmap() from making semi-transparent and fully transparent areas in the destination more opaque.
(In reply to Patrick Luby from comment #5) > So, I think mpGraphics->BlendBitmap() needs to use the following blending > algorithm: > > r = 1 - ((1 - d) + ((1 - d) * (1 - s))) Disclaimer: the above is a little too simplistic as the source grayscale can still override the destination grayscale in certain cases. But it hopefully illustrates what I think is happening. I think the safest way to blend the alpha masks is to do the following: 1. In mpGraphics->BlendAlphaBitmap(), after drawing is finished, extract the destination surface's alpha channel from its SkAlphaShader, invert it, and overwrite the pSalAlphaBmp2 parameter. 2. Remove call to mpGraphics->BlendBitmap() since mpGraphics->BlendAlphaBitmap() will pSalAlphaBmp2 has been overwritten.
the slow path ends up here: https://opengrok.libreoffice.org/xref/core/vcl/source/outdev/bitmap.cxx?r=8bc077c9#AlphaBlend so that kMultiply looks rather wrong, kSrcOver looks like a closer match
Ah, but we are in the transparency world still with this bug, so we would have to convert r = s + (1-sa)*d to r = 1 - (s + ((1-(1-sa))*d) which simplifies to r = 1 - s - sa*d which is not something Skia has a SkBlendMode for, so no easy fix.
(In reply to Noel Grandin from comment #8) > which simplifies to > > r = 1 - s - sa*d > > which is not something Skia has a SkBlendMode for, so no easy fix. Your formula looks much closer to what I could come up with. Also, I realized my proposal in https://bugs.documentfoundation.org/show_bug.cgi?id=156361#c6 won't work since a VirtualDevice is initially filled with opaque white (which renders as white) and its AlphaMask is also filled with opaque white (which makes the VirtualDevice transparent). So, the alpha channel in the VirtualDevice is immediately out of sync with its AlphaMask. So, my understanding is that the AlphaMask starts as all white (transparent) and reflects the cumulatively drawn areas as gray or black. I haven't been able to come up with a blending algorithm for this, but it was what I was trying to do in https://bugs.documentfoundation.org/show_bug.cgi?id=156361#c4. Effectively, I think I want to repeat the drawing to the VirtualDevice but draw all white instead of the bitmap. I no longer think that all of the copying and inverting in that comment are needed. So, below is what I am thinking of replacing mpAlphaVDev->BlendBitmap(aTR, rAlpha) with: mpAlphaVDev->mpGraphics->BlendAlphaMasks(aTR, /* no bitmap */, *pSalAlphaBmp, *pSalAlphaBmp2, *this)) BlendAlphaMasks() would be basically be a copy of the code block in BlendAlphaBitmap() at vcl/skia/gdiimpl.cxx:1326 with the following changes: 1. Fill the surface with SK_ColorBLACK before blending 2. Blend the alpha masks the same, no change there 3. Blend the blended alpha masks with SK_ColorWHITE instead of a bitmap 4. Invert the surface with kDifference to get black == opaque and white == transparent Thoughts? I can try coding this in the next couple of days and see if it fixes anything.
(In reply to Patrick Luby from comment #9) > Thoughts? I can try coding this in the next couple of days and see if it > fixes anything. Honestly, I think the best thing to do is to remove this whole troublesome code block and return false for that routine, falling back to the slow path. It is not a widely used thing, and modern machines have really good single threaded performance, and modern compilers will turn that slow path routine into a chunk of efficient SIMD code. In the long run, when we have combined alpha, all of this complication will fall away, and we can use normal GPU optimised blend modes.
Created attachment 188498 [details] Semi-transparent, overlapping gradients
(In reply to Noel Grandin from comment #10) > Honestly, I think the best thing to do is to remove this whole troublesome > code block and return false for that routine, falling back to the slow path. > It is not a widely used thing, and modern machines have really good single > threaded performance, and modern compilers will turn that slow path routine > into a chunk of efficient SIMD code. > > In the long run, when we have combined alpha, all of this complication will > fall away, and we can use normal GPU optimised blend modes. I agree completely. I have submitted the following Gerrit change: https://gerrit.libreoffice.org/c/core/+/154678 Once we're happy with that change, I'll backport to libreoffice-7-6 and then I'll see if it fixes anything in the the following Gerrit change: https://gerrit.libreoffice.org/c/core/+/114168
Note: this bug does *not* occur in LibreOffice 7.5.5. It only occurs in libreoffice-7-6 and master branches.
Patrick Luby committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/3c63cb1298fc17b71629e04d181244b5acdf8409 tdf#156361 use slow blending path if alpha mask blending is diabled It will be available in 24.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.
Patrick Luby committed a patch related to this issue. It has been pushed to "libreoffice-7-6": https://git.libreoffice.org/core/commit/cde3a60e99ea52fb5d5e27963c74f1526a0560c7 tdf#156361 use slow blending path if alpha mask blending is diabled It will be available in 7.6.0.2. 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.
Created attachment 188548 [details] Breeze toolbar icon in Writer with gray horizontal line (circled in red)
Reopening as the fix that was committed is causing a gray horizontal line to be drawn in the Breeze and Breeze (SVG) font background color toolbar icon like in https://bugs.documentfoundation.org/attachment.cgi?id=188548. This regression only occurs when Skia/Metal is enabled but renders fine with Skia/Raster and Skia disabled so it looks like the bug is in the OutputDevice::DrawDeviceAlphaBitmapSlowPath(). Interestingly, the Skia alpha blending methods work in this case.
Update: I have narrowed the cause of this bug down a bit. So what is happening is that the two font color and font background color icons are 25 x 25. AFAIK most icons are 17 x 17 so the two problem icons have a VirtualDevice with an 8 x 25 undrawn area at the bottom. I think that it is how the pixels are handled in this undrawn area that cause this bug. The "aSrcCol = pP->GetColor( nMapY, nMapX );" call in vcl/source/outdev/bitmap.cxx:767 returns slightly different colors for both Skia/Metal and Skia/Raster, but they are both very dark gray so I don't think that is the cause. So, my next area to investigate is writing the blended pixels back to the SalBitmap and see if something is failing there with Skia/Metal.
(In reply to Patrick Luby from comment #18) > So, my next area to investigate is writing the blended pixels back to the > SalBitmap and see if something is failing there with Skia/Metal. Found the cause of the problem. The following patches fixes this the gray horizontal line in Breeze's font color and font background color toolbar icons. I think it is a very hacky patch and would not recommend committing it so I'll just post it below so it won't be forgotten: diff --git a/vcl/inc/skia/utils.hxx b/vcl/inc/skia/utils.hxx index 3c19192e1c3a..245776e46d4a 100644 --- a/vcl/inc/skia/utils.hxx +++ b/vcl/inc/skia/utils.hxx @@ -207,7 +207,9 @@ inline SkSamplingOptions makeSamplingOptions(BmpScaleFlag scalingType, const Siz if (srcSize.Width() / destSize.Width() >= downscaleRatioThreshold || srcSize.Height() / destSize.Height() >= downscaleRatioThreshold) return SkSamplingOptions(SkFilterMode::kLinear, SkMipmapMode::kLinear); - return SkSamplingOptions(SkCubicResampler::Mitchell()); + if (SkiaHelper::isAlphaMaskBlendingEnabled()) + return SkSamplingOptions(SkCubicResampler::Mitchell()); + return SkSamplingOptions(SkFilterMode::kNearest, SkMipmapMode::kNone); case BmpScaleFlag::Default: // As in the first overload, use kNearest. return SkSamplingOptions(SkFilterMode::kLinear, SkMipmapMode::kNearest);