New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 734811 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Closed: Jul 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 731255



Sign in to add a comment

Figure out why ScreenRotationAnimatorSmoothAnimationTests are failing in --mus

Project Member Reported by sky@chromium.org, Jun 19 2017

Issue description

They timeout. Need to investigate more thoroughly to understand why.
 

Comment 1 by sky@chromium.org, Jun 21 2017

Cc: fsam...@chromium.org sadrul@chromium.org m...@chromium.org
These tests are failing because Layer::CopyOutputRequest() returns an empty result, which makes the code early out, which the tests don't expect. So, once we make CopyOutputRequest() work, I supsect these tests will just work.

miu, is part of 695429 getting Layer::CopyOutputRequest working?

Comment 2 by sky@chromium.org, Jun 26 2017

Cc: danakj@chromium.org
This appears to happen because mus/mash tests use a FakeCompositorFactory, where as classic ash tests use an InProcessContextProvider. I'm not familiar enough with this code to know if it makes sense to make the CopyOutputRequests appear to succeed.

Dana, any suggestions?

Comment 3 by danakj@chromium.org, Jun 26 2017

Does FakeCompositorFactory implement any of it? It would need to return some non-empty CopyOutputResult. https://cs.chromium.org/chromium/src/cc/output/copy_output_request.h?rcl=16d4bfed7e1a4f29cdc9e471dbaaddfd12419e50&l=78

Comment 4 by danakj@chromium.org, Jun 26 2017

That said why would unit tests care if that's succeeding, unless they are looking for pixels.. in which case FakeCompositorFactory sounds like the wrong fit.

Comment 5 by sky@chromium.org, Jun 26 2017

It looks like an empty result is triggered when the RenderPass is destroyed, which triggers sending the empty result.

It's not the unit test that cares, it's the actual code under test. When the code under test sees the empty result it early outs, which triggers the test to hang. The code under tests uses the TextureMailbox from the result. See ScreenRotationAnimator::CopyLayerTree.

Ash when run against mus uses FakeContextFactory, I believe that's because the test code doesn't actually talk to mus.

Comment 6 by danakj@chromium.org, Jun 26 2017

Ah ok so if testing code logic with copyrequests in it, then I guess FakeContextFactory should be making some result with a made up texture id in it.

Comment 7 by sky@chromium.org, Jun 27 2017

I'm not familiar with FakeContextFactory (or the surrounding classes). Is there any similar code you could point me at?

Comment 8 by danakj@chromium.org, Jun 27 2017

These tests use fake GL contexts and test copy request logics.

https://cs.chromium.org/chromium/src/cc/trees/layer_tree_host_pixeltest_readback.cc?q=layer_tree_.*readback&sq=package:chromium&dr

> It looks like an empty result is triggered when the RenderPass is destroyed, which triggers sending the empty result

But actually, that sounds to me like the compositor is failing to draw or something? What's the callstack when the request is destroyed?

To me that doesn't sound like FakeContextFactory is wrong, a FakeLayerTreeFrameSink will generate beginframes https://cs.chromium.org/chromium/src/cc/test/fake_layer_tree_frame_sink.cc?rcl=7f008cf28fc67b1be4726b9ed0366401ca43ce63&l=37 and such, but that the test is doing something wrong.

Comment 9 by sky@chromium.org, Jun 27 2017

I think this is the backtrace:

