With the patch in bug 595656 landed when scrolling around in the add-ons manager you'll see bits of the border scroll along with the content. They remain there for a short time, presumably a repaint is going on after that. Screenshot: https://bugzilla.mozilla.org/attachment.cgi?id=478953

A reduced testcase would probably be helpful here.

Pretty straightforward to reproduce, just put an iframe inside a box with border radius

Also see odd issues with just a scrollable div, there it occasionally paints the content outside the radius and sometimes the text inside gets clipped

Thanks for the testcases.

We clip display items as we render them into the ThebesLayer for the scrolled content. Then when we scroll, we bump the transform on the ThebesLayer instead of repainting its contents. The problem is that the clipping here is invariant as the scroll position changes, so the rendering of the clipped content in the ThebesLayer is out of date. (This actually affects rectangular clips too, but it's less obvious because in almost all cases the area that gets scrolled into view is the only area affected, and it gets repainted anyway.) I need to think about the cleanest way to fix this.

The simplest thing to do here is to force scrolling to be "inactive" when there's an ancestor with rounded-rect clipping. Once layers support clip-to-path (post-FF4), we should then change the way we build ThebesLayers so that for a display item D that is assigned to a layer L for its nearest active scrolled root ancestor A, clipping induced by A or its ancestors is applied directly to L instead of each display item in L. That will give us accelerated scrolling even in the presence of rounded-rect clipping ancestors.

Comment on attachment 493326 [details] [diff] [review] Part 2: Ensure that a scrollframe is 'inactive' if it can't be scrolled by blitting >@@ -1501,16 +1500,19 @@ CanScrollWithBlitting(nsIFrame* aFrame) >+ nsIScrollableFrame* sf = do_QueryFrame(f); >+ if (sf && nsLayoutUtils::HasNonZeroCorner(f->GetStyleBorder()->mBorderRadius)) >+ return PR_FALSE; I meant to think about this but haven't had a chance, so I'll just ask. Why do we only need to check the scrollable frame ancestors for border radius and not all ancestors?

Because border-radius only clips the children if overflow is not 'visible'.

Comment on attachment 493326 [details] [diff] [review] Part 2: Ensure that a scrollframe is 'inactive' if it can't be scrolled by blitting +++ b/layout/reftests/border-radius/scroll-1.html +<div style="border-radius:100px; width:300px; height:300px; overflow:hidden; border:1px solid black;" id="d"> + <div style="height:1000px;"> + <div style="margin-top:100px; height:20px; background:blue;"></div> + </div> +</div> Can you indent the div that isn't? That confused me. @@ -1737,16 +1754,20 @@ nsGfxScrollFrameInner::BuildDisplayList( + if (IsScrollingActive() && !CanScrollWithBlitting(mOuter)) { + MarkInactive(); + } Do we want to do this only when building a display list for painting? +void nsGfxScrollFrameInner::MarkInactive() +{ + if (IsAlwaysActive()) + return; + + mScrollingActive = PR_FALSE; + mOuter->InvalidateFrameSubtree(); +} MarkInactive is actually somewhat dangerous in that it invalidates the entire frame subtree; its name seems rather innocuous. Perhaps guard the invalidate on mScrollingActive being true? @@ -1596,20 +1607,26 @@ void nsGfxScrollFrameInner::ScrollVisual + PRBool canScrollWithBlitting = CanScrollWithBlitting(mOuter); + if (IsScrollingActive()) { + if (!canScrollWithBlitting) { + MarkInactive(); + } else { + flags |= nsIFrame::INVALIDATE_NO_THEBES_LAYERS; + } + } else if (canScrollWithBlitting) { + MarkActive(); } - MarkActive(); Don't we want to still MarkActive if IsScrollingActive() && canScrollWithBlitting? This extends the time we keep the scroll frame active.

(In reply to comment #12) > +++ b/layout/reftests/border-radius/scroll-1.html > +<div style="border-radius:100px; width:300px; height:300px; overflow:hidden; > border:1px solid black;" id="d"> > + <div style="height:1000px;"> > + <div style="margin-top:100px; height:20px; background:blue;"></div> > + </div> > +</div> > > Can you indent the div that isn't? That confused me. Sure. > @@ -1737,16 +1754,20 @@ nsGfxScrollFrameInner::BuildDisplayList( > + if (IsScrollingActive() && !CanScrollWithBlitting(mOuter)) { > + MarkInactive(); > + } > > Do we want to do this only when building a display list for painting? Yeah OK. We should check IsPaintingToWindow() actually. > +void nsGfxScrollFrameInner::MarkInactive() > +{ > + if (IsAlwaysActive()) > + return; > + > + mScrollingActive = PR_FALSE; > + mOuter->InvalidateFrameSubtree(); > +} > > MarkInactive is actually somewhat dangerous in that it invalidates the entire > frame subtree; its name seems rather innocuous. Perhaps guard the invalidate on > mScrollingActive being true? Yeah OK. > @@ -1596,20 +1607,26 @@ void nsGfxScrollFrameInner::ScrollVisual > + PRBool canScrollWithBlitting = CanScrollWithBlitting(mOuter); > + if (IsScrollingActive()) { > + if (!canScrollWithBlitting) { > + MarkInactive(); > + } else { > + flags |= nsIFrame::INVALIDATE_NO_THEBES_LAYERS; > + } > + } else if (canScrollWithBlitting) { > + MarkActive(); > } > - MarkActive(); > > Don't we want to still MarkActive if IsScrollingActive() && > canScrollWithBlitting? This extends the time we keep the scroll frame active. Yes, good point.

comments addressed

This is a regression in that pages which scrolled OK before now won't.

http://hg.mozilla.org/mozilla-central/rev/e870951c9167 http://hg.mozilla.org/mozilla-central/rev/2a0d0ed04874

Marked test random, since it fails on some of the edge pixels on Mac and Windows. http://hg.mozilla.org/mozilla-central/rev/6d5bec5bfb8d

Should we mark is failing on windows and mac so we at least get test coverage on linux?

I'm going to try to fix the test for all platforms by removing the border.

http://hg.mozilla.org/try/pushloghtml?changeset=b3f754733e39 FWIW

http://hg.mozilla.org/try/rev/a112ac6b4aa3 was green on try, and can be landed.

All visible issues I have seen with older builds are solved. Marking as verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b9pre) Gecko/20110105 Firefox/4.0b9pre ID:20110105030550

fix test http://hg.mozilla.org/mozilla-central/rev/b6286cc2817a