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

Issue 707507 link

Starred by 2 users
Status: Fixed
Owner:
█ fsam...@chromium.org
Last visit > 30 days ago
Closed: Apr 2017
Cc:
danakj@chromium.org
█ staraz@chromium.org
samans@chromium.org
rjkroege@chromium.org
vmp...@chromium.org
sadrul@chromium.org
█ mfomitchev@chromium.org
piman@chromium.org
kylec...@chromium.org
enne@chromium.org
sky@chromium.org
mustash-bugs@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Surface Synchronization: Consider interaction with visibility

Project Member Reported by fsamuel@google.com, Apr 1 2017 
With surface synchronization, the parent's CompositorFrame will block
on the child. If the child is hidden, it will not generate CompositorFrames. Thus, the parent will always wait 4 BeginFrames and activate on deadline. This effectively drops the system's frame rate to 15fps all the time.

Obviously this is undesirable. In Mus+Ash today, this is mitigated by not embedding surface IDs for a hidden child in the parent.

The problem now is what to do when the window is shown again? We need to trigger a new CompositorFrame...

One way is changing visibility to true allocates a new LocalSurfaceId and plumbs it down to the child which triggers a commit flow to generate a CompositorFrame.

Another way is to push knowledge of visiblity down into cc::Surface, and cc::SurfaceDependencyTracker. In that world, the "display compositor host" tells the "display compositor" that a given FrameSinkId is no longer visible.

This removes surface Ids from that particular FrameSinkId from blocking activation of any other surface Ids.

The first solution is perhaps easier to implement while the latter is more generic as it does not need to be implemented twice (for UI and OOPIF).

I'd like to implement the second solution. WDYT?

Comment 1 by fsam...@chromium.org, Apr 1 2017 

Thinking out loud:

FrameSinkManager::SetFrameSinkClientVisible(bool visible);
FrameSinkManager::IsFrameSinkClientVisible() const;

If the FrameSinkClient becomes NOT visible, then it calls out to the SurfaceDependencyTracker, maybe via a FrameSinkObserver thing (like PendingFrameObserver).

So..SurfaceDependencyTracker is a FrameSinkObserver.

SurfaceDependencyTracker::OnFrameSinkClientVisbilityChange(bool visible)...

Maybe SurfaceManager::GetSurfacesForFrameSinkId?

For each surfaceID of the given FrameSinkId:
  NotifySurfaceIdAvailable(surfaceID); <==== This unblocks parents.


The question is...what do we do if it becomes visible again?

Do we ADD a blocker as appropriate? Maybe? Certainly we also need to consider what to do when new CompositorFrames arrive.


I think as a first cut, we shouldn't block on not visible => visible, but we should UN-block on visible => not visible.

Comment 2 by kylec...@chromium.org, Apr 3 2017 

Can you clarify exactly what you mean by visible / not visible here? For example, is it some higher layers concept of visibility? Is it surfaces that are occluded? etc.

Comment 3 by mfomitchev@chromium.org, Apr 3 2017

Cc: mfomitchev@chromium.org
+1 for the second solution
This would also means that visibility changes could be handled entirely in Mus without a round-trip to the client, which seems like a good thing.

Comment 4 by vmi...@chromium.org, Apr 3 2017

Labels: -Type-Bug Type-Feature
Status: Available (was: Untriaged)
Set Available/Feature on this type of tracking issue so we don't have to go through them in our triage.

Comment 5 by fsam...@chromium.org, Apr 17 2017

Owner: fsam...@chromium.org
Status: Fixed (was: Available)
I'm marking as FIXED here. This was a problem with the way referenced_surfaces referred to ALL SurfaceLayers even hidden windows. This was fixed here:

https://codereview.chromium.org/2803913004/

and improved here:

https://codereview.chromium.org/2811813004/

Comment 6 by fsam...@chromium.org, Jun 13 2017

Blocking: -601863

Comment 7 by lafo...@chromium.org, Feb 26 2018

Components: -Internals>MUS Internals>Services>WindowService
►

Sign in to add a comment