#0  cc::PostCopyCallbackToMainThread (main_thread_task_runner=..., request=std::unique_ptr<cc::CopyOutputRequest> containing 0xc131acee4a0, result=std::unique_ptr<\
cc::CopyOutputResult> containing 0xc131af8e3e0) at ../../cc/layers/layer.cc:1133
#1  0x00007ffff174ce71 in base::internal::FunctorTraits<void (*)(scoped_refptr<base::SingleThreadTaskRunner>, std::unique_ptr<cc::CopyOutputRequest, std::default_d\
elete<cc::CopyOutputRequest> >, std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResult> >), void>::Invoke<scoped_refptr<base::SingleThreadT\
askRunner> const&, std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> >, std::unique_ptr<cc::CopyOutputResult, std::default_delete<c\
c::CopyOutputResult> > >(void (*)(scoped_refptr<base::SingleThreadTaskRunner>, std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> >,\
 std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResult> >), scoped_refptr<base::SingleThreadTaskRunner> const&, std::unique_ptr<cc::CopyOu\
tputRequest, std::default_delete<cc::CopyOutputRequest> >&&, std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResult> >&&) (function=0x7ffff\
17404e0 <cc::PostCopyCallbackToMainThread(scoped_refptr<base::SingleThreadTaskRunner>, std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputReq\
uest> >, std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResult> >)>, args=<unknown type in /secondary/builds/build1/src/out/Debug2/./libcc\
.so, CU 0x0, DIE 0x3f87b>, args=<unknown type in /secondary/builds/build1/src/out/Debug2/./libcc.so, CU 0x0, DIE 0x3f87b>, args=<unknown type in /secondary/builds/\
build1/src/out/Debug2/./libcc.so, CU 0x0, DIE 0x3f87b>) at ../../base/bind_internal.h:164
#2  0x00007ffff174cdb2 in base::internal::InvokeHelper<false, void>::MakeItSo<void (* const&)(scoped_refptr<base::SingleThreadTaskRunner>, std::unique_ptr<cc::Copy\
OutputRequest, std::default_delete<cc::CopyOutputRequest> >, std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResult> >), scoped_refptr<base\
::SingleThreadTaskRunner> const&, std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> >, std::unique_ptr<cc::CopyOutputResult, std::d\
efault_delete<cc::CopyOutputResult> > >(void (* const&)(scoped_refptr<base::SingleThreadTaskRunner>, std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc:\
:CopyOutputRequest> >, std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResult> >), scoped_refptr<base::SingleThreadTaskRunner> const&, std:\
:unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> >&&, std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResult> \
>&&) (functor=@0xc131ac91148: 0x7ffff17404e0 <cc::PostCopyCallbackToMainThread(scoped_refptr<base::SingleThreadTaskRunner>, std::unique_ptr<cc::CopyOutputRequest, \
std::default_delete<cc::CopyOutputRequest> >, std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResult> >)>, args=<unknown type in /secondary\
/builds/build1/src/out/Debug2/./libcc.so, CU 0x0, DIE 0x3f821>, args=<unknown type in /secondary/builds/build1/src/out/Debug2/./libcc.so, CU 0x0, DIE 0x3f821>, arg\
s=<unknown type in /secondary/builds/build1/src/out/Debug2/./libcc.so, CU 0x0, DIE 0x3f821>) at ../../base/bind_internal.h:275
#3  0x00007ffff174cd38 in base::internal::Invoker<base::internal::BindState<void (*)(scoped_refptr<base::SingleThreadTaskRunner>, std::unique_ptr<cc::CopyOutputReq\
uest, std::default_delete<cc::CopyOutputRequest> >, std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResult> >), scoped_refptr<base::SingleT\
hreadTaskRunner>, base::internal::PassedWrapper<std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> > > >, void (std::unique_ptr<cc::\
CopyOutputResult, std::default_delete<cc::CopyOutputResult> >)>::RunImpl<void (* const&)(scoped_refptr<base::SingleThreadTaskRunner>, std::unique_ptr<cc::CopyOutpu\
tRequest, std::default_delete<cc::CopyOutputRequest> >, std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResult> >), std::tuple<scoped_refpt\
r<base::SingleThreadTaskRunner>, base::internal::PassedWrapper<std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> > > > const&, 0ul,\
 1ul>(void (* const&)(scoped_refptr<base::SingleThreadTaskRunner>, std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> >, std::unique\
_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResult> >), std::tuple<scoped_refptr<base::SingleThreadTaskRunner>, base::internal::PassedWrapper<std:\
:unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> > > > const&, base::IndexSequence<0ul, 1ul>, std::unique_ptr<cc::CopyOutputResult, st\
d::default_delete<cc::CopyOutputResult> >&&) (functor=@0xc131ac91148: 0x7ffff17404e0 <cc::PostCopyCallbackToMainThread(scoped_refptr<base::SingleThreadTaskRunner>,\
 std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> >, std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResul\
t> >)>, bound=empty std::tuple, unbound_args=<unknown type in /secondary/builds/build1/src/out/Debug2/./libcc.so, CU 0x0, DIE 0x34d11>) at ../../base/bind_internal\
.h:351
#4  0x00007ffff174cc3c in base::internal::Invoker<base::internal::BindState<void (*)(scoped_refptr<base::SingleThreadTaskRunner>, std::unique_ptr<cc::CopyOutputReq\
uest, std::default_delete<cc::CopyOutputRequest> >, std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResult> >), scoped_refptr<base::SingleT\
hreadTaskRunner>, base::internal::PassedWrapper<std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> > > >, void (std::unique_ptr<cc::\
CopyOutputResult, std::default_delete<cc::CopyOutputResult> >)>::Run(base::internal::BindStateBase*, std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::\
CopyOutputResult> >&&) (base=0xc131ac91120, unbound_args=<unknown type in /secondary/builds/build1/src/out/Debug2/./libcc.so, CU 0x0, DIE 0x34c9d>) at ../../base/b\
ind_internal.h:329
#5  0x00007ffff17af4d3 in base::Callback<void (std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResult> >), (base::internal::CopyMode)1, (ba\
se::internal::RepeatMode)1>::Run(std::unique_ptr<cc::CopyOutputResult, std::default_delete<cc::CopyOutputResult> >) && (this=0x7fffffffa678, args=std::unique_ptr<c\
c::CopyOutputResult> containing 0x0) at ../../base/callback.h:91
#6  0x00007ffff17ae89b in cc::CopyOutputRequest::SendResult (this=0xc131ad956e0, result=std::unique_ptr<cc::CopyOutputResult> containing 0x0) at ../../cc/output/co\
py_output_request.cc:46
#7  0x00007ffff17ae7f3 in cc::CopyOutputRequest::~CopyOutputRequest (this=0xc131ad956e0) at ../../cc/output/copy_output_request.cc:41
#8  0x00007ffff17467db in std::default_delete<cc::CopyOutputRequest>::operator() (this=0xc131adfde20, __ptr=0xc131ad956e0) at ../../build/linux/debian_jessie_amd64\
-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/unique_ptr.h:67
#9  0x00007ffff17446a3 in std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> >::~unique_ptr (this=0xc131adfde20) at ../../build/linu\
x/debian_jessie_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/unique_ptr.h:184
#10 0x00007ffff17470a5 in std::_Destroy<std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> > > (__pointer=0xc131adfde20) at ../../bu\
ild/linux/debian_jessie_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_construct.h:93
#11 0x00007ffff174706f in std::_Destroy_aux<false>::__destroy<std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> >*> (__first=0xc131\
adfde20, __last=0xc131adfde28) at ../../build/linux/debian_jessie_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_construct.h:1\
03
#12 0x00007ffff174702d in std::_Destroy<std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> >*> (__first=0xc131adfde20, __last=0xc131\
adfde28) at ../../build/linux/debian_jessie_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_construct.h:126
#13 0x00007ffff1746fc1 in std::_Destroy<std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> >*, std::unique_ptr<cc::CopyOutputRequest\
, std::default_delete<cc::CopyOutputRequest> > > (__first=0xc131adfde20, __last=0xc131adfde28) at ../../build/linux/debian_jessie_amd64-sysroot/usr/lib/gcc/x86_64-\
linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_construct.h:151
#14 0x00007ffff1746f8e in std::__cxx1998::vector<std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> >, std::allocator<std::unique_pt\
r<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> > > >::~vector (this=0xc131aed66a0) at ../../build/linux/debian_jessie_amd64-sysroot/usr/lib/gc\
c/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_vector.h:415
#15 0x00007ffff174201f in std::__debug::vector<std::unique_ptr<cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> >, std::allocator<std::unique_ptr<\
cc::CopyOutputRequest, std::default_delete<cc::CopyOutputRequest> > > >::~vector (this=0xc131aed66a0) at ../../build/linux/debian_jessie_amd64-sysroot/usr/lib/gcc/\
x86_64-linux-gnu/4.8/../../../../include/c++/4.8/debug/vector:144
#16 0x00007ffff18349eb in cc::RenderPass::~RenderPass (this=0xc131aed6560) at ../../cc/quads/render_pass.cc:91
#17 0x00007ffff1777c8b in std::default_delete<cc::RenderPass>::operator() (this=0xc131a927ac0, __ptr=0xc131aed6560) at ../../build/linux/debian_jessie_amd64-sysroo\
t/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/unique_ptr.h:67
#18 0x00007ffff17772b3 in std::unique_ptr<cc::RenderPass, std::default_delete<cc::RenderPass> >::~unique_ptr (this=0xc131a927ac0) at ../../build/linux/debian_jessi\
e_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/unique_ptr.h:184
#19 0x00007ffff17aa505 in std::_Destroy<std::unique_ptr<cc::RenderPass, std::default_delete<cc::RenderPass> > > (__pointer=0xc131a927ac0) at ../../build/linux/debi\
an_jessie_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_construct.h:93
#20 0x00007ffff17aa4cf in std::_Destroy_aux<false>::__destroy<std::unique_ptr<cc::RenderPass, std::default_delete<cc::RenderPass> >*> (__first=0xc131a927ac0, __las\
t=0xc131a927ad0) at ../../build/linux/debian_jessie_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_construct.h:103
#21 0x00007ffff17aa48d in std::_Destroy<std::unique_ptr<cc::RenderPass, std::default_delete<cc::RenderPass> >*> (__first=0xc131a927ac0, __last=0xc131a927ad0) at ..\
/../build/linux/debian_jessie_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/stl_construct.h:126
#22 0x00007ffff17aa421 in std::_Destroy<std::unique_ptr<cc::RenderPass, std::default_delete<cc::RenderPass> >*, std::unique_ptr<cc::RenderPass, std::default_delete\
<cc::RenderPass> > > (__first=0xc131a927ac0, __last=0xc131a927ad0) at ../../build/linux/debian_jessie_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../in\
clude/c++/4.8/bits/stl_construct.h:151
#23 0x00007ffff17aac4e in std::__cxx1998::vector<std::unique_ptr<cc::RenderPass, std::default_delete<cc::RenderPass> >, std::allocator<std::unique_ptr<cc::RenderPa\
ss, std::default_delete<cc::RenderPass> > > >::~vector (this=0xc131aedccf8) at ../../build/linux/debian_jessie_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../..\
/../../include/c++/4.8/bits/stl_vector.h:415
#24 0x00007ffff17aa02f in std::__debug::vector<std::unique_ptr<cc::RenderPass, std::default_delete<cc::RenderPass> >, std::allocator<std::unique_ptr<cc::RenderPass\
, std::default_delete<cc::RenderPass> > > >::~vector (this=0xc131aedccf8) at ../../build/linux/debian_jessie_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../.\
./../include/c++/4.8/debug/vector:144
#25 0x00007ffff17a9d75 in cc::CompositorFrame::~CompositorFrame (this=0xc131aedcb60) at ../../cc/output/compositor_frame.cc:13
#26 0x0000000000e1e15b in std::default_delete<cc::CompositorFrame>::operator() (this=0xc131a7ef5c8, __ptr=0xc131aedcb60) at ../../build/linux/debian_jessie_amd64-s\
ysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/unique_ptr.h:67
#27 0x0000000000e1d78c in std::unique_ptr<cc::CompositorFrame, std::default_delete<cc::CompositorFrame> >::reset (this=0xc131a7ef5c8, __p=0xc131aedcb60) at ../../b\
uild/linux/debian_jessie_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/unique_ptr.h:262
#28 0x0000000000e1d149 in cc::FakeLayerTreeFrameSink::SubmitCompositorFrame (this=0xc131a7ef420, frame=...) at ../../cc/test/fake_layer_tree_frame_sink.cc:52
#29 0x00007ffff19c75af in cc::LayerTreeHostImpl::DrawLayers (this=0xc131a84d020, frame=0x7fffffffbe18) at ../../cc/trees/layer_tree_host_impl.cc:1765
#30 0x00007ffff1a87523 in cc::SingleThreadProxy::DoComposite (this=0xc131a7dcaa0, frame=0x7fffffffbe18) at ../../cc/trees/single_thread_proxy.cc:593
#31 0x00007ffff1a884c3 in cc::SingleThreadProxy::ScheduledActionDrawIfPossible (this=0xc131a7dcaa0) at ../../cc/trees/single_thread_proxy.cc:760
#32 0x00007ffff18d4eb6 in cc::Scheduler::DrawIfPossible (this=0xc131a5ef620) at ../../cc/scheduler/scheduler.cc:582
#33 0x00007ffff18d089c in cc::Scheduler::ProcessScheduledActions (this=0xc131a5ef620) at ../../cc/scheduler/scheduler.cc:675
#34 0x00007ffff18d0492 in cc::Scheduler::OnBeginImplFrameDeadline (this=0xc131a5ef620) at ../../cc/scheduler/scheduler.cc:570
#35 0x00007ffff172816f in base::internal::FunctorTraits<void (base::CancelableCallback<void ()>::*)() const, void>::Invoke<base::WeakPtr<base::CancelableCallback<v\
oid ()> > const&>(void (base::CancelableCallback<void ()>::*)() const, base::WeakPtr<base::CancelableCallback<void ()> > const&) (method=(void (base::CancelableCal\
lback<void ()>::*)(const base::CancelableCallback<void ()> * const)) 0x7ffff18d0350 <cc::Scheduler::OnBeginImplFrameDeadline()>, receiver_ptr=...) at ../../base/bi\
nd_internal.h:224
#36 0x00007ffff18d7bca in base::internal::InvokeHelper<true, void>::MakeItSo<void (cc::Scheduler::* const&)(), base::WeakPtr<cc::Scheduler> const&> (functor=@0xc13\
1a751fc8: (void (cc::Scheduler::*)(cc::Scheduler * const)) 0x7ffff18d0350 <cc::Scheduler::OnBeginImplFrameDeadline()>, weak_ptr=...) at ../../base/bind_internal.h:\
295
#37 0x00007ffff18d7b52 in base::internal::Invoker<base::internal::BindState<void (cc::Scheduler::*)(), base::WeakPtr<cc::Scheduler> >, void ()>::RunImpl<void (cc::\
Scheduler::* const&)(), std::tuple<base::WeakPtr<cc::Scheduler> > const&, 0ul>(void (cc::Scheduler::* const&)(), std::tuple<base::WeakPtr<cc::Scheduler> > const&, \
base::IndexSequence<0ul>) (functor=@0xc131a751fc8: (void (cc::Scheduler::*)(cc::Scheduler * const)) 0x7ffff18d0350 <cc::Scheduler::OnBeginImplFrameDeadline()>, bou\
nd=empty std::tuple) at ../../base/bind_internal.h:351
#38 0x00007ffff18d7a9c in base::internal::Invoker<base::internal::BindState<void (cc::Scheduler::*)(), base::WeakPtr<cc::Scheduler> >, void ()>::Run(base::internal\
#28 0x0000000000e1d149 in cc::FakeLayerTreeFrameSink::SubmitCompositorFrame (this=0xc131a7ef420, frame=...) at ../../cc/test/fake_layer_tree_frame_sink.cc:52

