Skip to content

Commit

Permalink
Revert "Getting rid of DelegatedFrameData"
Browse files Browse the repository at this point in the history
This reverts commit 4ba3a0e, which
caused a browser-animation framerate regression on Android.

NOTRY=true
TBR=boliu@chromium.org,danakj@chromium.org,fsamuel@chromium.org,dtrainor@chromium.org,piman@chromium.org,reveman@chromium.org,sky@chromium.org,tsepez@chromium.org
BUG=663516,653741
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2503203002
Cr-Commit-Position: refs/heads/master@{#432390}
  • Loading branch information
alexelias authored and Commit bot committed Nov 16, 2016
1 parent 41e9482 commit eb9ecef
Show file tree
Hide file tree
Showing 66 changed files with 1,117 additions and 374 deletions.
4 changes: 2 additions & 2 deletions android_webview/browser/browser_view_renderer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,8 @@ void BrowserViewRenderer::ReturnUnusedResource(
return;

cc::ReturnedResourceArray resources;
cc::TransferableResource::ReturnResources(child_frame->frame->resource_list,
&resources);
cc::TransferableResource::ReturnResources(
child_frame->frame->delegated_frame_data->resource_list, &resources);
content::SynchronousCompositor* compositor =
FindCompositor(child_frame->compositor_id);
if (compositor && !resources.empty())
Expand Down
13 changes: 9 additions & 4 deletions android_webview/browser/hardware_renderer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ void HardwareRenderer::DrawGL(AwDrawGLInfo* draw_info) {
std::move(child_frame_->frame);

gfx::Size frame_size =
child_compositor_frame->render_pass_list.back()->output_rect.size();
child_compositor_frame->delegated_frame_data->render_pass_list.back()
->output_rect.size();
bool size_changed = frame_size != frame_size_;
frame_size_ = frame_size;
if (!child_id_.is_valid() || size_changed) {
Expand Down Expand Up @@ -175,7 +176,10 @@ void HardwareRenderer::DestroySurface() {
DCHECK(child_id_.is_valid());

// Submit an empty frame to force any existing resources to be returned.
surface_factory_->SubmitCompositorFrame(child_id_, cc::CompositorFrame(),
cc::CompositorFrame empty_frame;
empty_frame.delegated_frame_data =
base::WrapUnique(new cc::DelegatedFrameData);
surface_factory_->SubmitCompositorFrame(child_id_, std::move(empty_frame),
cc::SurfaceFactory::DrawCallback());

surfaces_->RemoveChildId(cc::SurfaceId(frame_sink_id_, child_id_));
Expand Down Expand Up @@ -237,8 +241,9 @@ void HardwareRenderer::ReturnChildFrame(
return;

cc::ReturnedResourceArray resources_to_return;
cc::TransferableResource::ReturnResources(child_frame->frame->resource_list,
&resources_to_return);
cc::TransferableResource::ReturnResources(
child_frame->frame->delegated_frame_data->resource_list,
&resources_to_return);

// The child frame's compositor id is not necessarily same as
// compositor_id_.
Expand Down
7 changes: 6 additions & 1 deletion android_webview/browser/surfaces_instance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,11 @@ void SurfacesInstance::DrawAndSwap(const gfx::Size& viewport,
surface_quad->SetNew(quad_state, gfx::Rect(quad_state->quad_layer_bounds),
gfx::Rect(quad_state->quad_layer_bounds), child_id);

std::unique_ptr<cc::DelegatedFrameData> delegated_frame(
new cc::DelegatedFrameData);
delegated_frame->render_pass_list.push_back(std::move(render_pass));
cc::CompositorFrame frame;
frame.render_pass_list.push_back(std::move(render_pass));
frame.delegated_frame_data = std::move(delegated_frame);
frame.metadata.referenced_surfaces = child_ids_;

if (!root_id_.is_valid()) {
Expand Down Expand Up @@ -166,6 +169,8 @@ void SurfacesInstance::RemoveChildId(const cc::SurfaceId& child_id) {

void SurfacesInstance::SetEmptyRootFrame() {
cc::CompositorFrame empty_frame;
empty_frame.delegated_frame_data =
base::WrapUnique(new cc::DelegatedFrameData);
empty_frame.metadata.referenced_surfaces = child_ids_;
surface_factory_->SubmitCompositorFrame(root_id_, std::move(empty_frame),
cc::SurfaceFactory::DrawCallback());
Expand Down
6 changes: 4 additions & 2 deletions android_webview/browser/test/rendering_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,13 @@ content::SynchronousCompositor* RenderingTest::ActiveCompositor() const {
std::unique_ptr<cc::CompositorFrame> RenderingTest::ConstructEmptyFrame() {
std::unique_ptr<cc::CompositorFrame> compositor_frame(
new cc::CompositorFrame);
std::unique_ptr<cc::DelegatedFrameData> frame(new cc::DelegatedFrameData);
std::unique_ptr<cc::RenderPass> root_pass(cc::RenderPass::Create());
gfx::Rect viewport(browser_view_renderer_->size());
root_pass->SetNew(cc::RenderPassId(1, 1), viewport, viewport,
gfx::Transform());
compositor_frame->render_pass_list.push_back(std::move(root_pass));
frame->render_pass_list.push_back(std::move(root_pass));
compositor_frame->delegated_frame_data = std::move(frame);
return compositor_frame;
}

Expand All @@ -130,7 +132,7 @@ std::unique_ptr<cc::CompositorFrame> RenderingTest::ConstructFrame(
std::unique_ptr<cc::CompositorFrame> compositor_frame(ConstructEmptyFrame());
cc::TransferableResource resource;
resource.id = resource_id;
compositor_frame->resource_list.push_back(resource);
compositor_frame->delegated_frame_data->resource_list.push_back(resource);
return compositor_frame;
}

Expand Down
3 changes: 2 additions & 1 deletion blimp/client/core/compositor/blimp_compositor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,8 @@ void BlimpCompositor::SubmitCompositorFrame(cc::CompositorFrame frame) {
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(bound_to_proxy_);

cc::RenderPass* root_pass = frame.render_pass_list.back().get();
cc::RenderPass* root_pass =
frame.delegated_frame_data->render_pass_list.back().get();
gfx::Size surface_size = root_pass->output_rect.size();

if (!local_frame_id_.is_valid() || current_surface_size_ != surface_size) {
Expand Down
2 changes: 2 additions & 0 deletions cc/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,8 @@ component("cc") {
"output/copy_output_request.h",
"output/copy_output_result.cc",
"output/copy_output_result.h",
"output/delegated_frame_data.cc",
"output/delegated_frame_data.h",
"output/direct_renderer.cc",
"output/direct_renderer.h",
"output/dynamic_geometry_binding.cc",
Expand Down
73 changes: 60 additions & 13 deletions cc/ipc/cc_param_traits.cc
Original file line number Diff line number Diff line change
Expand Up @@ -672,9 +672,62 @@ void ParamTraits<cc::SurfaceId>::Log(const param_type& p, std::string* l) {
l->append(")");
}

namespace {
enum CompositorFrameType {
NO_FRAME,
DELEGATED_FRAME,
};
}

void ParamTraits<cc::CompositorFrame>::Write(base::Pickle* m,
const param_type& p) {
WriteParam(m, p.metadata);
if (p.delegated_frame_data) {
WriteParam(m, static_cast<int>(DELEGATED_FRAME));
WriteParam(m, *p.delegated_frame_data);
} else {
WriteParam(m, static_cast<int>(NO_FRAME));
}
}

bool ParamTraits<cc::CompositorFrame>::Read(const base::Pickle* m,
base::PickleIterator* iter,
param_type* p) {
if (!ReadParam(m, iter, &p->metadata))
return false;

int compositor_frame_type;
if (!ReadParam(m, iter, &compositor_frame_type))
return false;

switch (compositor_frame_type) {
case DELEGATED_FRAME:
p->delegated_frame_data.reset(new cc::DelegatedFrameData());
if (!ReadParam(m, iter, p->delegated_frame_data.get()))
return false;
break;
case NO_FRAME:
break;
default:
return false;
}
return true;
}

void ParamTraits<cc::CompositorFrame>::Log(const param_type& p,
std::string* l) {
l->append("CompositorFrame(");
LogParam(p.metadata, l);
l->append(", ");
if (p.delegated_frame_data)
LogParam(*p.delegated_frame_data, l);
l->append(")");
}

void ParamTraits<cc::DelegatedFrameData>::Write(base::Pickle* m,
const param_type& p) {
DCHECK_NE(0u, p.render_pass_list.size());

size_t to_reserve = 0u;
to_reserve += p.resource_list.size() * sizeof(cc::TransferableResource);
for (const auto& pass : p.render_pass_list) {
Expand All @@ -693,12 +746,9 @@ void ParamTraits<cc::CompositorFrame>::Write(base::Pickle* m,
}
}

bool ParamTraits<cc::CompositorFrame>::Read(const base::Pickle* m,
base::PickleIterator* iter,
param_type* p) {
if (!ReadParam(m, iter, &p->metadata))
return false;

bool ParamTraits<cc::DelegatedFrameData>::Read(const base::Pickle* m,
base::PickleIterator* iter,
param_type* p) {
const size_t kMaxRenderPasses = 10000;
const size_t kMaxSharedQuadStateListSize = 100000;
const size_t kMaxQuadListSize = 1000000;
Expand All @@ -708,7 +758,7 @@ bool ParamTraits<cc::CompositorFrame>::Read(const base::Pickle* m,
uint32_t num_render_passes;
if (!ReadParam(m, iter, &p->resource_list) ||
!ReadParam(m, iter, &num_render_passes) ||
num_render_passes > kMaxRenderPasses)
num_render_passes > kMaxRenderPasses || num_render_passes == 0)
return false;
for (uint32_t i = 0; i < num_render_passes; ++i) {
uint32_t quad_list_size;
Expand Down Expand Up @@ -736,15 +786,12 @@ bool ParamTraits<cc::CompositorFrame>::Read(const base::Pickle* m,
pass_set.insert(render_pass->id);
p->render_pass_list.push_back(std::move(render_pass));
}

return true;
}

void ParamTraits<cc::CompositorFrame>::Log(const param_type& p,
std::string* l) {
l->append("CompositorFrame(");
LogParam(p.metadata, l);
l->append(", ");
void ParamTraits<cc::DelegatedFrameData>::Log(const param_type& p,
std::string* l) {
l->append("DelegatedFrameData(");
LogParam(p.resource_list, l);
l->append(", [");
for (size_t i = 0; i < p.render_pass_list.size(); ++i) {
Expand Down
10 changes: 10 additions & 0 deletions cc/ipc/cc_param_traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,16 @@ struct CC_IPC_EXPORT ParamTraits<cc::CompositorFrame> {
static void Log(const param_type& p, std::string* l);
};

template <>
struct CC_IPC_EXPORT ParamTraits<cc::DelegatedFrameData> {
typedef cc::DelegatedFrameData param_type;
static void Write(base::Pickle* m, const param_type& p);
static bool Read(const base::Pickle* m,
base::PickleIterator* iter,
param_type* p);
static void Log(const param_type& p, std::string* l);
};

template <>
struct CC_IPC_EXPORT ParamTraits<cc::DrawQuad::Resources> {
typedef cc::DrawQuad::Resources param_type;
Expand Down
29 changes: 16 additions & 13 deletions cc/ipc/cc_param_traits_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
#include "base/file_descriptor_posix.h"
#endif

using cc::CompositorFrame;
using cc::DelegatedFrameData;
using cc::DebugBorderDrawQuad;
using cc::DrawQuad;
using cc::FilterOperation;
Expand Down Expand Up @@ -431,15 +431,16 @@ TEST_F(CCParamTraitsTest, AllQuads) {
EXPECT_EQ(same_shared_quad_state_cmp, same_shared_quad_state_in);
}

CompositorFrame frame_in;
DelegatedFrameData frame_in;
frame_in.render_pass_list.push_back(std::move(child_pass_in));
frame_in.render_pass_list.push_back(std::move(pass_in));

IPC::ParamTraits<CompositorFrame>::Write(&msg, frame_in);
IPC::ParamTraits<DelegatedFrameData>::Write(&msg, frame_in);

CompositorFrame frame_out;
DelegatedFrameData frame_out;
base::PickleIterator iter(msg);
EXPECT_TRUE(IPC::ParamTraits<CompositorFrame>::Read(&msg, &iter, &frame_out));
EXPECT_TRUE(
IPC::ParamTraits<DelegatedFrameData>::Read(&msg, &iter, &frame_out));

// Make sure the out and cmp RenderPasses match.
std::unique_ptr<RenderPass> child_pass_out =
Expand Down Expand Up @@ -523,15 +524,16 @@ TEST_F(CCParamTraitsTest, UnusedSharedQuadStates) {
ASSERT_EQ(5u, pass_in->shared_quad_state_list.size());
ASSERT_EQ(2u, pass_in->quad_list.size());

CompositorFrame frame_in;
DelegatedFrameData frame_in;
frame_in.render_pass_list.push_back(std::move(pass_in));

IPC::Message msg(1, 2, IPC::Message::PRIORITY_NORMAL);
IPC::ParamTraits<CompositorFrame>::Write(&msg, frame_in);
IPC::ParamTraits<DelegatedFrameData>::Write(&msg, frame_in);

CompositorFrame frame_out;
DelegatedFrameData frame_out;
base::PickleIterator iter(msg);
EXPECT_TRUE(IPC::ParamTraits<CompositorFrame>::Read(&msg, &iter, &frame_out));
EXPECT_TRUE(
IPC::ParamTraits<DelegatedFrameData>::Read(&msg, &iter, &frame_out));

std::unique_ptr<RenderPass> pass_out =
std::move(frame_out.render_pass_list[0]);
Expand Down Expand Up @@ -591,16 +593,17 @@ TEST_F(CCParamTraitsTest, Resources) {
renderpass_in->SetNew(RenderPassId(1, 1), gfx::Rect(), gfx::Rect(),
gfx::Transform());

CompositorFrame frame_in;
DelegatedFrameData frame_in;
frame_in.resource_list.push_back(arbitrary_resource1);
frame_in.resource_list.push_back(arbitrary_resource2);
frame_in.render_pass_list.push_back(std::move(renderpass_in));

IPC::ParamTraits<CompositorFrame>::Write(&msg, frame_in);
IPC::ParamTraits<DelegatedFrameData>::Write(&msg, frame_in);

CompositorFrame frame_out;
DelegatedFrameData frame_out;
base::PickleIterator iter(msg);
EXPECT_TRUE(IPC::ParamTraits<CompositorFrame>::Read(&msg, &iter, &frame_out));
EXPECT_TRUE(
IPC::ParamTraits<DelegatedFrameData>::Read(&msg, &iter, &frame_out));

ASSERT_EQ(2u, frame_out.resource_list.size());
Compare(arbitrary_resource1, frame_out.resource_list[0]);
Expand Down
4 changes: 3 additions & 1 deletion cc/ipc/cc_serialization_perftest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ class CCSerializationPerfTest : public testing::Test {
uint32_t num_passes,
UseSingleSharedQuadState single_sqs) {
CompositorFrame frame;
frame.delegated_frame_data.reset(new DelegatedFrameData);

for (uint32_t i = 0; i < num_passes; ++i) {
std::unique_ptr<RenderPass> render_pass = RenderPass::Create();
Expand All @@ -213,7 +214,8 @@ class CCSerializationPerfTest : public testing::Test {
quad->SetNew(render_pass->shared_quad_state_list.back(), bounds, bounds,
SK_ColorRED, kForceAntiAliasingOff);
}
frame.render_pass_list.push_back(std::move(render_pass));
frame.delegated_frame_data->render_pass_list.push_back(
std::move(render_pass));
}
RunTest(test_name, std::move(frame));
}
Expand Down
5 changes: 3 additions & 2 deletions cc/ipc/compositor_frame_struct_traits.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ bool StructTraits<cc::mojom::CompositorFrameDataView,
if (!data.ReadMetadata(&out->metadata))
return false;

return data.ReadResources(&out->resource_list) &&
data.ReadPasses(&out->render_pass_list);
out->delegated_frame_data.reset(new cc::DelegatedFrameData());
return data.ReadResources(&out->delegated_frame_data->resource_list) &&
data.ReadPasses(&out->delegated_frame_data->render_pass_list);
}

} // namespace mojo
4 changes: 2 additions & 2 deletions cc/ipc/compositor_frame_struct_traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ struct StructTraits<cc::mojom::CompositorFrameDataView, cc::CompositorFrame> {

static const cc::TransferableResourceArray& resources(
const cc::CompositorFrame& input) {
return input.resource_list;
return input.delegated_frame_data->resource_list;
}

static const cc::RenderPassList& passes(const cc::CompositorFrame& input) {
return input.render_pass_list;
return input.delegated_frame_data->render_pass_list;
}

static bool Read(cc::mojom::CompositorFrameDataView data,
Expand Down
17 changes: 11 additions & 6 deletions cc/ipc/struct_traits_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,10 @@ TEST_F(StructTraitsTest, CompositorFrame) {
input.metadata.root_scroll_offset = root_scroll_offset;
input.metadata.page_scale_factor = page_scale_factor;
input.metadata.scrollable_viewport_size = scrollable_viewport_size;
input.render_pass_list.push_back(std::move(render_pass));
input.resource_list.push_back(resource);
input.delegated_frame_data.reset(new DelegatedFrameData);
input.delegated_frame_data->render_pass_list.push_back(
std::move(render_pass));
input.delegated_frame_data->resource_list.push_back(resource);

mojom::TraitsTestServicePtr proxy = GetTraitsTestProxy();
CompositorFrame output;
Expand All @@ -207,15 +209,18 @@ TEST_F(StructTraitsTest, CompositorFrame) {
EXPECT_EQ(page_scale_factor, output.metadata.page_scale_factor);
EXPECT_EQ(scrollable_viewport_size, output.metadata.scrollable_viewport_size);

ASSERT_EQ(1u, output.resource_list.size());
TransferableResource out_resource = output.resource_list[0];
EXPECT_NE(nullptr, output.delegated_frame_data);
ASSERT_EQ(1u, output.delegated_frame_data->resource_list.size());
TransferableResource out_resource =
output.delegated_frame_data->resource_list[0];
EXPECT_EQ(tr_id, out_resource.id);
EXPECT_EQ(tr_format, out_resource.format);
EXPECT_EQ(tr_filter, out_resource.filter);
EXPECT_EQ(tr_size, out_resource.size);

EXPECT_EQ(1u, output.render_pass_list.size());
const RenderPass* out_render_pass = output.render_pass_list[0].get();
EXPECT_EQ(1u, output.delegated_frame_data->render_pass_list.size());
const RenderPass* out_render_pass =
output.delegated_frame_data->render_pass_list[0].get();
ASSERT_EQ(2u, out_render_pass->quad_list.size());
ASSERT_EQ(1u, out_render_pass->shared_quad_state_list.size());

Expand Down
1 change: 1 addition & 0 deletions cc/output/compositor_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ CompositorFrame::CompositorFrame(CompositorFrame&& other) = default;
CompositorFrame::~CompositorFrame() {}

CompositorFrame& CompositorFrame::operator=(CompositorFrame&& other) = default;

} // namespace cc
Loading

0 comments on commit eb9ecef

Please sign in to comment.