137295 - Gaps appear in gif images when not using back buffer on OS X

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Description

[Chris Petersen]

Build: 2002-04-12-08
Platform: OS X
Expected Results: Maximizing window should not cause painting issue with some of
the gif images.

What I got:
When the window is maximized (so no horizontal scrollbar is present), gaps
appear in the some of the gif images. I first thought this was a dup of
http://bugzilla.mozilla.org/show_bug.cgi?id=93170. 

However, no embedded content like flash, QT, or applets are present on this
page. Also, this is a regression starting with the 2002-04-12-08 build since it
works fine with the 2002-04-12-03 build.

Comment 1

[Chris Petersen]

This problem seems to only occur under Millions of color video setting. 

Comment 2

[Chris Petersen]

Attachment: screenshot of problem

Three images on page should this problem.

Comment 3

[Steve Dagley]

Didn't pav land some image code about the time this regressed?  I could swear I
saw chatter about this on IRC but I wasn't logging so I can't go back and verify.

Comment 4

[Steve Dagley]

BTW, Chris, is this a trunk or Moz1.0 branch build?

Being an equal opportunity finger pointer I'll also mention the trunk build on
this date would have pink's change to always turn off double buffering whenever
running on Mac OS X.  That makes me suspect this _is_ related to #93170 as we
would previously also turn off double buffering for pages that contained a plugin.

Comment 5

[Steve Dagley]

Sho'nuff, it was pink's checkin that exposed this (sorry pav).  This means 93170
isn't a plug-in problem, it's a problem with images in general when double
buffering is turned off on Mac OS X and we're in millions of colors mode.

Comment 6

[Chris Petersen]

I have a branch build from April 11th (2002-04-11-17) which doesn't seem to have
this problem. The trunk builds (2002-04-12-08) and (2002-04-13-08) have
displayed this issue as described.

Comment 7

[Peter Lubczynski]

Interesting....turning off double buffering causes these gaps...that explains
why pages with plugins were seeing this.

Comment 8

[Simon Fraser [no longer active]]

I'll take this.

Comment 9

[Simon Fraser [no longer active]]

Attachment: Testcase: a page with an image and an <iframe>

This problem only happens when there is an <iframe> (or possibly some other
element that has its own widget) on the page. That's why it doesn't happen on
every page with just images.

Comment 10

[Simon Fraser [no longer active]]

Attachment: Testcase with transparent and animated GIF

This testcase shows that image transparency matters, and that non-transparent
animated images are also messed up.

Comment 11

[Simon Fraser [no longer active]]

Attachment: nsImageMac patch

This patch works around the bug, which appears to be an OS bug, by passing the
current clip region to CopyDeepMask, and forwarding all masked-image draws
through CopyDeepMask. We have do to this because CopyMask exhibits the bug, but
does not take a maskRgn parameter that we can use to work around it.
Performance should be roughly the same.

I also put in some const goodness in the CopyBitsWithMask() method so that the
params match those taken by CopyBits/CopyDeepMask, and left some useful
debugging code #if 0'd in.

Comment 12

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 79340 [details] [diff] [review]
nsImageMac patch

r=pink

Comment 13

[Peter Lubczynski]

*** Bug 93170 has been marked as a duplicate of this bug. ***

Comment 14

[Patrick C. Beard]

Comment on attachment 79340 [details] [diff] [review]
nsImageMac patch

sr=beard
Excellent work.

Comment 15

[Simon Fraser [no longer active]]

Fix checked into the trunk. Want this on the branch too.

Comment 16

[Michael Buckland]

chris, can you test this on the trunk and update the bug with your results?

Comment 17

[Chris Petersen]

With the April 17th build (2002-04-17-03), the test cases render correctly
without gaps. But I now I see other issues at netscape.com. Some of the animated
gif banner ads only are drawn partially. 

Comment 18

[Chris Petersen]

Attachment: screen shot of animated gif issue

Only portion of animated gif is painted

Comment 19

[scottputterman]

Please address the regression before we consider this for approval on the branch.

Comment 20

[Chris Petersen]