FakeLayerTreeFrameSink::SubmitCompositorFrame() does not actually do anything with the CompositorFrame so it won't reply to a request. And the next submitted frame will delete them.

https://cs.chromium.org/chromium/src/cc/test/fake_layer_tree_frame_sink.cc?rcl=ac1834dbfa2f5ceb60187e5526d868e5d9802032&l=52

If you want things to get drawn then you need a display compositor, ala TestLayerTreeFrameSink, which owns such a thing, and would draw and respond to copy requests. https://cs.chromium.org/chromium/src/cc/test/test_layer_tree_frame_sink.h?rcl=94f28c6da612384d34cffaea539e29efb287236b&l=128
> If you want things to get drawn 

Tbc, "drawn" here is just going thru the motions still, with a fake gl context.

Comment 12 by sky@chromium.org, Jun 28 2017

I got further with using TextLayerTreeFrameSink. I'm now at one test hitting a DCHECK. The DCHECK I'm hitting is here https://cs.chromium.org/chromium/src/cc/test/test_web_graphics_context_3d.cc?q=TestWebGraphicsContext3D&sq=package:chromium&dr=CSs&l=346 . texture_id is 2. Here's the full stack:

#0  base::debug::(anonymous namespace)::DebugBreak () at ../../base/debug/debugger_posix.cc:239
#1  0x00007ffff674f878 in base::debug::BreakDebugger () at ../../base/debug/debugger_posix.cc:258
#2  0x00007ffff67be5b2 in logging::LogMessage::~LogMessage (this=0x7fffffff81d0) at ../../base/logging.cc:784
#3  0x0000000000e38cd9 in cc::TestWebGraphicsContext3D::bindTexture (this=0x3d3db819a520, target=3553, texture_id=2) at ../../cc/test/test_web_graphics_context_3d.\
cc:346
#4  0x0000000000e2a0e9 in cc::TestGLES2Interface::BindTexture (this=0x3d3db81bd250, target=3553, texture=2) at ../../cc/test/test_gles2_interface.cc:78
#5  0x00007ffff17d822e in cc::GLRenderer::UpdateRPDQTexturesForSampling (this=0x3d3db7e78820, params=0x7fffffff8d80) at ../../cc/output/gl_renderer.cc:1286
#6  0x00007ffff17d6c4f in cc::GLRenderer::DrawRenderPassQuadInternal (this=0x3d3db7e78820, params=0x7fffffff8d80) at ../../cc/output/gl_renderer.cc:1082
#7  0x00007ffff17d1b1b in cc::GLRenderer::DrawRenderPassQuad (this=0x3d3db7e78820, quad=0x3d3db85a3440, clip_region=0x0) at ../../cc/output/gl_renderer.cc:1067
#8  0x00007ffff17d0bee in cc::GLRenderer::DoDrawQuad (this=0x3d3db7e78820, quad=0x3d3db85a3440, clip_region=0x0) at ../../cc/output/gl_renderer.cc:587
#9  0x00007ffff17b6df5 in cc::DirectRenderer::DrawRenderPass (this=0x3d3db7e78820, render_pass=0x3d3db855fc60) at ../../cc/output/direct_renderer.cc:594
#10 0x00007ffff17b5cda in cc::DirectRenderer::DrawRenderPassAndExecuteCopyRequests (this=0x3d3db7e78820, render_pass=0x3d3db855fc60) at ../../cc/output/direct_rend\
erer.cc:491
#11 0x00007ffff17b5b0f in cc::DirectRenderer::DrawFrame (this=0x3d3db7e78820, render_passes_in_draw_order=0x7fffffffaf48, device_scale_factor=1, device_viewport_si\
ze=...) at ../../cc/output/direct_renderer.cc:354
#12 0x00007ffff02fe406 in cc::Display::DrawAndSwap (this=0x3d3db7f4a920) at ../../cc/surfaces/display.cc:312
#13 0x00007ffff030d35b in cc::DisplayScheduler::DrawAndSwap (this=0x3d3db7f4c820) at ../../cc/surfaces/display_scheduler.cc:193
#14 0x00007ffff030c386 in cc::DisplayScheduler::AttemptDrawAndSwap (this=0x3d3db7f4c820) at ../../cc/surfaces/display_scheduler.cc:405
#15 0x00007ffff030bc9f in cc::DisplayScheduler::OnBeginFrameDeadline (this=0x3d3db7f4c820) at ../../cc/surfaces/display_scheduler.cc:421
#16 0x00007ffff02edc8f in base::internal::FunctorTraits<void (cc::CompositorFrameSinkSupport::*)(), void>::Invoke<base::WeakPtr<cc::CompositorFrameSinkSupport> con\
st&> (method=(void (cc::CompositorFrameSinkSupport::*)(cc::CompositorFrameSinkSupport * const)) 0x7ffff030bb10 <cc::DisplayScheduler::OnBeginFrameDeadline()>, rece\
iver_ptr=...) at ../../base/bind_internal.h:209
#17 0x00007ffff031152a in base::internal::InvokeHelper<true, void>::MakeItSo<void (cc::DisplayScheduler::* const&)(), base::WeakPtr<cc::DisplayScheduler> const&> (\
functor=@0x3d3db827b9c8: (void (cc::DisplayScheduler::*)(cc::DisplayScheduler * const)) 0x7ffff030bb10 <cc::DisplayScheduler::OnBeginFrameDeadline()>, weak_ptr=...\
) at ../../base/bind_internal.h:295
#18 0x00007ffff03114b2 in base::internal::Invoker<base::internal::BindState<void (cc::DisplayScheduler::*)(), base::WeakPtr<cc::DisplayScheduler> >, void ()>::RunI\
mpl<void (cc::DisplayScheduler::* const&)(), std::tuple<base::WeakPtr<cc::DisplayScheduler> > const&, 0ul>(void (cc::DisplayScheduler::* const&)(), std::tuple<base\
::WeakPtr<cc::DisplayScheduler> > const&, base::IndexSequence<0ul>) (functor=@0x3d3db827b9c8: (void (cc::DisplayScheduler::*)(cc::DisplayScheduler * const)) 0x7fff\
f030bb10 <cc::DisplayScheduler::OnBeginFrameDeadline()>, bound=empty std::tuple) at ../../base/bind_internal.h:351
#19 0x00007ffff03113fc in base::internal::Invoker<base::internal::BindState<void (cc::DisplayScheduler::*)(), base::WeakPtr<cc::DisplayScheduler> >, void ()>::Run(\
base::internal::BindStateBase*) (base=0x3d3db827b9a0) at ../../base/bind_internal.h:329
#20 0x00007ffff03148fd in base::Callback<void (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>::Run() const & (this=0x3d3db7f4c900) at ../../base/ca\
llback.h:80
#21 0x00007ffff03143a9 in base::CancelableCallback<void ()>::Forward() const (this=0x3d3db7f4c8f8) at ../../base/cancelable_callback.h:110
#22 0x00007ffff02edc8f in base::internal::FunctorTraits<void (cc::CompositorFrameSinkSupport::*)(), void>::Invoke<base::WeakPtr<cc::CompositorFrameSinkSupport> con\
st&> (method=(void (cc::CompositorFrameSinkSupport::*)(cc::CompositorFrameSinkSupport * const)) 0x7ffff0314390 <base::CancelableCallback<void ()>::Forward() const>\
, receiver_ptr=...) at ../../base/bind_internal.h:209
#23 0x00007ffff031459a in base::internal::InvokeHelper<true, void>::MakeItSo<void (base::CancelableCallback<void ()>::* const&)() const, base::WeakPtr<base::Cancel\
ableCallback<void ()> > const&>(void (base::CancelableCallback<void ()>::* const&)() const, base::WeakPtr<base::CancelableCallback<void ()> > const&) (functor=@0x3\
d3db7f56f48: (void (base::CancelableCallback<void ()>::*)(const base::CancelableCallback<void ()> * const)) 0x7ffff0314390 <base::CancelableCallback<void ()>::Forw\
ard() const>, weak_ptr=...) at ../../base/bind_internal.h:295
#24 0x00007ffff0314522 in base::internal::Invoker<base::internal::BindState<void (base::CancelableCallback<void ()>::*)() const, base::WeakPtr<base::CancelableCall\
back<void ()> > >, void ()>::RunImpl<void (base::CancelableCallback<void ()>::* const&)() const, std::tuple<base::WeakPtr<base::CancelableCallback<void ()> > > con\
st&, 0ul>(void (base::CancelableCallback<void ()>::* const&)() const, std::tuple<base::WeakPtr<base::CancelableCallback<void ()> > > const&, base::IndexSequence<0u\
l>) (functor=@0x3d3db7f56f48: (void (base::CancelableCallback<void ()>::*)(const base::CancelableCallback<void ()> * const)) 0x7ffff0314390 <base::CancelableCallba\
ck<void ()>::Forward() const>, bound=empty std::tuple) at ../../base/bind_internal.h:351
#25 0x00007ffff031446c in base::internal::Invoker<base::internal::BindState<void (base::CancelableCallback<void ()>::*)() const, base::WeakPtr<base::CancelableCall\
back<void ()> > >, void ()>::Run(base::internal::BindStateBase*) (base=0x3d3db7f56f20) at ../../base/bind_internal.h:329
#26 0x00007ffff6711a5e in base::Callback<void (), (base::internal::CopyMode)0, (base::internal::RepeatMode)0>::Run() && (this=0x7fffffffbe00) at ../../base/callbac\
k.h:91
#27 0x00007ffff6757ec1 in base::debug::TaskAnnotator::RunTask (this=0x3d3db7d386e0, queue_function=0x7ffff6a5f4dc "MessageLoop::PostTask", pending_task=0x7fffffffb\
de8) at ../../base/debug/task_annotator.cc:59


