Closed Bug 297563 Opened 20 years ago Closed 19 years ago

Transparency doesn't seem to work in 16bit color depth

Categories

(Core :: XUL, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: martijn.martijn, Assigned: jonitis)

References

Details

(Keywords: fixed1.8)

Attachments

(2 files, 2 obsolete files)

I've tried the testcases in bug 113232. They don't work for me in 16 bit color depth, but when I set the video card to 32 bit color depth, they do work for me (after that I can reset to 16 bit color depth and then it still works). The result is that I don't see anything, while in 32 bit color depth, I can see the transparent demo. I'm using WinXP, NVidia GMX200 32MB.
SetDIBits call in nsWindow::UpdateTranslucentWindow fails in 16bit mode. Investigating.
Ere, Tim, Arron, Any ideas why ::SetDIBits fails in 16-bit mode, but works fine in 32bit mode? SDK documentation says that bitmap should not be selected in any device context, but moving ::SelectObject(hMemoryDC, hAlphaBitmap); line after ::SetDIBits does not help. The problem is same both on ATI and nVidia cards, so this is not a video driver problem.
UpdateTranslucentWindow() is using ::CreateCompatibleDC, which means no alpha information will be stored in our bitmap unless the display mode is precisely 32-bit. We have to use CreateDC instead since we need a 32-bit DC for storing 32-bit values in our bitmap.
I had this thought, too. But then ::UpdateLayeredWindow fails.
I replaced the CreateBitmap/SetDIBits with CreateDIBSection. It improves things slightly, but it is still buggy: 1. Animated octopus displays only the black animated outline, everything else is transparent 2. The gradient square has some black rectangles by right side 3. Buttons are drawn corrupted - mostly transparent 4. Simple animated gif from 3 pixels with R,G & B works fine, though. Could be some rounding errors during color conversion from 16-bit compatible bitmap to 32-bit one in GetDIBits? I am out of ideas what exactly causes these problems :( The bug 228399 mentioned some hardware/driver specific bugs with 16-bit display modes. Anyone remembers what exactly was screwed up? I guess I will try to not use the compatible bitmap for memory backstore, but always enforce at least 24 bits. How significantly this will reduce performance? Have you any better suggestions? Thanx
Attached patch Fix for 16-bit color modes (obsolete) (deleted) — Splinter Review
This patch like in bug 228399 enforces at least 24bit device independent memory bitmaps to draw into. If dispaly is in 32bit color mode then no conversion is required and we can directly access RGBA memory bitmap bits to prmultiply color values with alpha. If display is in 24bit or less color mode then we create temporary 32bit RGBA bitmap to use it in call to UpdateLayeredWindow.
Assignee: nobody → Dainis_Jonitis
Attachment #186926 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #187203 - Flags: superreview?(roc)
Attachment #187203 - Flags: review?(emaijala)
Would be nice to have this working correctly in all video modes in upcoming Firefox 1.1
Flags: blocking1.8b3?
Flags: blocking-aviary1.1?
I must be missing something, but where do we clean up mMemoryDC, mMemoryBitmap and mMemoryBits? Also we need some comments in nsWindow.h stating what these are for.
mMemoryDC and mMemoryBitmap GDI handles are released in SetupTranslucentWindowMemoryBitmap (PR_FALSE), which is called from Destroy (). There are no GDI object or memory leaks. Only the mMemoryBits variable was added in this patch. It is allocated and controlled by GDI subsystem after a call to CreateDIBSection. This memory goes away after call to DeleteObject(w2k.mMemoryBitmap) in SetupTranslucentWindowMemoryBitmap (). I am not sure that I can in a brief comment say anything better than "mMemoryBitmap is a memory bitmap" :) I think I used enough comments (compared to others, of course) in fix for bug 252067 and in this bug I have not introduced anything new.
Flags: blocking1.8b4+
Flags: blocking1.8b3?
Flags: blocking1.8b3-
Flags: blocking-aviary1.1?
Whiteboard: needs review ere+roc
Attachment #187203 - Flags: superreview?(roc) → superreview+
Microsoft engineer Lou Amadio was kind enough to investigate the SetDIBits problem and document the GDI alpha related limitations in his blog. Thank you very much for being helpful, Lou! Here is the qoute: "It turns out that SetDIBits doesn't do exactly what you think it should" And here is the blog entry about alpha aware GDI functions: "http://ooeygui.typepad.com/ooey_gui/2005/07/gdi_is_to_alpha.html"
Comment on attachment 187203 [details] [diff] [review] Fix for 16-bit color modes After a reminder I noticed I've already reviewed this but forgot to add r+. Sorry about that.
Attachment #187203 - Flags: review?(emaijala) → review+
Does the patch on this bug need an approval request?
Yes, this bug needs approval request. It is very low risk - affects only Win32 new feature that was not present in previous version of Firefox and only in 65536 color video mode. Please also consider bug 300297 for 1.8b4, which is also the low risk and also fixes the transparency problem on Win32, but can cause undesirable effect that your parent window disappears. I do not have access rights to check this in, so someone else should help with getting all approvals and checking into the tree.
Whiteboard: needs review ere+roc
You need to set the approval1.8b4 ? flag on the patch, not?
Comment on attachment 187203 [details] [diff] [review] Fix for 16-bit color modes asking for approval
Attachment #187203 - Flags: approval1.8b4?
Attachment #187203 - Flags: approval1.8b4? → approval1.8b4+
Whiteboard: [needs checkin][has patch with r/sr/a]
Comment on attachment 187203 [details] [diff] [review] Fix for 16-bit color modes I couldn't apply the patch cleanly to the current cvs tip in branch or trunk. Could you create a new diff against the latest versions, please?
Updated the patch to the latest trunk to avoid merge conflicts. I also took an opportunity to replace tab symbols with spaces.
Attachment #187203 - Attachment is obsolete: true
This is for MOZILLA_1_8_BRANCH (without any whitespace changes like in trunk version).
Fixes checked into trunk and branch.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Whiteboard: [needs checkin][has patch with r/sr/a]
Keywords: fixed1.8
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: