Canvas capture stream causes two glReadPixels calls per frame |
||||
Issue descriptionHTMLCanvasElement.captureStream(), implemented in Issue 524218 , is causing two ReadPixels calls per frame. brianderson@ noticed this while examining traces (via about:tracing) from www.shadertoy.com. Attached is a minimized test case, a trace which can be loaded into about:tracing, and backtraces from lldb on macOS which show the problem. The problem stack trace is as follows: libgles2_implementation.dylib`gpu::gles2::GLES2Implementation::ReadPixels(...) at gles2_implementation.cc:3997 [opt] [lambda] [lambda] [lambda] [lambda] libskia.dylib`GrGLGpu::onReadPixels(...) [inlined] std::__1::function<...>::operator(...)(int, int, int, int, unsigned int, unsigned int, void*) const at functional:1924 [opt] libskia.dylib`GrGLGpu::onReadPixels(...) at GrGLGpu.cpp:2478 [opt] libskia.dylib`GrGpu::readPixels(...) at GrGpu.cpp:371 [opt] libskia.dylib`GrContextPriv::readSurfacePixels(...) at GrContext.cpp:537 [opt] libskia.dylib`GrSurfaceContext::readPixels(...) at GrSurfaceContext.cpp:49 [opt] libskia.dylib`SkImage_Gpu::onReadPixels(...) const at SkImage_Gpu.cpp:203 [opt] libcontent.dylib`content::CanvasCaptureHandler::CreateNewFrame(...) at canvas_capture_handler.cc:219 [opt] libblink_core.dylib`blink::HTMLCanvasElement::NotifyListenersCanvasChanged(...) at HTMLCanvasElement.cpp:556 [opt] libblink_core.dylib`blink::HTMLCanvasElement::DoDeferredPaintInvalidation(...) at HTMLCanvasElement.cpp:402 [opt] libblink_core.dylib`blink::HTMLCanvasPaintInvalidator::InvalidatePaint(...) at HTMLCanvasPaintInvalidator.cpp:20 [opt] libblink_core.dylib`blink::LayoutHTMLCanvas::InvalidatePaint(...) const at LayoutHTMLCanvas.cpp:88 [opt] libblink_core.dylib`blink::PaintInvalidator::InvalidatePaint(...) at PaintInvalidator.cpp:489 [opt] libblink_core.dylib`blink::PrePaintTreeWalk::Walk(...) at PrePaintTreeWalk.cpp:316 [opt] libblink_core.dylib`blink::PrePaintTreeWalk::Walk(...) at PrePaintTreeWalk.cpp:331 [opt] libblink_core.dylib`blink::PrePaintTreeWalk::Walk(...) at PrePaintTreeWalk.cpp:331 [opt] libblink_core.dylib`blink::PrePaintTreeWalk::Walk(...) at PrePaintTreeWalk.cpp:331 [opt] libblink_core.dylib`blink::PrePaintTreeWalk::Walk(...) at PrePaintTreeWalk.cpp:331 [opt] libblink_core.dylib`blink::PrePaintTreeWalk::Walk(...) at PrePaintTreeWalk.cpp:331 [opt] libblink_core.dylib`blink::PrePaintTreeWalk::Walk(...) at PrePaintTreeWalk.cpp:331 [opt] libblink_core.dylib`blink::PrePaintTreeWalk::Walk(...) at PrePaintTreeWalk.cpp:331 [opt] libblink_core.dylib`blink::PrePaintTreeWalk::Walk(...) at PrePaintTreeWalk.cpp:101 [opt] libblink_core.dylib`blink::PrePaintTreeWalk::Walk(...) at PrePaintTreeWalk.cpp:75 [opt] libblink_core.dylib`blink::FrameView::PrePaint(...) at FrameView.cpp:3261 [opt] libblink_core.dylib`blink::FrameView::UpdateLifecyclePhasesInternal(...) at FrameView.cpp:3190 [opt] libblink_core.dylib`blink::PageAnimator::UpdateAllLifecyclePhases(...) at PageAnimator.cpp:100 [opt] libblink_web.dylib`blink::WebViewImpl::UpdateAllLifecyclePhases(...) at WebViewImpl.cpp:2036 [opt] libcontent.dylib`content::RenderWidget::UpdateVisualState(...) at render_widget.cc:962 [opt] This one should not be provoking a ReadPixels call. This is the stack trace which should be (and currently is): libgles2_implementation.dylib`gpu::gles2::GLES2Implementation::ReadPixels(...) at gles2_implementation.cc:3997 [opt] [lambda] [lambda] [lambda] [lambda] libskia.dylib`GrGLGpu::onReadPixels(...) [inlined] std::__1::function<...>::operator(...)(int, int, int, int, unsigned int, unsigned int, void*) const at functional:1924 [opt] libskia.dylib`GrGLGpu::onReadPixels(...) at GrGLGpu.cpp:2478 [opt] libskia.dylib`GrGpu::readPixels(...) at GrGpu.cpp:371 [opt] libskia.dylib`GrContextPriv::readSurfacePixels(...) at GrContext.cpp:537 [opt] libskia.dylib`GrSurfaceContext::readPixels(...) at GrSurfaceContext.cpp:49 [opt] libskia.dylib`SkImage_Gpu::onReadPixels(...)=kAllow_CachingHint) const at SkImage_Gpu.cpp:203 [opt] libcontent.dylib`content::CanvasCaptureHandler::CreateNewFrame(...) at canvas_capture_handler.cc:219 [opt] libblink_core.dylib`blink::HTMLCanvasElement::NotifyListenersCanvasChanged(...) at HTMLCanvasElement.cpp:556 [opt] libblink_platform.dylib`blink::DrawingBuffer::PrepareTextureMailboxInternal(...) [inlined] base::Callback<void (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>::Run(this=<unavailable>) const at callback.h:80 [opt] libblink_platform.dylib`blink::DrawingBuffer::PrepareTextureMailboxInternal(...) [inlined] WTF::Function<void (), (WTF::FunctionThreadAffinity)1>::operator(this=<unavailable>)() at Functional.h:221 [opt] libblink_platform.dylib`blink::DrawingBuffer::PrepareTextureMailboxInternal(...) at DrawingBuffer.cpp:286 [opt] libblink_platform.dylib`blink::DrawingBuffer::PrepareTextureMailbox(...) at DrawingBuffer.cpp:256 [opt] libcc.dylib`cc::TextureLayer::Update(...) at texture_layer.cc:188 [opt] libcc.dylib`cc::LayerTreeHost::DoUpdateLayers(...) at layer_tree_host.cc:1091 [opt] libcc.dylib`cc::LayerTreeHost::DoUpdateLayers(...) at layer_tree_host.cc:738 [opt] libcc.dylib`cc::LayerTreeHost::UpdateLayers(...) at layer_tree_host.cc:618 [opt] libcc.dylib`cc::ProxyMain::BeginMainFrame(...) at proxy_main.cc:225 [opt] Separately, we have some questions about whether the CanvasCaptureMediaStream should be grabbing frames at all unless its output is actually being consumed, but will ask those offline.
,
May 19 2017
Thanks Emircan for pointing out this recent change. Justin, can I assign this to you? It seems to me that we should be able to perform the frame capture just before producing the frame for the compositor in all cases, thereby getting rid of the call during paint invalidation.
,
May 22 2017
Kai added an async read pixels that might be able to further bring <canvas> capture down to 0 blocking read pixels: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/webgl/WebGLGetBufferSubDataAsync.cpp?sq=package:chromium&l=90 emircan@, would that be something worth pursuing and opening up a separate issue for?
,
May 23 2017
Re #3, it is definitely worth looking at. Currently, we are doing it on synchronously on main thread. https://cs.chromium.org/chromium/src/content/renderer/media_capture_from_element/canvas_capture_handler.cc?rcl=884608358e1fd3064ad56cba1d39049e4f130943&l=219
,
May 29 2017
,
May 29 2017
I am spinning off a separate issue for making the readback asyc. This issue is about the double readback. Async readback: issue 727385
,
May 31 2017
Fixed by: https://chromium.googlesource.com/chromium/src/+/7afb5d6ecb4e07b91c16c85cee3e70d91467452c
,
May 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7afb5d6ecb4e07b91c16c85cee3e70d91467452c commit 7afb5d6ecb4e07b91c16c85cee3e70d91467452c Author: Justin Novosad <junov@chromium.org> Date: Wed May 31 19:18:04 2017 Fix redundant frame capture in stream capture from WebGL In CL https://codereview.chromium.org/2768683002, we neglected to consider that WebGL had an independent mechanism for signaling animation frames to canvas content listeners (like stream captures) The existing WebGL mechanism suffered the same problem as the 2D canvas mechanism: it is tied to compositing, and therefore does not trigger when the canvas is not visible. This CL simply removes the old code path to leave only the mechanism from CL https://codereview.chromium.org/2768683002, which works for both 2D canvas and WebGL, visible or not visible. BUG= 724664 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: Ia42596682eaba248d53787e5a41777cd80f584a6 Reviewed-on: https://chromium-review.googlesource.com/517764 Reviewed-by: Kenneth Russell <kbr@chromium.org> Commit-Queue: Justin Novosad <junov@chromium.org> Cr-Commit-Position: refs/heads/master@{#475980} [modify] https://crrev.com/7afb5d6ecb4e07b91c16c85cee3e70d91467452c/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp [modify] https://crrev.com/7afb5d6ecb4e07b91c16c85cee3e70d91467452c/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h [modify] https://crrev.com/7afb5d6ecb4e07b91c16c85cee3e70d91467452c/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp [modify] https://crrev.com/7afb5d6ecb4e07b91c16c85cee3e70d91467452c/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h |
||||
►
Sign in to add a comment |
||||
Comment 1 by emir...@chromium.org
, May 19 2017