WIP patch is here: http://chromium-review.googlesource.com/550587 .
Cool, maybe you need to track where this "2" comes from, sounds like it wasn't from the context's CreateTexture methods.

Comment 14 by sky@chromium.org, Jun 28 2017

The 2 comes from GLRenderer::UpdateRPDQTexturesForSampling().
Specifically.
    GLuint filter_image_id =
        skia::GrBackendObjectToGrGLTextureInfo(
          params->filter_image->getTextureHandle(true, &origin))
            ->fID;

returns 2

More specifically, SkImage_Gpu::onGetTextureHandle returns texture->getTextureHandle()

Which is:

GrBackendObject GrGLTexture::getTextureHandle() const {                          
  return reinterpret_cast<GrBackendObject>(&fInfo);                            }

And the fID of that is 2.
The |filter_image| is the thing with the |texture| then which it pulls the fID off right?

filter_image comes from https://cs.chromium.org/chromium/src/cc/output/gl_renderer.cc?rcl=24ddf7e038dd86c52bc1998b83416627e0e9d592&l=1253

Which should use the context to make that texture id, you could track that down in there.

It's possible there's some faked method that isn't hooked up just right that skia uses but cc doesn't, and we don't have any tests that use renderpass filters that aren't pixel tests (ie that use fake GL contexts).

