Bug 79951

Summary: [chromium] Mark root layer scrollbars as always opaque to disable blending
Product: WebKit Reporter: Adrienne Walker <enne>
Component: New BugsAssignee: Adrienne Walker <enne>
Status: RESOLVED FIXED    
Severity: Normal CC: enne, jamesr, vangelis, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 78872    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Whoops, remove printf
none
Patch for landing webkit.review.bot: commit-queue-

Adrienne Walker
Reported 2012-02-29 15:30:39 PST
[chromium] Mark root layer scrollbars as always opaque to disable blending
Attachments
Patch (1.94 KB, patch)
2012-02-29 15:36 PST, Adrienne Walker
no flags
Patch (5.48 KB, patch)
2012-03-09 15:42 PST, Adrienne Walker
no flags
Whoops, remove printf (4.38 KB, patch)
2012-03-09 15:43 PST, Adrienne Walker
no flags
Patch for landing (4.88 KB, patch)
2012-03-09 16:20 PST, Adrienne Walker
webkit.review.bot: commit-queue-
Adrienne Walker
Comment 1 2012-02-29 15:36:19 PST
James Robinson
Comment 2 2012-02-29 15:37:12 PST
Comment on attachment 129537 [details] Patch I do not think overlay root layer scrollbars are always opaque (custom or non-custom). This needs some more checks, methinks.
James Robinson
Comment 3 2012-02-29 15:39:12 PST
Comment on attachment 129537 [details] Patch I think checking for isOverlayScrollbar() might be sufficient - all non-overlay root layer scrollbars are definitely opaque. We need to handle overlay->non overlay transitions and vice versa, but we should get a setViewport() call when that happens.
Adrienne Walker
Comment 4 2012-02-29 15:44:40 PST
(In reply to comment #3) > (From update of attachment 129537 [details]) > I think checking for isOverlayScrollbar() might be sufficient - all non-overlay root layer scrollbars are definitely opaque. We need to handle overlay->non overlay transitions and vice versa, but we should get a setViewport() call when that happens. Oh, quite right. Scrollbar::isOverlayScrollbar is pretty far away from LayerChromium at that point, and it seems like a layering violation to pipe it in. I suppose the other option is to wait and do this at the ScrollbarLayer level after https://bugs.webkit.org/show_bug.cgi?id=78872 lands.
James Robinson
Comment 5 2012-02-29 15:52:40 PST
(In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 129537 [details] [details]) > > I think checking for isOverlayScrollbar() might be sufficient - all non-overlay root layer scrollbars are definitely opaque. We need to handle overlay->non overlay transitions and vice versa, but we should get a setViewport() call when that happens. > > Oh, quite right. Scrollbar::isOverlayScrollbar is pretty far away from LayerChromium at that point, and it seems like a layering violation to pipe it in. > Ah yeah, that is farther than I thought (I was thinking we could just reach in to the ScrollableArea but we've helpfully abstracted NonCompositedContentHost away from being able to do things like that). > I suppose the other option is to wait and do this at the ScrollbarLayer level after https://bugs.webkit.org/show_bug.cgi?id=78872 lands. Coincidentally I've been looking at that patch. That would suck hardcore to have to merge if we need to do that. Other gross option: pass isOverlay bits in the setViewport call from WebViewImpl.
Adrienne Walker
Comment 6 2012-03-09 15:42:03 PST
Adrienne Walker
Comment 7 2012-03-09 15:43:19 PST
Created attachment 131127 [details] Whoops, remove printf
James Robinson
Comment 8 2012-03-09 15:55:32 PST
Comment on attachment 131127 [details] Whoops, remove printf If we pass FrameView* to the scrollbarLayerDidChange function then we can also move the scrollLayerForFrameView() code into that helper too, instead of duplicating it across both of the frameView*ScrollbarLayerDidChange() functions
Adrienne Walker
Comment 9 2012-03-09 16:20:45 PST
Created attachment 131130 [details] Patch for landing
WebKit Review Bot
Comment 10 2012-03-09 21:13:01 PST
Comment on attachment 131130 [details] Patch for landing Rejecting attachment 131130 [details] from commit-queue. New failing tests: platform/chromium/virtual/gpu/fast/canvas/canvas-text-alignment.html Full output: http://queues.webkit.org/results/11892959
Adrienne Walker
Comment 11 2012-03-12 13:03:20 PDT
Comment on attachment 131130 [details] Patch for landing This passes locally for me. Let me try this again.
WebKit Review Bot
Comment 12 2012-03-12 14:22:10 PDT
Comment on attachment 131130 [details] Patch for landing Rejecting attachment 131130 [details] from commit-queue. New failing tests: platform/chromium/virtual/gpu/fast/canvas/canvas-text-alignment.html Full output: http://queues.webkit.org/results/11949021
Adrienne Walker
Comment 13 2012-03-13 15:06:47 PDT
Adrienne Walker
Comment 14 2012-03-13 15:09:59 PDT
(In reply to comment #13) > Committed r110620: <http://trac.webkit.org/changeset/110620> Failing test was due to https://bugs.webkit.org/show_bug.cgi?id=78529 (and test being disabled on Linux debug). This patch should make debug and release equivalent and I will rebaseline after this goes through the bots.
Note You need to log in before you can comment on or make changes to this bug.