RESOLVED WONTFIX80348
[chromium] Remove texture readback from WebViewImpl::paint 
https://bugs.webkit.org/show_bug.cgi?id=80348
Summary [chromium] Remove texture readback from WebViewImpl::paint
Alexandre Elias
Reported 2012-03-05 16:39:45 PST
[chromium] Remove texture readback from WebViewImpl::paint
Attachments
Patch (24.53 KB, patch)
2012-03-05 17:38 PST, Alexandre Elias 		jamesr: review-
webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from ec2-cr-linux-02 (1.28 MB, application/zip)
2012-03-05 18:27 PST, WebKit Review Bot 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Alexandre Elias
Comment 1 2012-03-05 17:38:39 PST
Created attachment 130248 [details] Patch
WebKit Review Bot
Comment 2 2012-03-05 17:41:17 PST
Please wait for approval from fishd@chromium.org, abarth@webkit.org or jamesr@chromium.org before submitting because this patch contains changes to the Chromium platform API.
James Robinson
Comment 3 2012-03-05 17:51:00 PST
This will produce odd-looking thumbnails for pages like poster circle. Can we do that?
Alexandre Elias
Comment 4 2012-03-05 18:02:44 PST
I understand the thumbnailer will be moving to capturing the browser-side texture/bitmap on most platforms anyway, addressing that issue in the long run. I guess we could hold off on submitting this until that transition is complete everywhere, although my inclination would be to go ahead and delete it now. The other uses are corner cases like Chrome for Android's link disambiguation popup feature, and it's probably not worth maintaining this codepath for a corner-case-on-a-corner-case.
W. James MacLean
Comment 5 2012-03-05 18:20:07 PST
(In reply to comment #4) > I understand the thumbnailer will be moving to capturing the browser-side texture/bitmap on most platforms anyway, addressing that issue in the long run. I guess we could hold off on submitting this until that transition is complete everywhere, although my inclination would be to go ahead and delete it now. > > The other uses are corner cases like Chrome for Android's link disambiguation popup feature, and it's probably not worth maintaining this codepath for a corner-case-on-a-corner-case. What's the rush to delete it? When the browser-texture patch is possible then it seems much simpler, but until then I think we need to provide some reasonable looking form of thumbnail.
Alexandre Elias
Comment 6 2012-03-05 18:27:12 PST
I trust the software path to give reasonable thumbnails more often than the hardware readback one in any case. In Chrome for Android, we ran into many bugs where we'd get an entirely black thumbnail returned. They went away when we switched to browser-side thumbnailing. I'd wager the other platforms have similar bugs, but they're just hidden because compositing is usually not enabled.
WebKit Review Bot
Comment 7 2012-03-05 18:27:38 PST
Comment on attachment 130248 [details] Patch Attachment 130248 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11837103 New failing tests: compositing/geometry/fixed-in-composited.html compositing/geometry/vertical-scroll-composited.html compositing/geometry/layer-due-to-layer-children.html compositing/direct-image-compositing.html compositing/geometry/fixed-position-transform-composited-page-scale-down.html compositing/geometry/layer-due-to-layer-children-deep.html compositing/geometry/fixed-position-transform-composited-page-scale.html compositing/geometry/fixed-position.html animations/opacity-transform-animation.html accessibility/aria-disabled.html compositing/backface-visibility.html compositing/geometry/fixed-position-iframe-composited-page-scale.html compositing/geometry/tall-page-composited.html compositing/culling/filter-occlusion-alpha-large.html animations/missing-values-last-keyframe.html compositing/flat-with-transformed-child.html compositing/geometry/horizontal-scroll-composited.html compositing/images/direct-image-background-color.html compositing/geometry/ancestor-overflow-change.html compositing/scrollbar-painting.html animations/missing-values-first-keyframe.html compositing/backface-visibility-hierarchical-transform.html compositing/geometry/clipping-foreground.html compositing/culling/filter-occlusion-alpha.html compositing/geometry/fixed-position-iframe-composited-page-scale-down.html compositing/masks/direct-image-mask.html compositing/culling/filter-occlusion-blur-large.html compositing/culling/filter-occlusion-blur.html compositing/color-matching/image-color-matching.html compositing/video-page-visibility.html
WebKit Review Bot
Comment 8 2012-03-05 18:27:43 PST
Created attachment 130259 [details] Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
James Robinson
Comment 9 2012-03-05 18:29:08 PST
Oh, and DumpRenderTree uses this path for all of the compositing layout tests. A bunch of browser_tests do the same thing. I forgot about those :) I would really like to get rid of this codepath ASAPly, it's a pain in the butt to maintain.
James Robinson
Comment 10 2012-03-06 13:21:02 PST
Comment on attachment 130248 [details] Patch I don't think we can do this yet.
Note You need to log in before you can comment on or make changes to this bug.