Comment 16 by sky@chromium.org, Jun 28 2017

The 2 is coming from NullInterface, specifically https://cs.chromium.org/chromium/src/third_party/skia/src/gpu/gl/GrGLCreateNullInterface.cpp?q=grglcreatenullinterface.cpp&sq=package:chromium&dr&l=772 . TestContextProvider creates NullInterface via the function GrGLCreateNullInterface. Does which implementation GrGLCreateNullInterface creates depend upon the link target? Should I possibly be linking with something other than I am now?
Ah

https://cs.chromium.org/chromium/src/cc/test/test_context_provider.cc?rcl=f9295c391265f2d6cf4e5c39e08102de6208e96b&l=146

This should be binding skia to the fake context instead of to null. Probably with a https://cs.chromium.org/chromium/src/gpu/skia_bindings/grcontext_for_gles2_interface.h?rcl=f9295c391265f2d6cf4e5c39e08102de6208e96b&l=26

That used to be in content/ so we couldn't use it from cc. Maybe it's easy now, as long as the context supports enough things for skia to not consider it broken, it might need to return some strings in the right places IIRC.

https://bugs.chromium.org/p/chromium/issues/detail?id=332502 was about this. Null has been enough until now I guess, as we didn't try to bind in the compositor context textures from the skia context without going thru mailboxes (for the display compositor). Render pass filters require binding them in the same context.

