337031 - svg opacity in translucent windows broken

Status: RESOLVED WORKSFORME

(Core :: SVG, defect)

Description

[Alex Fritze]

In translucent windows (background-color:transparent), SVG elements loose their alpha channel information when their opacity == 1. The result is that an opaque black svg element is displayed as fully transparent. For other opaque colors the blending is messed up. Opacity values < 1 work fine.

Looking widget/src/windows/nsWidget.cpp: nsWindow::mMemoryBits :
<rect fill="black" fill-opacity="1" .../> yields ARGB = 0 0 0 0 and not 255 0 0 0,
whereas
<rect fill="black" fill-opacity="0.9" .../> yields ARGB = 230 0 0 0 as expected.

Testcase coming up.

Comment 1

[Alex Fritze]

Attachment: testcase

Chromeless, transparent xul testcase. The upper part of the window should be black with full opacity, but is rendered fully transparent. The lower part should be black at 0.9 opacity and is rendered correctly.

Comment 2

[Alex Fritze]

Attachment: patch v1

So the problem here is that cairo_win32_surface_create initializes the surface's pixel format by querying the device caps for BITSPIXEL. This value doesn't necessarily reflect the pixel format of the memory bitmap selected into the DC.

This patch adds code to determine the pixel format from the DC's bitmap before querying the device caps. 

I don't know much about cairo, so I'm not sure if some of the mappings make sense (bmBitsPixel=16 -> CAIRO_FORMAT_RGB24). Also, the if-branch that queries for device caps might not be needed at all.

Comment 3

[Jonathan Watt [:jwatt]]

Attachment: patch v1 updated to TIP

Updated to TIP and changed to use the (nasty tab+space) indentation style used in the file.

I know that bug 343655 changed ARGB32 to RGB24 for GetDeviceCaps(hdc,BITSPIXEL)==32, but I've left Alex's addition as using ARGB32 because it doesn't fix the bug if changed to RGB24.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

What's the actual underlying bug here?  Is SVG creating its own HBITMAP/HDC and then using win32_create and passing in just the DC?  (Thus having the depth forced to RGB24, and meaning that BitBlt gets used instead of alpha blend, blowing away the alpha?)

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 236528 [details] [diff] [review]
patch v1 updated to TIP

No response to my earlier question...

>-    if (GetDeviceCaps(hdc, TECHNOLOGY) == DT_RASDISPLAY) {
>+    if ((hbitmap = GetCurrentObject (hdc, OBJ_BITMAP)) &&
>+	GetObject (hbitmap, sizeof(BITMAP), &bitmap)) {
>+	if (bitmap.bmBitsPixel == 32)
>+	    format = CAIRO_FORMAT_ARGB32;

This won't work; bmBitsPixel == 32 does not mean that this should be ARGB32, because we have no idea whether that extra byte is storing alpha information or if it is to be ignored.  If anything, the only thing we can do here is set CAIRO_FORMAT_RGB24, which should already be handled by the GetDeviceCaps(hdc, BITSPIXEL) case.

Comment 6

[Jonathan Watt [:jwatt]]

Comment on attachment 236528 [details] [diff] [review]
patch v1 updated to TIP

Yeah, while this patch (hack) previously enabled transparent windows, it interacts very badly with the new version of cairo we picked up in bug 354388. E.g. see . see https://bugzilla.mozilla.org/show_bug.cgi?id=355440#c5 and https://bugzilla.mozilla.org/show_bug.cgi?id=354388#c11 for two side effects.

I need to do more research on this before I can answer your question vlad. It's not high priority for me right now though, so that won't happen immediately.

Comment 7

[Robert Longson [:longsonr]]

Testcase works for me on current trunk.

Comment 8

[Jonathan Watt [:jwatt]]

Yup. Run as -chrome, the testcase works for me too.