After further testing using April 17th (2002-04-17-03) , I still see the gap
issue on gifs. However problem appears to be more random than before and can be
fixed by resizing window.

Our netscape site can should the gap when doing the following steps:

* First, delete cache so that the images are read from disk

1) Launch app and go to www.netscape.com
2) Notice house picture under Business/Finance section renders without gap
3) Now, open Mail window so that browser window is no longer top -most window
4) After mail window is opened, close it.
5) The house image on page now appears with gap (at least for me).
6) Resizing window with fix problem though.

Comment 21

[Chris Petersen]

I'm also seeing this randomly with site's using plugins too of course..

http://www.shockwave.com/sw/games/

Comment 22

[Simon Fraser [no longer active]]

Problems with fix, reopening.

Comment 23

[Jaime Rodriguez, Jr.]

Removing adt1.0.0 as there were problems with the fix, and this bug was reopned
for the trunk. This is a good one to fix though, and we'd like it before Friday
(if possible). Pls add your best estimate on when this can be fixed to Status
Whiteboard.

Comment 24

[Simon Fraser [no longer active]]

Attachment: New patch fixes animated GIFs issue

The reason this regressed is that CopyBitsWithMask(), which I changed, is also
called for animated GIFs to copy between frames (when you don't want any
clipping). This patch conditionalizes the previous patch to only apply when
we're drawing to the window (when we need it), not to another image. I've
surfed around with this patch, and didn't see any image problems.

Comment 25

[Simon Fraser [no longer active]]

Attachment: New version of patch fixes scoping problem.

Pinkerton tested the previous patch, and said that everything looked good. This
version is the same, but fixes an issue with scoping the StRegionFromPool.

Comment 26

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 80078 [details] [diff] [review]
New version of patch fixes scoping problem.

r=pink, as promised.

Comment 27

[Patrick C. Beard]

Comment on attachment 80078 [details] [diff] [review]
New version of patch fixes scoping problem.

sr=beard

Comment 28

[Simon Fraser [no longer active]]

Patch checked in on the trunk. Putting adt1.0.0 for branch consideration, and
reassigning to pinkerton, because I'm going on sabbatical (sorry pink).

Comment 29

[Simon Fraser [no longer active]]

Now marking fixed.

Comment 30

[Chris Petersen]

Wonderful ! This hideous bug is now fixed ... Marking verified on OS X trunk
(2002-04-22-03). Thanks , Simon !!

Comment 31

[Chris Petersen]

Oops. Maybe this isn't quiet fixed like I thought. All of my tests worked fine
though. However, I still see problem with the image (under business and finance)
when sidebar opens.

In millions of colors, 

1) Go to netscape.com 

2) Open the sidebar.

3) Notice gap on this image (under business and finance).

Comment 32

[Chris Petersen]

Reopening for now.

Comment 33

[Chris Petersen]

Same problem with top gif logo on this page (whichs uses an applet). Opening
sidebar displays gap.


http://pbskids.org/jayjay/colr.paintbox.herky.html

Comment 34

[Mike Pinkerton (not reading bugmail)]

does this only happen in millions of colors? if so, it's an os bug and the patch
at least fixes a bunch of cases, so we should take it.

if it also happens in thousands, we should dig more.

Comment 35

[Chris Petersen]

This problem doesn't occur in thousands, only millions. I agree that the fix
does resolve the tests I checked. I was just surprised to see it again when
opening the sidebar. Re-sizing the window does paint the image correctly. Maybe
there should be something mentioned in the read me documentation. 

Comment 36

[Chris Petersen]

I see this problem at http://autotrader.com/. Simply by placing focus in the
select menu causes gaps in numerous images on page. Tested with build 2002-04-22-03.

Comment 37

[Mike Pinkerton (not reading bugmail)]

cc'ing some other mac folk.

simon and i think that the patch, as is, is still a big improvement and deserves
to go onto the branch. if there are still some lingering edge-cases that freak
out only in millions of colors, they shouldn't hold back a patch that fixes 95%.
As far as we can tell, this is an OS bug that (reportedly) has been addressed
for an upcoming version of MacOS X by apple.

I say fixed, let's get this on the branch.

Comment 38

[Chris Petersen]

Ok, that's reasonable since this issue is in a milder form now. I would like to
recommend adding this to the read me notes. Marking verified in the OS X April
22 trunk (2002-04-22-03) build. Tested under OS X 10.1.4.

Comment 39

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 80078 [details] [diff] [review]
New version of patch fixes scoping problem.

a=dbaron for branch checkin

Comment 40

[scottputterman]

adding adt1.0.0+.  Please check this into the branch as soon as possible and add
the fixed1.0.0 keyword.  Please open a bug for the remaining issues.

Comment 41

[Mike Pinkerton (not reading bugmail)]

landed on branch.

i still see issues with images when clicking in form controls, but oh well.

Comment 42

[Bruce Davidson]

*** Bug 139486 has been marked as a duplicate of this bug. ***

Comment 43

[Chris Petersen]

Verified on OS X 2002-04-26-05 branch. Tested under OS X 10.1.4 in millions of
colors.

Comment 44

[Bruce Davidson]

*** Bug 141658 has been marked as a duplicate of this bug. ***

Comment 45

[Greg K.]

*** Bug 139486 has been marked as a duplicate of this bug. ***

Comment 46

[Chris Petersen]

*** Bug 142691 has been marked as a duplicate of this bug. ***

Comment 47

[Greg K.]

Reopening since bug 142691, just duplicated to this one, was confirmed against
FizzillaCFM/2002061014.

Comment 48

[Greg K.]

Still happens frequently when using 1.1 to access <http://www.bayarea.com/>.

Comment 49

[Greg K.]

Re: comment 37, what was the OS X version that fixes this bug?

Comment 50

[Steve Dagley]

I believe the implication from Apple was that it would be fixed in Jaguar.

Comment 51

[Jaime Rodriguez, Jr.]

Removing, verified1.0.0, as this bug has been reopened.

Comment 52

[Simon Fraser [no longer active]]

Can we get a screenshot from 10.2 with a recent build?

Comment 53

[Jaime Rodriguez, Jr.]

petersen: can you pls verify that this is fixed when using jaguar? thanks!

Comment 54

[Chris Petersen]

Yes, this problem is still occuring in 10.2.1 and 10.2 running in million of
colors (video setting). Autotrader.com is one of the sites where I still see
this problem.

http://autotrader.com/

Go to url and place focus on the Select make menu. Gaps should appear on the
various images on the page.

Comment 55

[Chris Petersen]

Tested with the 2002-09-19-05 trunk under 10.2 and 10.2.1. 

Comment 56

[Chris Petersen]

Attachment: Screen shot of autotrader.com (with gaps)

Comment 57

[Chris Petersen]

Another site that shows this problem was http://www.tvguide.com/. This page uses
a flash based ad which is probably triggering the problem.

Comment 58

[Chris Petersen]

Attachment: Screen shot of TV guide

Comment 59

[Chris Petersen]

Comment 54 should have mentioned the following:

Clicking on the Select Make field will display it's popup menu. Notice now that
some of the images on the page appears with a gap.

Comment 60

[Teruko Kobayashi]

Nominating this to edt1.0.2.

I can reproduce this in www.aol.de with embedding product on MacOS X 10.1.5 and
10.2.  When I go to www.aol.de, "Community" "Shopping" or other words are
truncated. I tried this with Netscape 7.0 and PPembed build, but I have not been
able to reproduce this.  

However, I could reproduce this in www.tvguilde.com with Netscape branch build. 

Comment 61

[[e-mail] (away - not reading bugmail)]

Attachment: screenshot of aol.de

Comment 62

[Jaime Rodriguez, Jr.]

Removing edt1.0.2, as their is no patch to evaluate for checkin approval to the
1.0 branch. Adding topembed+ as this is an issue for a major embedding customer.

Comment 63

[Simon Fraser [no longer active]]

