Closed Bug 36694 Opened 20 years ago Closed 20 years ago

PNG Alpha Support not implemented on Windows

Categories

(Core Graveyard :: GFX, defect, P3)

x86
Windows 98
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: sicking, Assigned: roc)

References

()

Details

(Keywords: verifyme, Whiteboard: [nsbeta2-]fix in tree, windows only)

Attachments

(5 files)

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
Don, is this a blender problem? or does this belong to imagelib?
Assignee: kmcclusk → dcone
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
This may be a duplicate (or extension) of bug 19283.  Alpha blending didn't
exist when that bug was filed, however.
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.
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.
Please confirm whether VC++ 5 can compile, since I don't have it.
Yes, it compiles now.  Thanks.
Status: NEW → ASSIGNED
Target Milestone: --- → M17
don:
I think this one is mine. And perhaps a duplicate at that.
reassigning to self.
-p
Assignee: dcone → pnunn
Status: ASSIGNED → NEW
duplicate of #19283.

*** This bug has been marked as a duplicate of 19283 ***
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → DUPLICATE
I
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Summary: PNG not tansparent correctly on non-white backgrounds → banding problems with 8bit mask on windows.
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
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)?
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.
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
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.
Does anybody know when this patch might be checked in?
Don, could you look at the patch? This is in an area that you implemented.
Assignee: kmcclusk → dcone
Checking out the patch currently.. and if everything is ok, I will check in on 
m17.
Status: NEW → ASSIGNED
Whiteboard: fix in tree
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
Putting on [nsbeta2-] radar.  Not critical to beta2.
Whiteboard: fix in tree, windows only → [nsbeta2-]fix in tree, windows only
This really needs to get in.. this is why PNG was invented.  If not beta2 then 
beta 3 is really important...
Keywords: nsbeta2nsbeta3
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
Blocks: 8415
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
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.

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?
The patch will be checked in when the tree opens again.. this will make the 
final ship...
Thank you...
*** Bug 40916 has been marked as a duplicate of this bug. ***
Is this in? Tree is now open, check-em-in if you got 'em.
If it's in please mark "FIXED" and await confirmation.
No this is not in yet.. the tree has to be branched -- post beta 2 before I can 
check this in.
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.
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!
Keywords: patch
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.  
I have installed the latest patch and everything appears to be working 
perfectly.
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?
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.
Sorry, I'd rather not check it in. I'm afraid of my Mozilla CVS password being 
sniffed.
here is another test url donated by Paluba Michal <michal@sklopan.cz>
http://www.spsselib.hiedu.cz/~zaruba/Temp/AlphaBlended_PNG-test.html
-p
Robert, dmose@mozilla.org says he'll get you SSH cvs access tonight.

/be
Cool, thanks.

That test looks nice.
[Note to petersen or self --- when verifying this bug, be sure to check all test 
cases, including those in 40916.]
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 :-).
Assignee: dcone → roc+moz
Status: ASSIGNED → NEW
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.
Status: NEW → ASSIGNED
Robert: Looks good. - Kevin
Patch checked in. Thanks all!
Status: ASSIGNED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
Keywords: verifyme
Works for me as well... as of build 2000072008 (Win2K)

Thanks a million!!!!
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 → ---
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)
I cannot reproduce with NT4 SP5.
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.
I'm running 1024x768, and the circles are certainly being scrolled offscreen and 
back on.
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.
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. 
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.
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.
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. 
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.
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?
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
Do I need to get approval again?
Not really, a= still applies, along with the r= provided reviewers saw the code 
you intended to check in.

/be
I heard AlphaBlend() bug is fixed on Windows Me RC1, so I disabled it only on 
Winsows 98.
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.
Checked in VC5 build fix and patch to disable AlphaBlend() on Win98.
Status: REOPENED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
The w3 url looks fine to me. Win2k running at 32-bit colour.
Status: RESOLVED → VERIFIED
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 → ---
this space intentionally left blank.
Status: REOPENED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
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.
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
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
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.