The default bug view has changed. See this FAQ.

drawWindow() reverses red and blue channels in 16 bit color desktop

RESOLVED FIXED

Status

()

Core
Canvas: 2D
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: ted, Assigned: mcs)

Tracking

({fixed1.8.1, verified1.8.0.2})

Trunk
x86
Windows XP
fixed1.8.1, verified1.8.0.2
Points:
---
Bug Flags:
blocking1.8.0.2 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [rft-dl])

Attachments

(4 attachments)

(Reporter)

Description

12 years ago
Running WindowsXP SP2, with an ATI Mobility Fire GL T2, driver version
6.14.10.6512, 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.
(Reporter)

Comment 1

12 years ago
Rolled back to 6.14.10.6476, driver date 8/25/2004, still not working right.
(Reporter)

Comment 2

12 years ago
Now I can't reproduce this anymore.  There's something very strange about this.

Comment 3

12 years ago
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.

Comment 4

12 years ago
Alright, I'm using Firefox 1.5 final on Windows XP SP2 with an ATI Radeon 9800 Pro using drivers 6.14.10.6476, 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.

Comment 7

12 years ago
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.

Comment 8

12 years ago
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.

Comment 9

12 years ago
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]

Comment 10

12 years ago
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?

Comment 12

12 years ago
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?
> 

Comment 13

12 years ago
Anyone want to verify if their computer reproduces bug out in 16 color mode?
(Reporter)

Comment 14

12 years ago
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.
Summary: drawWindow() reverses red and blue channels → drawWindow() reverses red and blue channels in 16 bit color desktop

Comment 15

12 years ago
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 :)

Comment 18

11 years ago
I see this too, with an ATI 9700 Pro, Windows 2000. CATALYST version 05.13, 2D version 6.14.10.6587.

Comment 19

11 years ago
related to bug 293726? (Solaris bug where red and blue are swapped)
(Assignee)

Comment 20

11 years ago
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().
(Assignee)

Comment 21

11 years ago
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.
(Assignee)

Comment 22

11 years ago
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.
Attachment #209636 - Flags: superreview+
Attachment #209636 - Flags: review+
(Assignee)

Updated

11 years ago
Assignee: nobody → mcs
(Assignee)

Comment 24

11 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
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 26

11 years ago
Comment on attachment 209636 [details] [diff] [review]
possible fix

requesting that this patch land on various branches (only applies to canvas feature on Windows)
Attachment #209636 - Flags: approval1.8.1?
Attachment #209636 - Flags: approval1.8.0.2?
Attachment #209636 - Flags: approval1.8.0.1?
Comment on attachment 209636 [details] [diff] [review]
possible fix

1.8.0.1 is already gone
Attachment #209636 - Flags: approval1.8.0.1?
Attachment #209636 - Flags: approval1.8.1? → branch-1.8.1?(roc)
Attachment #209636 - Flags: approval-branch-1.8.1?(roc) → approval-branch-1.8.1+
(Assignee)

Comment 28

11 years ago
Thanks for the 1.8.1 approval.  Committed to MOZILLA_1_8_BRANCH:

mozilla/gfx/src/windows/nsDrawingSurfaceWin.cpp
new revision: 3.21.8.1; 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.
Keywords: fixed1.8.1
(Reporter)

Comment 29

11 years ago
This would be nice for 1.8.0.2, I consistently get mail from people using Tab Preview in 16 bit color seeing this bug.
(Assignee)

Comment 30

11 years ago
I'd also like to reinterate my strong belief that this fix should be approved for 1.8.0.2 (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
Attachment #209636 - Flags: approval1.8.0.2? → approval1.8.0.2+
Flags: blocking1.8.0.2+
(Assignee)

Comment 33

11 years ago
Thank you very much for the 1.8.0.2 approval!  Committed to MOZILLA_1_8_0_BRANCH:

mozilla/gfx/src/windows/nsDrawingSurfaceWin.cpp
new revision: 3.21.16.1; 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.
Keywords: fixed1.8.0.2
please attach a testcase to this bug
Whiteboard: [tcn-dl]
(Reporter)

Comment 35

11 years ago
Created attachment 213143 [details]
Simple drawWindow testcase

Testcase, requires signed.applets.codebase_principal_support set to true.  You should see two red boxes.
(Reporter)

Comment 36

11 years ago
Hopefully davel doesn't mind me twiddling his whiteboard flags...
Whiteboard: [tcn-dl] → [rft-dl]

Comment 37

11 years ago
v.fixed on 1.8.0 branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060308 Firefox/1.5.0.2, I see both red squares with the simple drawWindow testcase (did not see the first with Firefox 1.5.0.1).
Keywords: fixed1.8.0.2 → verified1.8.0.2

Comment 38

11 years ago
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 1.5.0.2. You should upgrade to that.

Comment 40

11 years ago
> (In reply to comment #38)
> > Will this be fixed in an upcoming Firefox 1.5.x version?
> 
> This should be fixed in firefox 1.5.0.2. You should upgrade to that.

I'm using Firefox 1.5.0.3

Comment 41

11 years ago
(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.
You need to log in before you can comment on or make changes to this bug.