19.73 KB, image/png
41.97 KB, image/png
1.33 KB, patch
|Details | Diff | Splinter Review|
799 bytes, text/html
Running WindowsXP SP2, with an ATI Mobility Fire GL T2, driver version 18.104.22.16812, driver date 02/01/2005. (Downloaded from Windows Update), drawWindow() swaps my red and blue channels. Tested on a VC6 nightly build and a VC71 self build. I am going to try rolling back the driver and see if that is indeed the problem. These same builds worked fine yesterday before updating my driver.
Rolled back to 22.214.171.12476, driver date 8/25/2004, still not working right.
Now I can't reproduce this anymore. There's something very strange about this.
I see this with my vc6 trunk self-build, running over Remote Desktop in 16-bit colour. Don't ask me what happens if I run it locally, I've never tried.
Alright, I'm using Firefox 1.5 final on Windows XP SP2 with an ATI Radeon 9800 Pro using drivers 126.96.36.19976, dated 8/25/2004. I see this problem when I use Firefox over Remote Desktop. Specifically: When I start a FF1.5 browser session locally, I do not see this problem. When I log into my PC via Remote Desktop (checked on a variety of machines), I see the red and blue channel swap issue on the browser session that was left open and worked fine locally. If, via RDP, I exit that same browser session and start a new one, it too will have the problem. If I log back into my PC locally, with the browser session that I started via RDP still running, it will also continue to exhibit the issue. If I exit that session and start a new one while logged in locally, it will once again perform normally.
On these non-working configurations, does 'opacity' work?
I'm guessing that this has something to do with our canvas/image code assuming a certain RGB ordering for certain platforms, and windows might change that for RDP. Thus images that we render will still look correct (since we explicitly tell windows that they're BGR or whatever), but canvas could look swapped because we create an image and then change its pixel data via lockpixels and the like.
Created attachment 204920 [details] Tab preview of page with opacity over Remote Desktop This is a screenshot of a tab preview of a page using opacity seen over Remote Desktop.
Created attachment 204921 [details] The actual page with opacity over Remote Desktop This is a screenshot of the actual page that the previous tab preview screen shot is of, also over Remote Desktop, showing that the page is rendered correctly, except in the tab preview.
I'm experiencing the bug with Tab Preview - and I'm not using remote desktop. I'm on a win2k machine with an nvidia ti4200, just installed firefox 1.5 final, and the following extensions installed: DOM inspector talkback image zoom gmail notifier mozilla calender [disabled, incompatible with 1.5] bugmenot FLST viamatic tabnail[disabled since I found your extension] foobar controls forecastfox tabpreview adblock plus [which I disabled since it was the only thing I installed yesterday, but that didn't fix anything]
BTW, bug is persistent - started yesterday [I didn't notice the bug until I'd used the extension for a day, not 100% sure if it was active when installed], and continues today through multiple sessions + reboots, every preview page made.
(In reply to comment #10) > BTW, bug is persistent - started yesterday [I didn't notice the bug until I'd > used the extension for a day, not 100% sure if it was active when installed], > and continues today through multiple sessions + reboots, every preview page > made. Wow, that's just odd. What are your display resolution and depth settings?
1600x1200 high color (16 bit) Changing resolutions - no fix Fix found: Change to 32 bit color 256 and 16 bit color both produce bug in any resolution, 32 bit color eliminates bug. I've had 16 bit color on because my monitor doesn't seem to get any benefit or penalty from 32. (In reply to comment #11) > (In reply to comment #10) > > BTW, bug is persistent - started yesterday [I didn't notice the bug until I'd > > used the extension for a day, not 100% sure if it was active when installed], > > and continues today through multiple sessions + reboots, every preview page > > made. > > Wow, that's just odd. What are your display resolution and depth settings? >
Anyone want to verify if their computer reproduces bug out in 16 color mode?
This is very reproducible for me by switching to 16 bit color. In fact, if you have Firefox running, you can switch to 16 bit, reproduce the bug, switch back to 32 bit and not see it anymore. That must have been what happened to me when I originally reported the bug. I'd also be willing to bet that RDP uses 16 bit color all the time, since Neil said his is 16 bit in comment 3.
drawWindow() itself may be at fault here, not just the extension. Bug is reproduceable in Viamatic Tabnail and likely other extensions that use the command.
Vlad, can you take this on, since it's presumably windows-only? Or get Pav to take a look?
Yeah, I've been meaning to take a look; it's on my todo list. I have a theory, but we'll see :)
I see this too, with an ATI 9700 Pro, Windows 2000. CATALYST version 05.13, 2D version 188.8.131.5287.
related to bug 293726? (Solaris bug where red and blue are swapped)
I also see this with our Pearl Crescent Page Saver extension (http://pearlcrescent.com/products/pagesaver/). Tested on Windows XPsp2 with NVIDIA GeForce FX 5200 video. I see the same behavior with the Tab Sidebar extension; both extensions use .drawWindow().
Kathy Brade and I debugged this some today. So far we believe: - Inside nsCanvasRenderingContext2D::UpdateImageFrame(), the RGB values are correct. - Inside nsCanvasRenderingContext2D::DrawNativeSurfaces(), the red and blue components are swapped in the destination. I think the root of the problem is inside nsDrawingSurfaceWin::CreateBitmapInfo() where the pixel format is determined. Patch coming soon.
Created attachment 209636 [details] [diff] [review] possible fix This patch changes the pixel format on Windows when the video depth is less than 32 (swapping the red and green offsets), which seems to fix this bug. I may be confused by Microsoft's documentation, endianness, etc. but my reading of the description of the biBitCount=24 section on this page tells me the order is blue, green, and red: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/gdi/bitmaps_1rw2.asp I am a little worried this might break something else though.
Comment on attachment 209636 [details] [diff] [review] possible fix That seems to agree with the MSDN docs. Thanks for finding this. Kathleen can check this in.
Thanks for the review. I looked around and do not really see any other places in the tree where the pixel format is used. Fix committed to the trunk: mozilla/gfx/src/windows/nsDrawingSurfaceWin.cpp new revision: 3.22; previous revision: 3.21 Bug 298375 - drawWindow() reverses red and blue channels in 16 bit color. Return correct pixel format information for color depth < 32 on Windows. r+sr=roc. I am not sure what the criteria are for landing this on MOZILLA_1_8_BRANCH, but it seems like it should also land there.
Please request review for 1.8.1 at least, and possibly 1.8.0. This patch is trivial and I believe it's very low risk.
Comment on attachment 209636 [details] [diff] [review] possible fix requesting that this patch land on various branches (only applies to canvas feature on Windows)
Comment on attachment 209636 [details] [diff] [review] possible fix 184.108.40.206 is already gone
Thanks for the 1.8.1 approval. Committed to MOZILLA_1_8_BRANCH: mozilla/gfx/src/windows/nsDrawingSurfaceWin.cpp new revision: 220.127.116.11; previous revision: 3.21 Commit fix from the trunk (a=roc): Bug 298375 - drawWindow() reverses red and blue channels in 16 bit color. Return correct pixel format information for color depth < 32 on Windows. r+sr=roc.
This would be nice for 18.104.22.168, I consistently get mail from people using Tab Preview in 16 bit color seeing this bug.
I'd also like to reinterate my strong belief that this fix should be approved for 22.214.171.124 (so it is included in an upcoming Firefox 1.5.x release). Statistics I examined show that about 20% of requests on the web come from browsers that are running in an environment with less than 32 bit color depth (I don't know what percentage of those are from Windows, but probably most of them). That means all of those users will experience this bug. And a lot of the new Firefox 1.5 extensions rely on drawWindow(), so this is an important user satisfaction issue. Here are just a few of the extensions I know about that almost certainly use drawWindow() and are therefore affected by this bug: Firefox Showcase (an Extend Firefox contest finalist) Reveal (also a finalist) Viamatic foXpose (also a finalist) Tab Sidebar Tab Preview I am one of the authors of the Pearl Crescent Page Saver and have received quite a few bug reports from people who are running with 16-bit color depth (the most common choice aside from 32-bit). The other side of the coin is risk. This is a trivial patch. After studying the code and poking around in the tree, we determined that the only consumer of the pixel format (which is what this bug fixes) is the canvas drawWindow() code... so there is very little risk of unrelated regressions.
I agree that the risk is minimal.
Comment on attachment 209636 [details] [diff] [review] possible fix approved for 1.8.0 branch, a=dveditz for drivers
Thank you very much for the 126.96.36.199 approval! Committed to MOZILLA_1_8_0_BRANCH: mozilla/gfx/src/windows/nsDrawingSurfaceWin.cpp new revision: 188.8.131.52; previous revision: 3.21 Commit fix from the trunk (a=dveditz): Bug 298375 - drawWindow() reverses red and blue channels in 16 bit color. Return correct pixel format information for color depth < 32 on Windows. r+sr=roc.
please attach a testcase to this bug
Created attachment 213143 [details] Simple drawWindow testcase Testcase, requires signed.applets.codebase_principal_support set to true. You should see two red boxes.
Hopefully davel doesn't mind me twiddling his whiteboard flags...
v.fixed on 1.8.0 branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:184.108.40.206) Gecko/20060308 Firefox/220.127.116.11, I see both red squares with the simple drawWindow testcase (did not see the first with Firefox 18.104.22.168).
I'm experiencing the same problem on my MacBook Pro. Firefox 1.5 is running fine but I use "reveal" for tab-preview and the colors of the thumbs are wrong. In 16,7mio colors mode every thumb is very cyan and in 32768 colors mode the thumbs are magenta. Will this be fixed in an upcoming Firefox 1.5.x version?
(In reply to comment #38) > Will this be fixed in an upcoming Firefox 1.5.x version? This should be fixed in firefox 22.214.171.124. You should upgrade to that.
> (In reply to comment #38) > > Will this be fixed in an upcoming Firefox 1.5.x version? > > This should be fixed in firefox 126.96.36.199. You should upgrade to that. I'm using Firefox 188.8.131.52
(In reply to comment #38) > I'm experiencing the same problem on my MacBook Pro. Firefox 1.5 is running > fine but I use "reveal" for tab-preview and the colors of the thumbs are wrong. > In 16,7mio colors mode every thumb is very cyan and in 32768 colors mode the > thumbs are magenta. The Intel Mac problem is covered in bug 338062. Please post to that bug and ask that the fix be approved for the branches (so it will make it into an upcoming Firefox release). So far the fix has landed on the trunk only.