Bug 46864 – [chromium] disappearing scrollbar in accelerated compositing mode

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Nat Duca]

Composited pages with scrollbars will sometimes lose their scrollbar handle when scrolling. This is caused by the damage rect for the scrollbar being incorrectly handled in content space.

[Nat Duca]

Created attachment 69294 [details]
Proposed fix.

[WebKit Review Bot]

Attachment 69294 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/4193020

[Nat Duca]

Created attachment 69351 [details]
Now, with 2x the foo

[Vangelis Kokkevis]

Comment on attachment 69351 [details]
Now, with 2x the foo

View in context: https://bugs.webkit.org/attachment.cgi?id=69351&action=review

> WebKit/chromium/src/WebViewImpl.cpp:2227
>      if (!view)

Is this a good opportunity to also take care of the FIXME at the top of this method?

> WebKit/chromium/src/WebViewImpl.cpp:2278
> +        m_rootLayerDirtyRect.unite(innerDamage);

Does this mean you'll now have damage in both the unscrolled location and the scrolled one? Could you just reset the m_rootLayerDirtyRect to be be equal to innerDamage?

[Nat Duca]

> Is this a good opportunity to also take care of the FIXME at the top of this method?
I was going to file a separate bug for that.

> Does this mean you'll now have damage in both the unscrolled location and the 
> scrolled one? Could you just reset the m_rootLayerDirtyRect to be be equal to 
> innerDamage?
Uniting rather than reset is important. Consider when you have an animated image and you scrolled. The damage rect in that case has both the damage from the animation, and damage on the actual scrollbar that moved. Assigning innerDamage to the rootLayerDirtyRect would drop the srollbar damage and we'd see the scrollbar looking funky.

[Vangelis Kokkevis]

(In reply to comment #5)
> > Is this a good opportunity to also take care of the FIXME at the top of this method?
> I was going to file a separate bug for that.
> 
> > Does this mean you'll now have damage in both the unscrolled location and the 
> > scrolled one? Could you just reset the m_rootLayerDirtyRect to be be equal to 
> > innerDamage?
> Uniting rather than reset is important. Consider when you have an animated image and you scrolled. The damage rect in that case has both the damage from the animation, and damage on the actual scrollbar that moved. Assigning innerDamage to the rootLayerDirtyRect would drop the srollbar damage and we'd see the scrollbar looking funky.

I chatted with Nat off line and I see now why this is necessary.  The patch looks good on my end.

[James Robinson]

Comment on attachment 69351 [details]
Now, with 2x the foo

Looks good to me.  We discussed how to test this offline, it depends on a bit more infrastructure on the chromium end but we'll get tests up as soon as we can.

[Nat Duca]

Created attachment 69840 [details]
Fix changelog

[Nat Duca]

Darn, Sheib just pointed out a problem with this patch. Gotta go figure out how to address it.

[James Robinson]

Comment on attachment 69840 [details]
Fix changelog

OK, clearing r+ in that case

[Nat Duca]

Created attachment 69858 [details]
Use content rect converted to screenspace rather than a hand-created rect. This ensures we factor in both scrollbars during scrolling.

[WebKit Commit Bot]

Comment on attachment 69858 [details]
Use content rect converted to screenspace rather than a hand-created rect. This ensures we factor in both scrollbars during scrolling.

Clearing flags on attachment: 69858

Committed r69162: <http://trac.webkit.org/changeset/69162>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.