Skip to content

Commit

Permalink
Add Satisfy() and Require() to MojoCompositorFrameSink
Browse files Browse the repository at this point in the history
Sometimes Wayland client requires a surface before it is created by
SubmitCompositorFrame() since the request and creation use different message
pipes. This CL moves the implementation of Require and Satisfy to
CompositorFrameSink so that they share one message pipe with
SubmitCompositorFrame and things would happen in the correct order.

This is a temporary fix. kylechar@'s change of surface reference (CL 2541683004)
would formally address this issue.

BUG=659601
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2551083003
Cr-Commit-Position: refs/heads/master@{#436806}
  • Loading branch information
staraz authored and Commit bot committed Dec 7, 2016
1 parent a170f58 commit e6b29c4
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 0 deletions.
11 changes: 11 additions & 0 deletions cc/ipc/mojo_compositor_frame_sink.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import "cc/ipc/frame_sink_id.mojom";
import "cc/ipc/local_frame_id.mojom";
import "cc/ipc/surface_reference.mojom";
import "cc/ipc/returned_resource.mojom";
import "cc/ipc/surface_sequence.mojom";

// A MojoCompositorFrameSink is an interface for receiving CompositorFrame
// structs. A CompositorFrame contains the complete output meant for display.
Expand Down Expand Up @@ -42,6 +43,16 @@ interface MojoCompositorFrameSink {
// that its resources gets returned in time.
EvictFrame();

// TODO(staraz): Delete Require() and Satisfy() once surface references
// (CL 2541683004) are ready.
// Add the provided |sequence| as a destruction dependency of the
// surface associated with the provided |local_frame_id|.
Require(cc.mojom.LocalFrameId local_frame_id,
cc.mojom.SurfaceSequence sequence);

// Mark the sequence as satisfied and garbage collect surfaces.
Satisfy(cc.mojom.SurfaceSequence sequence);

// TODO(fsamuel): ReadbackBitmap API would be useful here.
};

Expand Down
17 changes: 17 additions & 0 deletions cc/surfaces/compositor_frame_sink_support.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "cc/scheduler/begin_frame_source.h"
#include "cc/surfaces/compositor_frame_sink_support_client.h"
#include "cc/surfaces/display.h"
#include "cc/surfaces/surface.h"
#include "cc/surfaces/surface_manager.h"

namespace cc {
Expand Down Expand Up @@ -83,6 +84,22 @@ void CompositorFrameSinkSupport::SubmitCompositorFrame(
}
}

void CompositorFrameSinkSupport::Require(const LocalFrameId& local_frame_id,
const SurfaceSequence& sequence) {
Surface* surface = surface_manager_->GetSurfaceForId(
SurfaceId(frame_sink_id_, local_frame_id));
if (!surface) {
DLOG(ERROR) << "Attempting to require callback on nonexistent surface";
return;
}
surface->AddDestructionDependency(sequence);
}

void CompositorFrameSinkSupport::Satisfy(const SurfaceSequence& sequence) {
std::vector<uint32_t> sequences = {sequence.sequence};
surface_manager_->DidSatisfySequences(sequence.frame_sink_id, &sequences);
}

void CompositorFrameSinkSupport::DidReceiveCompositorFrameAck() {
DCHECK_GT(ack_pending_count_, 0);
ack_pending_count_--;
Expand Down
3 changes: 3 additions & 0 deletions cc/surfaces/compositor_frame_sink_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ class CC_SURFACES_EXPORT CompositorFrameSinkSupport
void SetNeedsBeginFrame(bool needs_begin_frame);
void SubmitCompositorFrame(const LocalFrameId& local_frame_id,
CompositorFrame frame);
void Require(const LocalFrameId& local_frame_id,
const SurfaceSequence& sequence);
void Satisfy(const SurfaceSequence& sequence);
void AddChildFrameSink(const FrameSinkId& child_frame_sink_id);
void RemoveChildFrameSink(const FrameSinkId& child_frame_sink_id);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,19 @@ void OffscreenCanvasCompositorFrameSink::EvictFrame() {
NOTIMPLEMENTED();
}

void OffscreenCanvasCompositorFrameSink::Require(
const cc::LocalFrameId& local_frame_id,
const cc::SurfaceSequence& sequence) {
// TODO(staraz): Implement this.
NOTIMPLEMENTED();
}

void OffscreenCanvasCompositorFrameSink::Satisfy(
const cc::SurfaceSequence& sequence) {
// TODO(staraz): Implement this.
NOTIMPLEMENTED();
}

void OffscreenCanvasCompositorFrameSink::ReturnResources(
const cc::ReturnedResourceArray& resources) {
if (resources.empty())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ class OffscreenCanvasCompositorFrameSink
void RemoveSurfaceReferences(
const std::vector<cc::SurfaceReference>& references) override;
void EvictFrame() override;
void Require(const cc::LocalFrameId& local_frame_id,
const cc::SurfaceSequence& sequence) override;
void Satisfy(const cc::SurfaceSequence& sequence) override;

// cc::SurfaceFactoryClient implementation.
void ReturnResources(const cc::ReturnedResourceArray& resources) override;
Expand Down
9 changes: 9 additions & 0 deletions services/ui/surfaces/gpu_compositor_frame_sink.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ void GpuCompositorFrameSink::RemoveSurfaceReferences(
display_compositor_->RemoveSurfaceReferences(references);
}

void GpuCompositorFrameSink::Require(const cc::LocalFrameId& local_frame_id,
const cc::SurfaceSequence& sequence) {
support_.Require(local_frame_id, sequence);
}

void GpuCompositorFrameSink::Satisfy(const cc::SurfaceSequence& sequence) {
support_.Satisfy(sequence);
}

void GpuCompositorFrameSink::DidReceiveCompositorFrameAck() {
if (client_)
client_->DidReceiveCompositorFrameAck();
Expand Down
3 changes: 3 additions & 0 deletions services/ui/surfaces/gpu_compositor_frame_sink.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ class GpuCompositorFrameSink
const std::vector<cc::SurfaceReference>& references) override;
void RemoveSurfaceReferences(
const std::vector<cc::SurfaceReference>& references) override;
void Require(const cc::LocalFrameId& local_frame_id,
const cc::SurfaceSequence& sequence) override;
void Satisfy(const cc::SurfaceSequence& sequence) override;

// cc::mojom::MojoCompositorFrameSinkPrivate:
void AddChildFrameSink(const cc::FrameSinkId& child_frame_sink_id) override;
Expand Down

0 comments on commit e6b29c4

Please sign in to comment.