Closed Bug 657401 Opened 9 years ago Closed 8 years ago

Page flickers once when scroll to top by dragging thumb of scrollbar. (disabled Hardware acceleration)

Categories

(Core :: Graphics, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla8
Tracking Status
firefox6 - ---

People

(Reporter: alice0775, Assigned: roc)

References

()

Details

(Keywords: regression, Whiteboard: nominated at comment 0)

Attachments

(2 files)

Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/6e4fb61ef475
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110516 Firefox/6.0a1 ID:20110516030622

If I disabled Hardware acceleration.
Page flickers once when scroll to top by dragging thumb of scrollbar.

Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile + Disable Hardware acceleration
2. Open URL
3. Scroll up about 200px (in order to hide header/menu part)
4. Scroll to top by dragging thumb of scrollbar

Actual Results:
 Page flickers once.

Expected Results:
 Page should not flicker.

Regression window(cached m-c hourly):
Works:
http://hg.mozilla.org/mozilla-central/rev/a28f4e72593a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110429 Firefox/6.0a1 ID:20110429151036
Fails:
http://hg.mozilla.org/mozilla-central/rev/8a650b9e55db
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110429 Firefox/6.0a1 ID:20110429152220
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a28f4e72593a&tochange=8a650b9e55db
Triggered by:
8a650b9e55db	Zack Weinberg — Bug 651498: call gfxPlatform::Init when necessary and not earlier. r=joedrew,bsmedberg
OS: Windows 7 → All
Between bug 651017 (2011-04-19) and bug 651498 (2011-04-29), HWA could not be disabled because some gfx prefs were not being read and applied, so the build right before bug 651498 actually had HWA enabled.
OS: All → Windows 7
(In reply to comment #1)
> Between bug 651017 (2011-04-19) and bug 651498 (2011-04-29), HWA could not
> be disabled because some gfx prefs were not being read and applied, so the
> build right before bug 651498 actually had HWA enabled.

OK
Regression window(cached m-c hourly):
Works:
http://hg.mozilla.org/mozilla-central/rev/0e8c23e50c6c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110418 Firefox/6.0a1 ID:20110418195729
Fails:
http://hg.mozilla.org/mozilla-central/rev/fc1ed658bf4b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110418 Firefox/6.0a1 ID:20110418201130
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0e8c23e50c6c&tochange=fc1ed658bf4b
Triggered by:
Bug 641426 - Unify point/size/rect types

This also happens on Ubuntu10.04(it is slightly difficult to reproduce, it need repeat step 3 and step4)
http://hg.mozilla.org/mozilla-central/rev/6e4fb61ef475
Mozilla/5.0 (X11; Linux i686; rv:6.0a1) Gecko/20110516 Firefox/6.0a1 ID:20110516030622
Blocks: 641426
No longer blocks: 651498
OS: Windows 7 → All
Attached file Reduced testcase
Assignee: nobody → roc
The problem is, after we've fixed bug 647560, that ChildrenPartitionVisibleRegion assumes that BasicLayers don't paint outside the bounds of their visible region, but this is not true in general. In this testcase, there's an L-shaped ThebesLayer that paints its whole area. But the same problem could conceivably arise for ImageLayers and other layers that don't clip to their visible region. So we need to make BasicLayers clip to their visible regions in some cases, at least when they have no ancestor doing double-buffering for them.
Attached patch fixSplinter Review
I don't think we can test this double-buffering code currently.
Attachment #532930 - Flags: review?(tnikkel)
This patch applies on top of the patches for bug 637852 and bug 647560.
I'm curious about how bug 641426 caused this.
Comment on attachment 532930 [details] [diff] [review]
fix

-   * in response to this method call. aContext will already have been
+   * in response to this method call. aContext will already have been1593

Seems unintended.

+  PRBool ForceClipToVisibleRegion() { return mClipToVisibleRegion; }

I think this should be called GetClipToVisibleRegion or something. ForceClipToVisibleRegion makes it sound like it is doing something, ie forcing us to clip to the visible region.
Attachment #532930 - Flags: review?(tnikkel) → review+
Done.
Whiteboard: [needs landing]
Whiteboard: [needs landing] → [needs landing] nominated at comment 0
Attachment #532930 - Flags: approval-mozilla-aurora?
roc, can you (or one of the reviewers here) help release drivers make the approval-aurora? call with a description of the benefit of taking this change and what kind of risk is here?
The change would need to be rebased so it doesn't depend on bug 647560, which is not trivial.

Personally I think this regression is very minor. AFAIK we have only Alice's report, and no duplicates. I'm not really worried if it ships in FF6. Alice, what do you think?
This is a minor issue.I agree with comment #12.
Comment on attachment 532930 [details] [diff] [review]
fix

agreeing with alice and roc. not for aurora.
Attachment #532930 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
http://hg.mozilla.org/integration/mozilla-inbound/rev/d63d5764397c
Whiteboard: [needs landing] nominated at comment 0 → [inbound] nominated at comment 0
https://hg.mozilla.org/mozilla-central/rev/d63d5764397c
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [inbound] nominated at comment 0 → nominated at comment 0
Target Milestone: --- → mozilla8
Please take a look at Bug 684032 - continuous pronounced flickering when loading and scrolling in 7.0 (thunderbird and firefox)

The effects that I am observing, though intermittent, are far more pronounced and unpleasant than what's described here (a single flicker), but it's likely that this fix addresses them.  It doesn't make sense to release 7.0 with such a significant visual regression, it's going to generate many negative impressions.

Because I can repro reliably, on demand, only in TB, I would need a TB build with this fix, to confirm it does in fact take care of the problem.
Please comment on Bug 684032.  Would it not make sense to address those symptoms for 7.0 releases?  You should be able to repro easily with TB, or watch the attached video.
I confirmed that this patch resolves the symptoms described in Bug 684032, which are more severe than in this one, so this fix should be in 7.0.
Duplicate of this bug: 684032
Comment on attachment 532930 [details] [diff] [review]
fix

Renominating based on new information about the scope of the problem...

If we don't take this for Firefox, is this something Thunderbird should consider taking on their relbranch?  :(
Attachment #532930 - Flags: approval-mozilla-beta?
Attachment #532930 - Flags: approval-mozilla-aurora?
Attachment #532930 - Flags: approval-mozilla-aurora-
Comment on attachment 532930 [details] [diff] [review]
fix

Clearing the aurora request flag as it looks like this landed on central before the aurora8 uplift (please request again if we're wrong).

Denying for beta due to the rebasing / additional dependencies needed. If TB wants this (as it looks like a bigger issue) they can take it on a relbranch.

Please request approval again with justification if you disagree.
Attachment #532930 - Flags: approval-mozilla-aurora?
Attachment #532930 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
(In reply to Christian Legnitto [:LegNeato] from comment #22)
>
> Denying for beta due to the rebasing / additional dependencies needed. If TB
> wants this (as it looks like a bigger issue) they can take it on a relbranch.
> 

Why is it a bigger issue for TB?  How often does one open web pages in TB (vs. Fx)?  Did you watch the screencap video?  The same problem happens intermittently in Fx, it makes it seem alpha quality.  Such regressions should not be acceptable, considering that staying with 6.0 is not an option.
Depends on: 690356
The following symptom is possibly related and still present in 8.0:

1. do a Google maps search
2. as you mouse over the results on the left, the corresponding pushpin in the map expands.  The pushpin flickers as it expands.  
3. Panning the map slightly, gets rid of the flicker, as you mouse-over again, the pushpin expands smoothly.
You should file a new bug and test in a nightly for that issue.
(In reply to Timothy Nikkel (:tn) from comment #25)
> You should file a new bug and test in a nightly for that issue.

Bug 695891 - Google maps pushpins flicker (not always) as they expand on results mouse over
You need to log in before you can comment on or make changes to this bug.