This bug happens when there are plugins, or an <iframe>, on the page. I'm trying
to get a reduced testcase that shows the problem. I've found that behaviour
differs between mozilla and PPEmebed; it only shows up in Mozilla when the
browser window status bar is visible (go figga). It has to do with clipping in
the destination port when the image is redrawn.

Comment 64

[Simon Fraser [no longer active]]

Attachment: nsImageMac patch that fixes the bug.

Some analysis suggests that the bug (which is an OS bug in CopyDeepMask)
happens when the clip region of the destination port is complex (possible
having 'holes' in). This patch works around the bug by temporarily setting the
port clip region to the intersection of the the original clip region, and the
destination rectangle of the the image being drawn. This fixes the bug in the
test cases that I've tried.

Comment 65

[Steve Dagley]

Comment on attachment 99942 [details] [diff] [review]
nsImageMac patch that fixes the bug.

r=sdagley (sure, regions _sound_ like a good idea... :-)

Comment 66

[Scott Collins]

Comment on attachment 99942 [details] [diff] [review]
nsImageMac patch that fixes the bug.

sr=scc

Comment 67

[Jaime Rodriguez, Jr.]

Nominating as adt1.0.2, as this is a very issue on a top100 site.

Comment 68

[Jaime Rodriguez, Jr.]

um ... meant "very visible issue".

Comment 69

[Simon Fraser [no longer active]]

Taking.

Comment 70

[Simon Fraser [no longer active]]

Fix checked in to the trunk.

Comment 71

[Jaime Rodriguez, Jr.]

petersen: can you pls verify this as fixed on tomorrow's trunk builds? thanks!

Comment 72

[bobj]

A bit late, but just for completeness:The description of Vantive 318061 is
following.

- start an fresh installed client
- Sign on
- open www.aol.de

*** Result
the site is not displayed correctly => The icons for "eMail", "SMS", "Boards"
and "Die Woche" are truncated and some captions are incomplete e.g. "community"
below the icons.
If you close and open the site several times, the site will be displayed
properly till you empty the browser cache.

*** Expected
the site should be always displayed properly

Comment 73

[Simon Fraser [no longer active]]

Fixed on the trunk; fix is in the 20020920 build.

Comment 74

[Chris Petersen]

With the 2002-09-20-08 OS X trunk build, I can't reproduce the problem on the
site's mentioned (autotrader.com ,tvguide.com) earlier. I no longer see this
problem on other sites like espn.com too. Tested with 10.2.1 under Millions of
Colors video setting. Thanks Simon !!

Comment 75

[Simon Fraser [no longer active]]

The patch actually may cause some over-drawing of images; I missed interesecting
the clip region, and the image bounds. I'll have to redo the fix.

Comment 76

[Simon Fraser [no longer active]]

Attachment: Better fix that intersects with existing clip region

Comment 77

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 100004 [details] [diff] [review]
Better fix that intersects with existing clip region

r=pink.

Comment 78

[Patrick C. Beard]

Comment on attachment 100004 [details] [diff] [review]
Better fix that intersects with existing clip region

sr=beard (good find)

Comment 79

[Simon Fraser [no longer active]]

Patch checked in, with feeling.

Comment 80

[Chris Petersen]

Marking verified in the 2002-09-23-08 OS X build.

Comment 81

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 99942 [details] [diff] [review]
nsImageMac patch that fixes the bug.

This isn't obsolete; the later patch depends on this one.

a=[e-mail] for 1.0 branch; change mozilla1.0.2+ to fixed1.0.2 when
checked in.  Please check in both related patches.

Comment 82

[Daniel Veditz [:dveditz]]

adt+ for the branch.

Comment 83

[Simon Fraser [no longer active]]

Paches checked into the mozilla 1.0 branch.

Comment 84

[Jaime Rodriguez, Jr.]

Marking as a adt1.0.2+ per Comment #82 From [e-mail].

petersen: can you pls verify this as fixed on the 1.0 branch, then replace
"fixed1.0.2" with "verified1.0.2"? thanks!

Comment 85

[Chris Petersen]

Verified on 2002-09-24-08 branch