Closed Bug 152373 Opened 22 years ago Closed 22 years ago

fixed background does not work properly

Categories

(Core :: Web Painting, defect, P3)

Product:
Core
Component:
Web Painting
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: Daniel.Steinberger, Assigned: roc)

References

(
URL
)

Details

(Keywords: css1, regression)

Attachments

(1 file, 1 obsolete file)

much better fix
5.77 KB, patch
kmcclusk
: review+
kinmoz
: superreview+
Details | Diff | Splinter Review 
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1a) Gecko/20020617
BuildID:    2002061707

visiting Eric Meyer's css/edge http://www.meyerweb.com/eric/css/edge/ made me
see that we've got a regression with fixed backgrounds. haven't found out
exactly which circumstances (=which combination of CSS) cause the regression,
but i'll investigate.

Reproducible: Always
Steps to Reproduce:
1. visit http://www.meyerweb.com/eric/css/edge/
2. scroll a little

Actual Results:  the should-be-fixed image (astronaut) scolls with the page and
breaks when scolled out of the viewport and back in again

Expected Results:  work like in v1.0 and before
i've checked back with 1.0 .. it's definitly a regression and not caused by eric
changing some css or something.
Keywords: regression
It looks like we're either not setting or not observing NS_VIEW_FLAG_DONT_BITBLT
-- the fixed background draws at the correct position if you move the page and
then re-expose the window.
*** Bug 152480 has been marked as a duplicate of this bug. ***
Layout is setting the DONT_BITBLT flag on at least some views.  ->Views.
Assignee: dbaron → kmcclusk
Component: Style System → Views
QA Contact: ian → petersen
(And I see this on Linux too.)
OS: Windows XP → All
Hardware: PC → All
Mine
Assignee: kmcclusk → roc+moz
Priority: -- → P3
Notice that the complexspiral demo
http://www.meyerweb.com/eric/css/edge/complexspiral/demo.html does still work.
That is very strange, I thought these two pages where using the same effect
The thing making the difference between http://www.meyerweb.com/eric/css/edge/
and http://www.meyerweb.com/eric/css/edge/complexspiral/demo.html appears to be
that complexspiral has the body set to background-attachment:fixed, which seems
to make this bug vanish. See
http://tom.me.uk/2002/8/moz-background-attachment.html and
http://tom.me.uk/2002/8/moz-background-attachment-fixed.html
I suck. I should have fixed this for 1.1. I will fix this soon.
Severity: normal → major
Attached patch fix (obsolete) — — Splinter Review 
Here's the fix. The problem was that we were obeying the "ALWAYS_BLIT" flag
which was always being set on the root scrollable view. That flag is, of
course, wrong.

Instead of these magic "ALWAYS_BLIT", "NEVER_BLIT" flags, we need to finish the
job I started with CanScrollWithBitBlt, which is to actually compute accurately
when blitting is possible rather than relying on hints.

So I eliminated the last use of ALWAYS_BLIT. The only problem is that
CanScrollWithBitBlit ended up deciding that the root scroll frame in normal
documents could not be blitted, because the unscrolled viewport frame is
visible in the scroll region. (It's not really visible but some of the
shortcuts we take in CanScrollWithBitBlt make it look visible.) However it is
still safe to scroll with bitblt in this case because the viewport frame (and
the root scroll frame) never paint anything, so the fact that they're visible
is irrelevant.

So I added the new view property "is blank". When set on a view, it means the
view is either a solid colour or completely transparent (if marked
transparent). CanScrollWithBitBlt can ignore blank views that cover the entire
area to be scrolled (because the pixels drawn by those views will look the same
"before" and "after"). For now we just mark viewport views and scrollport views
as blank. There are additional optimizations we could do with this property in
the future --- e.g., dropping blank+transparent views from display lists early.


This patch eliminates the last use of ALWAYS_BLIT (NEVER_BLIT was already
unused). A future round of cruft removal should remove all mention of these
flags and all mention of Get/SetScrollProperties (since they're only used for
these flags).
> (It's not really visible but some of the shortcuts we take in
> CanScrollWithBitBlt make it look visible.)

Having slept on it, I think the viewport view really should not be showing up as
visible. Perhaps this blank stuff isn't actually necessary if I can fix some
other bug.
Although support for "blank" would help fix a DHTML problem we have: a view with
child views moving around, but no content of its own, keeps getting resized
which forces repainting of the areas affected by the resize.
Attached patch much better fix — — Splinter Review 
Here's the better fix. No BLANK required. We just do the following:
-- Get rid of the ALWAYS_BLIT hack (so all these magic flags can be ripped out
later as mentioned above)
-- Fix bug in the scrolling views to pass in the scrolled view to
CanScrollWithBitBlt (this is how it should have been all along)
-- The only problem left is that this causes us to use slow scrolling when
fixed-position stuff is present. Blitting is legal in this situation only
because fixed-position elements have native widgets, and the areas covered by
those native widgets aren't affected by the blit. To detect this in
CanScrollWithBitBlit we need to figure out the areas covered by native widgets
that will not be affected by a blit, and exclude these areas from the display
list calculation and inspection. This is unfortunately hard because I don't
trust the native widget layer's z-index information; given two sibling widgets,
I don't think we can be sure which one gets drawn in front of the other. The
only thing I do trust is that positioned elements have their native widgets in
front of the main scrolling widget for the document. So this code detects
exactly that situation and excludes those areas from the display list, and the
calculations work and we continue to get fast scrolling in the presence of
fixed position elements.
Attachment #97001 - Attachment is obsolete: true
The patch also enhances the display list debugging code a little bit.

I'd like to get this in before the freeze on Tuesday night. This is a pretty bad
bug because it screws up a CSS1 feature that many CSS demo sites use.
Comment on attachment 97363 [details] [diff] [review]
much better fix

sr=kin@netscape.com

Should we be using nsRectFast? I thought I saw a patch fly by that was trying
to remove all uses of it outside of the nsRegion code?
Attachment #97363 - Flags: superreview+
Thanks. I will change nsRectFast to nsRect before checking in.
Fix checked in.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Don't suppose this fix also takes care of bug 165149 (another nasty background 
bug)? Someone suggested it might be related to this one...
Comments there indicate that this has fixed bug 106580.
excelent work!
Status: RESOLVED → VERIFIED
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: