190311 - Redraws of then alpha background of the second absolutely positioned element fail.

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Gtk, defect)

Description

[Orien Vandenbergh]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3a) Gecko/20021212
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3a) Gecko/20021212

In the page at http://www.icecode.com/menu_trans.html the alpha background of
the #menu element is not redrawn correctly.  There are links to other versions
of the page where the menu is affected or where both absolutely positioned
elements use solid backgrounds which are unaffected.  Elements such as the table
cells, for which I also used an alpha background exhibit no such problems

Reproducible: Always

Steps to Reproduce:
1. Load example page using alpha background.
2. Force mozilla to redraw the page, either by iconifying, shading, changing
workspaces, or just scrolling up and down.  The alpha backgrounds should exhibit
redraw problems.
3. Repeat with the pages without alpha backgrounds and see that the redraws are
successful
Actual Results:  
The alpha backgrounds do not redraw correctly

Expected Results:  
It should have redrawn the alpha backgrounds with the same robustness as the
solid versions.

Comment 1

[Ruslan Ismailov]

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

Comment 2

[Orien Vandenbergh]

This might be the same as 189894, but I couldn;t figure out what the other bug
report was talking about, so I submitted my own.  

Also I tested these pages in the most recent nightly build, and experienced the
same problems.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Probably a Linux image rendering bug, but I'm not sure. Does it happen on Windows?

Comment 4

[Orien Vandenbergh]

I had a friend test it out with a rather old version "Mozilla/5.0 (Windows; U;
Windows NT 5.1; en-US; rv:1.1b) Gecko/20020721".  Unfortunately I don't have
access to windows myself to test it with a newer release.

Comment 5

[Orien Vandenbergh]

Sorry, failed to mention, the bug does not appear to be in the windows version
that he tested.  Whether it come into existance later in the tree, I can't say,
but it wasn't there 6 months ago.

Comment 6

[ArronM (:paper)]

tor's temporarily unavailable, but I'm CCing him anyway.  He might be able to help.

Comment 7

[Andrew Schultz]

this regressed between linux trunk builds 2002061108 and 2002061304, indicating
the culrprit is bug 85595

Comment 8

[ArronM (:paper)]

CCing people from Bug 148350.  If I follow the logic correctly, these two are
not the same: 
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/view/src&command=DIFF_FRAMESET&file=nsView.cpp&rev1=3.156&rev2=3.157&root=/cvsroot

This bug happens with the code on the left, but does exist on the code on the right.

Comment 9

[ArronM (:paper)]

Attachment: Patch

Comment 10

[ArronM (:paper)]

Simpler Testcase:
http://mozilla.animecity.nu/GTKBug/menu_trans.html

Comment 11

[ArronM (:paper)]

Comment on attachment 113254 [details] [diff] [review]
Patch

kevin, roc, please check to see if I'm actually correcting the logic.  Thanks
:)

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Arron, I'm pretty sure those pieces of code are equivalent and your patch is
wrong. Can you describe the variable values where you think they're not equivalent?

Comment 13

[ArronM (:paper)]

Comment on attachment 113254 [details] [diff] [review]
Patch

I checked again, an you're right, the patch is totally wrong.

Comment 14

[ArronM (:paper)]

I can't fix this.

It is caused by Bug 148350.  Prior to that patch, skipping of parentIsFloating
assigning was common because if !lastViewIsFloating == True, the rest of the
while  logic did not need to be run, which is where parentIsFloating was set. 
In those cases parentIsFloating was uninitialized and probably evaluated to
true.  At the end of the while, lastViewIsFloating is set to parentIsFloating,
which would make lastViewIsFloating invalid, and the while loop would screw up.

Bug 148350 fixed that problem, and unveiled this bug.

I don't know enough about clippings and flow to even guess where to begin. 
Resetting owner and QA.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I found the problem. It's the intersection of at least 3 bugs, 2 in GFX-GTK and
1 in views.

One bug in GFX-GTK is that DrawTile() does not call UpdateGC() after calling
nsIRenderingContext::SetClipRect and before actual rendering (in the 8-bit alpha
case). This means that the image is tiled using the clip rect set by the last
UpdateGC(), which in this case is the area of the last view rendered. Oops.

When I added an UpdateGC() call in what seemed to be the right place, I found
another bug in DrawTile(). It calls nsIRenderingContext::SetClipRect and passes
in a rectangle in pixels, but the rect is supposed to be in twips. So the rect
gets scaled down and the image is mutilated. So I added a new
SetClipRectInPixels and used it.

The thing that caused all this to manifest is that the view system was setting
VIEW_CLIPPED on too many views because of insufficient logic in GetClippedRect.
This *should* have been benign (the only use of VIEW_CLIPPED is to avoid setting
the clip rect if we don't really need to) but because of the above bugs it
caused this problem to appear.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fixes

Comment 17

[Stuart Parmenter]

Comment on attachment 113493 [details] [diff] [review]
fixes

r=pavlov.  glad you added comments about SetClipRect vs SetClipRegion.	I know
that has bitten people in the past.

Comment 18

[Kevin McCluskey (gone)]

r=[e-mail] for the view module changes.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 113493 [details] [diff] [review]
fixes

fun.....  we really need to get unit stuff sorted out.	:(

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked in.

Comment 21

[Roland Mainz]

Reopen bug - the patch did not cover Xlib gfx... ;-(

Comment 22

[Roland Mainz]

Attachment: Patch for 2003-02-07-08-trunk

Comment 23

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 115463 [details] [diff] [review]
Patch for 2003-02-07-08-trunk

Have we considered subclassing a common superclass that will contain all the
identical methods that can be shared between gtk/xlib/qt/whatever?

Comment 24

[Roland Mainz]

Boris Zbarsky wrote:
> (From update of attachment 115463 [details] [diff] [review])
> Have we considered subclassing a common superclass that will contain all the
> identical methods that can be shared between gtk/xlib/qt/whatever?

subclassing may be the wrong approach. In theory we could migrate gfx/src/gtk,
gfx/src/xlib, gfx/src/xprint and gfx/src/qt into one template placed in
gfx/src/x11shared/ - but AFAIK C++ templates are not allowed per mozilla.org's
coding policy... ;-(

Comment 25

[ArronM (:paper)]

bz: sort of, that would be Bug 182658, at least for nsImage*.  We already have a
superclass for nsIRenderingContext, which is nsRenderingContextImpl.

Unfortunately, it wouldn't helpfull for this bug, since SetClipRectInPixels and
UpdateGC are being added to the platform specific code, and not the platform
independent nsRenderingContextImpl.  (IMO, they should have been added to
nsRenderingContextImpl, but I realize that would cause problems)

Comment 26

[Leonid Romanov]

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

Comment 27

[Roland Mainz]

Attachment 115463 [details] [diff] checked-in into "trunk"
(http://bonsai.mozilla.org/cvsquery.cgi?module=MozillaTinderboxAll&branch=HEAD&cvsroot=/cvsroot&date=explicit&mindate=1046225340&maxdate=1046225400&who=smontagu%25netscape.com),
marking bug as FIXED.