Closed
Bug 36694
Opened 24 years ago
Closed 24 years ago
PNG Alpha Support not implemented on Windows
Categories
(Core Graveyard :: GFX, defect, P3)
Tracking
(Not tracked)
VERIFIED
FIXED
M17
People
(Reporter: sicking, Assigned: roc)
References
()
Details
(Keywords: verifyme, Whiteboard: [nsbeta2-]fix in tree, windows only)
Attachments
(5 files)
14.18 KB,
patch
|
Details | Diff | Splinter Review | |
14.26 KB,
patch
|
Details | Diff | Splinter Review | |
14.60 KB,
patch
|
Details | Diff | Splinter Review | |
151.17 KB,
application/gzip
|
Details | |
1.14 KB,
patch
|
Details | Diff | Splinter Review |
transparent png images are not draw correctly on non-white backgrounds. This is tested on a build that did NOT have CAN_SUPPORT_8_BIT_ALPHA enabled. Transparency is however drawn correcly on text as seen on http://bugzilla.mozilla.org/showattachment.cgi?attach_id=2527 Build 2000-04-20-08 on Win98SE
Comment 1•24 years ago
|
||
Don, is this a blender problem? or does this belong to imagelib?
Assignee: kmcclusk → dcone
Comment 2•24 years ago
|
||
Ditto for today's nightly Win32 build, which presumably DOES have CAN_SUPPORT_8_BIT_ALPHA enabled (per Pam's final comments on bug 3013 yesterday afternoon). Rendering of http://www.cdrom.com/pub/png/pngs-img.html is screwed up in all of the transparent areas, regardless of whether the display is 16 or 24 bits deep, and even for the penguin image at the top (which has only GIF-like binary transparency). The IceAlpha and MagnoliaAlpha pages are likewise messed up--there's some correlation with the texture of the background image, but not with its colors. Cc'ing Pam since she did the Win32 testing.
Summary: Png not tansparent correctly on non-white backgronds → PNG not tansparent correctly on non-white backgrounds
Comment 3•24 years ago
|
||
This may be a duplicate (or extension) of bug 19283. Alpha blending didn't exist when that bug was filed, however.
Comment 4•24 years ago
|
||
See my comments at the bottom of bug 19283, I probably should have filed a new bug but new we have this one. The masks in nsImageWin.cpp don't work (both 1-bit and 8-bit). This is not a blender issue. The blender is currently only used for the opacity property, not per-pixel alpha. This is Windows specific. It is not a PNG decoder bug. Only if the color data for the transparent pixels of the image is black do 1-bit masks work (as it does in GIF). Over in 19283, I have a workaround patch that makes transparent pixels black for PNG to fix 1-bit. Strictly speaking, this is a bug it the Windows code. The reason it works on white backgrounds is because the OR raster op is responsible for this problem when OR is used on top of white, the pixel remains white. 8-bit needs to be correctly implemented on Windows, and this means custom code as no system functionality is available on all Windows versions exists to draw with an 8-bit mask. The GDK image implementation blends by hand, and something similar is needed on Windows.
Comment 5•24 years ago
|
||
Comment 6•24 years ago
|
||
Comment 7•24 years ago
|
||
I tried the first patch but I couldn't compile because I'm using VC++ 5, which probably has some older headers that don't define BLENDFUNC. I solved it by using conditional defines around the use of BLENDFUNC. I can compile the rest of mozilla, so I think there should be an option to compile without AlphaBlend support (or copy the definition of the structure so that VC++ 5 can build an AlphaBlend compatible version). Perhaps the conditional use could be tied to the WINVER constant. Is there a new one for Win98/Win2k? The patch looks great on my NT 4.0 system with 32-bit video.
Comment 8•24 years ago
|
||
Comment 9•24 years ago
|
||
Please confirm whether VC++ 5 can compile, since I don't have it.
Comment 10•24 years ago
|
||
Yes, it compiles now. Thanks.
Updated•24 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → M17
Comment 11•24 years ago
|
||
don: I think this one is mine. And perhaps a duplicate at that. reassigning to self. -p
Assignee: dcone → pnunn
Status: ASSIGNED → NEW
Comment 12•24 years ago
|
||
duplicate of #19283. *** This bug has been marked as a duplicate of 19283 ***
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → DUPLICATE
Summary: PNG not tansparent correctly on non-white backgrounds → banding problems with 8bit mask on windows.
Comment 14•24 years ago
|
||
I'm reopening bug. And I'm changing the Summary to 'banding problems with 8bit mask on windows'. I'm also attaching a test page that displays a bit more of the problem. Unfortunately a patch for this will need alot of testing on all platforms. What fixes one may break another. Thanks for keeping this bug alive, Chris. -P
Comment 15•24 years ago
|
||
Comment 16•24 years ago
|
||
The banding problem is Windows specific, right? It happens not because of the decoded PNG data or any XP issues, but because the nsImageWin implementation is broken. The patch from VYV03354@nifty.ne.jp only changes mozilla/gfx/src/windows/nsImageWin.h and mozilla/gfx/src/windows/nsImageWin.cpp It should not have any impact on other platforms. Or did you mean different Windows platforms (Win95, Win98, NT 4.0, Win2k)?
Comment 17•24 years ago
|
||
I don't see the banding problem on linux. I applied the patch and recompiled and things are still fine. This patch only changes files in mozilla/gfx/src/windows and seems unliekly to affect other platforms besides windows.
Comment 18•24 years ago
|
||
Kevin: I reassigning this to you since you are the module owner for gfx. The attached patch would need your blessing before it could be checked in. (I have not had time to test out the patch.) My take on the alphachannel banding problem on windows is due to incorrect values used as the 8 bit mask....maybe for some reason windows expands the grayscale values to 24 bit, but only uses the value on R and not G&B..or something like that. This is just a guess after looking at the test urls. -pn
Assignee: pnunn → kmcclusk
Status: REOPENED → NEW
Comment 19•24 years ago
|
||
The banding is because the 8-bit mask is combined with the destination using the SRCAND raster operation, as would be done with a 1-bit mask, before laying down the color data with SRCPAINT (and OR op). The mask must be used to blend the source color with the destination color, there is no simple raster op to blend. The patch looks for OS support for blending and uses it if present. If it is not present, it blends manually.
Comment 20•24 years ago
|
||
Does anybody know when this patch might be checked in?
Comment 21•24 years ago
|
||
Don, could you look at the patch? This is in an area that you implemented.
Assignee: kmcclusk → dcone
Comment 22•24 years ago
|
||
Checking out the patch currently.. and if everything is ok, I will check in on m17.
Status: NEW → ASSIGNED
Whiteboard: fix in tree
Updated•24 years ago
|
Keywords: nsbeta2
Summary: banding problems with 8bit mask on windows. → PNG Alpha Support not implemented on Windows
Whiteboard: fix in tree → fix in tree, windows only
Comment 23•24 years ago
|
||
Putting on [nsbeta2-] radar. Not critical to beta2.
Whiteboard: fix in tree, windows only → [nsbeta2-]fix in tree, windows only
Comment 24•24 years ago
|
||
This really needs to get in.. this is why PNG was invented. If not beta2 then beta 3 is really important...
Comment 25•24 years ago
|
||
Adding a simple testcase with "alpha balls" and R,G,B,Y,C,M,White backgrounds to help visualization: http://www.geckonnection.com/wed2/alphatest.html
Comment 26•24 years ago
|
||
Marcio's web server is misconfigured, so I'm not sure his test case is going to
be useful:
> HEAD http://www.geckonnection.com/wed2/images/binocle2.png
200 OK
Connection: close
Date: Thu, 22 Jun 2000 18:18:13 GMT
Accept-Ranges: bytes
Server: Apache/1.2.6 FrontPage/3.0.4
Content-Length: 10832
Content-Type: text/plain <-- should be image/png
ETag: "19add6-2a50-39484ff0"
Last-Modified: Thu, 15 Jun 2000 03:39:28 GMT
Client-Date: Thu, 22 Jun 2000 18:18:22 GMT
Client-Peer: 192.41.26.206:80
Also, unless I'm going blind, the test page has a useless SCRIPT tag in it with
no closing tag. Might be good to run it past the W3C validator...
Greg
Comment 27•24 years ago
|
||
I just fixed mime-type and <script> tag of testcase Greg, thanks! I just included this example page because it's easy to visualize a problem with blending in the png image that is over the pure-green background. Tested with: 20000623/Win98.
Comment 28•24 years ago
|
||
Recent discussion on n.p.m.seamonkey (well, a staircase thread between myself and Phil Peterson under the subject "Suggestion: moz-help keyword...") suggests that just because the bug is nsbeta2- does not mean that the patch can't be checked in (although it is, of course, subject to code review and approval by the module owner). It doesn't sound like it would be a "destabilizing" patch. Judging by the number of cc's and votes on this bug, quite a lot of people would like to see it fixed... is there any chance that the patch could be checked in soon?
Comment 29•24 years ago
|
||
The patch will be checked in when the tree opens again.. this will make the final ship...
Comment 30•24 years ago
|
||
Thank you...
Comment 31•24 years ago
|
||
*** Bug 40916 has been marked as a duplicate of this bug. ***
Comment 32•24 years ago
|
||
Is this in? Tree is now open, check-em-in if you got 'em. If it's in please mark "FIXED" and await confirmation.
Comment 33•24 years ago
|
||
No this is not in yet.. the tree has to be branched -- post beta 2 before I can check this in.
Comment 34•24 years ago
|
||
Post beta2? Is it that unstable? PNGs really can't be used for skins if this isn't fixed. And aren't skins a key point of the second beta? You would think that the powers that be would be a bit sensible regarding this.
Comment 35•24 years ago
|
||
According to Phil Peterson on n.p.m.seamonkey, you don't *have* to wait until after beta2 branches to put this in, since the patch was written by an external contributor. Please add the "patch" keyword to this bug as it contains an externally contributed patch. I just tried but I don't have the permissions. The rules for checkins of external contributor's patches are different than those for NS employees - "code review by module owner, approval by Waterson or Brendan". And NS employees *are* allowed to perform the checkin subject to these rules, if the patch was written by an external contributor. Please try to get the patch in by *these* rules instead of the more restrictive rules for patches by NS employees. Phil just posted a message in n.p.m.seamonkey specifically addressing this issue (subject: bugs with PATCH keyword). If you have any other reason for delaying this then that's a different issue. But I don't think you "have to" delay it. Thanks!
Comment 36•24 years ago
|
||
I would rather an external contributor check this in if thats the route you want to go. There are issues with this loophole.. and I really don't want to explore these so close to beta.
Assignee | ||
Comment 37•24 years ago
|
||
I have installed the latest patch and everything appears to be working perfectly.
Comment 38•24 years ago
|
||
can you check the patch in.. get Kevin McClusky to review.. or tell him I (Don Cone reviewed it).. then ask for Mozilla.org permission to check in?
Comment 39•24 years ago
|
||
There is no "loophole". If you, dcone, can argue convincingly that there is too much risk in taking this particular patch (not just any patch) right now, please do so. You have final say as module owner. On the other hand, if you would rather delegate the checkin to roc+moz@cs.cmu.edu so he takes cvsblame, that's fine too. What's not fine is to default control of the Mozilla open source project's CVS trunk to netscape.com exclusively. Sorry if I'm preaching to the choir. Robert, are you ready to check in? Has kmmclusk given an r= (not clear whether that's necessary given dcone's r=)? Should you assign this bug to yourself, since you're checking in the fix? Anyway, a=brendan@mozilla.org.
Assignee | ||
Comment 40•24 years ago
|
||
Sorry, I'd rather not check it in. I'm afraid of my Mozilla CVS password being sniffed.
Comment 41•24 years ago
|
||
here is another test url donated by Paluba Michal <michal@sklopan.cz> http://www.spsselib.hiedu.cz/~zaruba/Temp/AlphaBlended_PNG-test.html -p
Comment 42•24 years ago
|
||
Robert, dmose@mozilla.org says he'll get you SSH cvs access tonight. /be
Assignee | ||
Comment 43•24 years ago
|
||
Cool, thanks. That test looks nice.
Comment 44•24 years ago
|
||
[Note to petersen or self --- when verifying this bug, be sure to check all test cases, including those in 40916.]
Assignee | ||
Comment 45•24 years ago
|
||
OK, I am now fully CVS and SSH enabled (thanks dmose) :-). Kevin, please review the patch: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=7864 Checkin has already been approved by mozilla.org. When Kevin says OK, I will check in. This bug can be assigned to me if dcone is tired of it :-).
Updated•24 years ago
|
Assignee: dcone → roc+moz
Status: ASSIGNED → NEW
Comment 46•24 years ago
|
||
Kevin.. I have been running with this patch for a while.. with no problems. Reassigning to Robert so he can check this in if Kevin approves.
Assignee | ||
Updated•24 years ago
|
Status: NEW → ASSIGNED
Comment 47•24 years ago
|
||
Robert: Looks good. - Kevin
Assignee | ||
Comment 48•24 years ago
|
||
Patch checked in. Thanks all!
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Comment 49•24 years ago
|
||
Works for me as well... as of build 2000072008 (Win2K) Thanks a million!!!!
Comment 50•24 years ago
|
||
Visit this test URL mentioned in the comments: http://www.geckonnection.com/wed2/alphatest.html Scroll down and then back up. On Win98, 7-20-00 build, the top 3, and occasionally 4 "circles" will become garbled. Now, switch focus to another app that covers the garbled circles. Switch back to Mozilla. The circles now display correctly. Is this a problem with the PNG rendering, or a more serious problem affecting all images? I don't notice this on any other sites with transparent GIFs.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 51•24 years ago
|
||
I cannot duplicate the results mentioned by Chris Nelson on 2000-07-20 at 12:31. Works flawlessly for me. All repaints and scrolling actions do not produce the "garbled" result. (Win2K, 1600x1200x32b)
Assignee | ||
Comment 52•24 years ago
|
||
I cannot reproduce with NT4 SP5.
Comment 53•24 years ago
|
||
To: those who can't reproduce the problem I mentioned above... Are the circles in question scrolling offscreen? My screensize is 1024x768, the top 4 circles scroll offscreen, and they're the ones that become garbled.
Assignee | ||
Comment 54•24 years ago
|
||
I'm running 1024x768, and the circles are certainly being scrolled offscreen and back on.
Comment 55•24 years ago
|
||
Tested with build 2000072008 On Win 98 I can reproduce this on Win 98. Win 9x only? Res is 1024 x 768. When I scroll all the way down the top 4 images scroll off the top and are garbled when I scroll back down. The 4th image scrolls off the bottom when I move the scroll bar all the way to the top. scrolling back down the 4th image is ungarbled when it is scrolled back from the into view from the bottom.
Comment 56•24 years ago
|
||
Farther tests. I tried lower resoulations and less than full screen size so I could scroll more images off the top and bottom of the window. All images are broken when scrolled off the top of the window. All images are fixed if they are scrolled off the bottom, then scrolled back into view, or if they are hidden by another ap then brought back into view.
Comment 57•24 years ago
|
||
The problem mentioned above occurs on: 2000072008 nightly build on Windows 98 (1024x480). Doesn't occur on: 2000072008 nightly build on Windows 2000 (1024x768). Probably this is specific to Win98.
Comment 58•24 years ago
|
||
http://www.geckonnection.com/wed2/alphatest.html Another twist. If you see the garbled images described in my previous test, clicking on the garbled image once it is fully on the screen will fix it properly -- it will be re-rendered and appear in the proper position. However, if you click on the garbled image while it is partially scrolled off the top of the viewport, it is re-rendered, however, it is not rendered in the proper position. Mozilla acts as if the top of the viewport is the proper top of the image (even though the "proper" top of the image is scrolled out of view), and renders the image there, cropping the bottom of the image at its "proper" bottom. John Dobbins, you were able to reproduce the initial problem. Maybe you could confirm this and describe it better.
Comment 59•24 years ago
|
||
Confirming Chris' last obervation. In addition I noticed that if 3 or more images are garbled sometime clicking on one will cause 2 of them to repaint. I could not find any pattern. Killed the cash and this disapeared. only 1 image is "fixed" with no mem or disk cash. seems to be a repaint problem with top of window on Win 9.x . Can someone test this on Win95 to see if this is a general 9.x problem or it's Win 98 only.
Comment 60•24 years ago
|
||
Another test. To check if this problem existed before fix was checked in and just not noticed. I tried build 20001910. There is no sign of this problem in yesterday's build.
Comment 61•24 years ago
|
||
This is a AlphaBlend() implementation bug on Windows 98. We can fix this by disabling AlphaBlend() on Win98. Windows 95/NT4 have no problem since their system have no AlphaBlend() support. I confirmed this problem doesn't occur on Windows 95 (res: 800x600). Can anyone test to see if this is fixed on Windows Me?
Assignee | ||
Comment 62•24 years ago
|
||
Ooops, I checked in the wrong version of the patch (minus the VC5 compatibility code). I will check in the VC5 compatibility code when the tree opens. It's very minor and will not affect VC6 builds: #if !defined(AC_SRC_OVER) #define AC_SRC_OVER 0x00 #define AC_SRC_ALPHA 0x01 #pragma pack(1) typedef struct { BYTE BlendOp; BYTE BlendFlags; BYTE SourceConstantAlpha; BYTE AlphaFormat; }BLENDFUNCTION; #pragma pack() #endif
Assignee | ||
Comment 63•24 years ago
|
||
Do I need to get approval again?
Comment 64•24 years ago
|
||
Not really, a= still applies, along with the r= provided reviewers saw the code you intended to check in. /be
Comment 65•24 years ago
|
||
Comment 66•24 years ago
|
||
I heard AlphaBlend() bug is fixed on Windows Me RC1, so I disabled it only on Winsows 98.
Assignee | ||
Comment 67•24 years ago
|
||
OK, Kevin and Brendan, can I get r= and a= to check in http://bugzilla.mozilla.org/showattachment.cgi?attach_id=11754 and the VC5 compatibility snippet? Thanks.
Comment 68•24 years ago
|
||
Reviewed http://bugzilla.mozilla.org/showattachment.cgi?attach_id=11754 - looks good.
Comment 69•24 years ago
|
||
a=brendan@mozilla.org. /be
Assignee | ||
Comment 70•24 years ago
|
||
Checked in VC5 build fix and patch to disable AlphaBlend() on Win98.
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Comment 71•24 years ago
|
||
The w3 url looks fine to me. Win2k running at 32-bit colour.
Status: RESOLVED → VERIFIED
Comment 72•24 years ago
|
||
Could this possibly be also checked on Win98 prior to formally marking Verified? (I'll volunteer if nobody would like to.)
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Comment 73•24 years ago
|
||
this space intentionally left blank.
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Comment 74•24 years ago
|
||
Folks, this is just to let you know: Maybe not related with this but I just filed today a bug:46607 PNG+alpha images inside DIV areas (with opacity 0<value<1 ) results in rendering problems or even a crash.
Reporter | ||
Comment 75•24 years ago
|
||
Verifying for Win98SE where the AlphaBlend() bug existed before the fix so I'm verifying the bug. Thank you guys, this is Beauuuuuuuuuuuuutiful =). Really love the png-over-text testcase, cool to resize the window and se stuff moving around below the png.
Status: RESOLVED → VERIFIED
Comment 76•24 years ago
|
||
Adding keyword to bugs which already show a nsbeta2 triage value in the status whiteboard so the queries don't get screwed up.
Keywords: nsbeta2
Updated•15 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•