Registering/Unregistering BeginFrameSource on visibility doesn't make sense to me. The expensive part of BeginFrameSource is the observers because that causes a timer to be scheduled.
fsamuel: Android doesn't use a timer, it's an OS signal for each OnBeginFrame(). Also the reason it's unregistered is because the viz::Display is destroyed when visibility changes to hidden anyways. The same code to recreate viz::Display and regregister BeginFrameSource gets used in all cases.
Ohh yea, I vaguely remember this conversation. We supposedly do this to return/save resources? Do we actually know if this is still the case? Won't Display::SetVisible(false) accomplish the desired effect?
Fady suggested that BeginFrames and CompositorFrameSink can be different message pipes and on Android BeginFrames can come from the browser. I think this idea would work.
Quick update:
It appears that the Choreographer service (which provides VSync / BeginFrame info on Android) can be run in the GPU process. There are two concerns in doing so:
#1 - Choreographer requires that the thread has a Java looper pumping it (not a native message loop). This should be a straightforward change.
#2 - There was concern that Choreographer would run on thread 0, so we'd need to move the Android GPU main thread to be thread 0 (it is currently a random thread we create later). It appears this isn't an issue. Per the docs "Each Looper thread has its own choreographer. Other threads can post callbacks to run on the choreographer but they will run on the Looper to which the choreographer belongs."
So only #1 needs to be addressed and we should be able to handle this in an ideal way. There might be some performance cost to running a Java looper, but hopefully this isn't major.
Comment 1 by ericrk@chromium.org
, Feb 13 2018