Comment 18 by sky@chromium.org, Jun 28 2017

Converting to skia_bindings::GrContextForGLES2Interface gets me further. It actually make this test (and all ash_unittests) to pass. I converted TestContextProvider to use GrContextForGLES2Interface , but this causes other tests (in cc_unittests) to crash trying to access GrContext. GrContext is null because GrGLInterface::validate() fails (GrScissorState::fEnabled is false).

I'm likely missing some wiring.

Patch for this is here: https://chromium-review.googlesource.com/c/550587/ .
> it might need to return some strings in the right places IIRC.

That's what I was thinking here. Maybe some required extensions need to be reported.

https://cs.chromium.org/chromium/src/cc/test/test_gles2_interface.h?rcl=9b345e436c2687898d7a9d8bcff509230b903607&l=13

TestGLES2Interface doesn't override GetString at all right now, so no extensions exist.

Comment 20 by sky@chromium.org, Jun 28 2017

Is there a list of extensions some where? Or just keep running tests until GrContext gets created?
I am not sure what skia would need, I think the latter. You might be able to shortcut with bsaloman@ but otherwise I would run it and see what it's looking for and add them. For each one, maybe check the spec for it and see if TestGLES2Interface needs do something to not have callers crash.

Comment 22 by sky@chromium.org, Jul 12 2017

I got a bit further. I copied parts of NullInterface (in skia) into a TestGLES2Interface:

class TestGLES2Interface2 : public TestGLES2Interface {
 public:
  TestGLES2Interface2() {
    extensions_.push_back("GL_EXT_stencil_wrap");
    extension_string_ = extensions_[0];
    for (size_t i = 1; i < extensions_.size(); ++i)
      extension_string_ += extensions_[i];
  }
  ~TestGLES2Interface2() override = default;

  // TestGLES2Interface2:
  const GLubyte* GetString(GLenum name) override {
    switch (name) {
      case GL_EXTENSIONS:
        return reinterpret_cast<const GLubyte*>(extension_string_.c_str());
      case GL_VERSION:
        return reinterpret_cast<const GrGLubyte*>("4.0 Null GL");
      case GL_SHADING_LANGUAGE_VERSION:
        return reinterpret_cast<const GrGLubyte*>("4.20.8 Null GLSL");
      case GL_VENDOR:
        return reinterpret_cast<const GrGLubyte*>("Null Vendor");
      case GL_RENDERER:
        return reinterpret_cast<const GrGLubyte*>("The Null (Non-)Renderer");
    }
    return nullptr;
  }
  const GrGLubyte* GetStringi(GrGLenum name, GrGLuint i) override {
    if (name == GL_EXTENSIONS && i < extensions_.size())
      return reinterpret_cast<const GLubyte*>(extensions_[i].c_str());
    return nullptr;
  }
  void GetIntegerv(GLenum name, GLint* params) override {
    switch (name) {
      case GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS:
        *params = 8;
        return;
      default:
        break;
    }
    TestGLES2Interface::GetIntegerv(name, params);
  }

 private:
  std::vector<std::string> extensions_;
  std::string extension_string_;

  DISALLOW_COPY_AND_ASSIGN(TestGLES2Interface2);
};

Which causes more tests to pass, but I'm now stuck at some tests that seem to be expecting GL, not GLES. GpuImageDecodeCacheTests/GpuImageDecodeCacheTest crashes with my patch. I believe the place leading to things going bad is GrGLCaps::isConfigTexturable(kBGRA_8888_GrPixelConfig) returning false. With my patch GrGLCaps is configured with kGLES_GrGLStandard, which leads to a different set of capabilities as compared to tip-of-tree, which uses kGL_GrGLStandard.

I could try to get the new code to use kGL_GrGLStandard, but then I'm not sure if that is appropriate given the code is subclassing TestGLES2Interface. WIP patch is here: https://chromium-review.googlesource.com/c/550587/ . Any suggestions?
Project Member

Comment 23 by bugdroid1@chromium.org, Jul 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/260dabfcdd2d0021f00b0e97ae97fc1fc38ee10d

commit 260dabfcdd2d0021f00b0e97ae97fc1fc38ee10d
Author: Scott Violet <sky@chromium.org>
Date: Fri Jul 14 17:17:51 2017

chromeos: enables more of non-fake cc for CopyOutputRequest to work

Specifically the following:
. TestContextProvider now uses
  skia_bindings::GrContextForGLES2Interface, without this skia doesn't
  use the right context and produces ids that aren't expected.
. Converts Ash to use TestLayerTreeFrameSinkClient vs
  FakeContextFactory. TestLayerTreeFrameSinkClient seems closer to
  what these tests need vs FakeContextFactory.

BUG= 734811 
TEST=test only changes

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I4f3d7a432d7f23a9e1e243d045846949b52c51fc
Reviewed-on: https://chromium-review.googlesource.com/550587
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486789}
[modify] https://crrev.com/260dabfcdd2d0021f00b0e97ae97fc1fc38ee10d/ash/BUILD.gn
[modify] https://crrev.com/260dabfcdd2d0021f00b0e97ae97fc1fc38ee10d/ash/test/DEPS
[modify] https://crrev.com/260dabfcdd2d0021f00b0e97ae97fc1fc38ee10d/ash/test/ash_test_suite.cc
[modify] https://crrev.com/260dabfcdd2d0021f00b0e97ae97fc1fc38ee10d/ash/test/ash_test_suite.h
[modify] https://crrev.com/260dabfcdd2d0021f00b0e97ae97fc1fc38ee10d/cc/test/test_context_provider.cc
[modify] https://crrev.com/260dabfcdd2d0021f00b0e97ae97fc1fc38ee10d/cc/test/test_context_provider.h
[modify] https://crrev.com/260dabfcdd2d0021f00b0e97ae97fc1fc38ee10d/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/260dabfcdd2d0021f00b0e97ae97fc1fc38ee10d/components/exo/BUILD.gn
[modify] https://crrev.com/260dabfcdd2d0021f00b0e97ae97fc1fc38ee10d/testing/buildbot/filters/ash_unittests_mus.filter
[modify] https://crrev.com/260dabfcdd2d0021f00b0e97ae97fc1fc38ee10d/ui/compositor/test/fake_context_factory.h

Comment 24 by sky@chromium.org, Jul 14 2017

Status: Fixed (was: Assigned)

Comment 25 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment