Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4409)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: cc/trees/property_tree.cc

Issue 2087963003: cc: Stop creating unused 0 property tree nodes other than transform Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« cc/trees/property_tree.h ('K') | « cc/trees/property_tree.h ('k') | cc/trees/property_tree_builder.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: cc/trees/property_tree.cc
diff --git a/cc/trees/property_tree.cc b/cc/trees/property_tree.cc
index 1bf2af33f0b238e34fd5bca6609f536d13aed9e7..626fc1d6cf314876f0c1f3dd52352998b4d8a6fe 100644
--- a/cc/trees/property_tree.cc
+++ b/cc/trees/property_tree.cc
@@ -66,9 +66,6 @@ template struct TreeNode<ScrollNodeData>;
template <typename T>
PropertyTree<T>::PropertyTree()
: needs_update_(false) {
- nodes_.push_back(T());
- back()->id = 0;
- back()->parent_id = -1;
}
template <typename T>
@@ -80,7 +77,7 @@ TransformTree::TransformTree()
page_scale_factor_(1.f),
device_scale_factor_(1.f),
device_transform_scale_factor_(1.f) {
- cached_data_.push_back(TransformCachedNodeData());
+ Insert(TreeNode<TransformNodeData>(), kInvalidNodeId);
}
TransformTree::~TransformTree() {
@@ -88,7 +85,9 @@ TransformTree::~TransformTree() {
template <typename T>
int PropertyTree<T>::Insert(const T& tree_node, int parent_id) {
- DCHECK_GT(nodes_.size(), 0u);
+ if (nodes_.size() == 0u)
+ DCHECK_EQ(parent_id, kInvalidNodeId) << "size" << nodes_.size()
+ << "parentid" << parent_id;
nodes_.push_back(tree_node);
T& node = nodes_.back();
node.parent_id = parent_id;
@@ -99,9 +98,6 @@ int PropertyTree<T>::Insert(const T& tree_node, int parent_id) {
template <typename T>
void PropertyTree<T>::clear() {
nodes_.clear();
- nodes_.push_back(T());
- back()->id = 0;
- back()->parent_id = -1;
}
template <typename T>
@@ -121,17 +117,12 @@ template <typename T>
void PropertyTree<T>::FromProtobuf(
const proto::PropertyTree& proto,
std::unordered_map<int, int>* node_id_to_index_map) {
- // Verify that the property tree is empty.
- DCHECK_EQ(static_cast<int>(nodes_.size()), 1);
- DCHECK_EQ(back()->id, 0);
- DCHECK_EQ(back()->parent_id, -1);
-
// Add the first node.
ajuma 2016/06/22 13:29:51 I think this comment is obsolete now.
- DCHECK_GT(proto.nodes_size(), 0);
- nodes_.back().FromProtobuf(proto.nodes(0));
+ DCHECK_GE(proto.nodes_size(), 0);
+ nodes_.clear();
- DCHECK(!node_id_to_index_map || (*node_id_to_index_map).empty());
- for (int i = 1; i < proto.nodes_size(); ++i) {
+ node_id_to_index_map->clear();
+ for (int i = 0; i < proto.nodes_size(); ++i) {
nodes_.push_back(T());
nodes_.back().FromProtobuf(proto.nodes(i));
(*node_id_to_index_map)[nodes_.back().owner_id] = nodes_.back().id;
@@ -395,7 +386,8 @@ void TransformNodeData::AsValueInto(
}
TransformCachedNodeData::TransformCachedNodeData()
- : target_id(-1), content_target_id(-1) {}
+ : target_id(TransformTree::kDeviceNodeId),
+ content_target_id(TransformTree::kDeviceNodeId) {}
TransformCachedNodeData::TransformCachedNodeData(
const TransformCachedNodeData& other) = default;
@@ -416,6 +408,7 @@ void TransformCachedNodeData::ToProtobuf(
TransformToProto(to_target, proto->mutable_to_target());
TransformToProto(from_screen, proto->mutable_from_screen());
TransformToProto(to_screen, proto->mutable_to_screen());
+ DCHECK(target_id >= 0) << target_id;
proto->set_target_id(target_id);
proto->set_content_target_id(content_target_id);
}
@@ -727,9 +720,9 @@ void ScrollNodeData::AsValueInto(base::trace_event::TracedValue* value) const {
int TransformTree::Insert(const TransformNode& tree_node, int parent_id) {
int node_id = PropertyTree<TransformNode>::Insert(tree_node, parent_id);
- DCHECK_EQ(node_id, static_cast<int>(cached_data_.size()));
-
cached_data_.push_back(TransformCachedNodeData());
+ DCHECK_EQ(size(), cached_data_.size());
+
return node_id;
}
@@ -739,7 +732,7 @@ void TransformTree::clear() {
nodes_affected_by_inner_viewport_bounds_delta_.clear();
nodes_affected_by_outer_viewport_bounds_delta_.clear();
cached_data_.clear();
- cached_data_.push_back(TransformCachedNodeData());
+ Insert(TransformNode(), TransformTree::kInvalidNodeId);
}
bool TransformTree::ComputeTransform(int source_id,
@@ -803,7 +796,8 @@ bool TransformTree::NeedsSourceToParentUpdate(TransformNode* node) {
}
void TransformTree::ResetChangeTracking() {
- for (int id = 1; id < static_cast<int>(size()); ++id) {
+ for (int id = TransformTree::kDeviceNodeId; id < static_cast<int>(size());
+ ++id) {
TransformNode* node = Node(id);
node->data.transform_changed = false;
}
@@ -1040,6 +1034,8 @@ void TransformTree::UpdateSublayerScale(TransformNode* node) {
void TransformTree::UpdateTargetSpaceTransform(TransformNode* node,
TransformNode* target_node) {
+ DCHECK(node);
+ DCHECK(target_node);
gfx::Transform target_space_transform;
if (node->data.needs_sublayer_scale) {
target_space_transform.MakeIdentity();
@@ -1235,7 +1231,7 @@ void TransformTree::UpdateNodeAndAncestorsAreAnimatedOrInvertible(
void TransformTree::SetDeviceTransform(const gfx::Transform& transform,
gfx::PointF root_position) {
gfx::Transform root_post_local = transform;
- TransformNode* node = Node(1);
+ TransformNode* node = Node(TransformTree::kRootNodeId);
root_post_local.Scale(node->data.post_local_scale_factor,
node->data.post_local_scale_factor);
root_post_local.Translate(root_position.x(), root_position.y());
@@ -1340,7 +1336,11 @@ int TransformTree::TargetId(int node_id) const {
}
void TransformTree::SetTargetId(int node_id, int target_id) {
- DCHECK(static_cast<int>(cached_data_.size()) > node_id);
+ DCHECK(static_cast<int>(cached_data_.size()) > node_id) << node_id;
+ DCHECK(static_cast<int>(size()) > target_id) << target_id << " v.s. "
ajuma 2016/06/22 13:29:51 These can be DCHECK_GE (and then we get a readable
+ << size();
+ DCHECK_GE(target_id, TransformTree::kDeviceNodeId) << target_id;
+ DCHECK_LE(target_id, node_id) << target_id << " v.s. " << node_id;
ajuma 2016/06/22 13:29:50 No need to include the error message here since DC
cached_data_[node_id].target_id = target_id;
}
@@ -1356,8 +1356,8 @@ void TransformTree::SetContentTargetId(int node_id, int content_target_id) {
gfx::Transform TransformTree::ToScreenSpaceTransformWithoutSublayerScale(
int id) const {
- DCHECK_GT(id, 0);
- if (id == 1) {
+ DCHECK_GE(id, TransformTree::kRootNodeId);
+ if (id == TransformTree::kRootNodeId) {
return gfx::Transform();
}
const TransformNode* node = Node(id);
@@ -1411,6 +1411,8 @@ void TransformTree::FromProtobuf(
std::unordered_map<int, int>* node_id_to_index_map) {
DCHECK(proto.has_property_type());
DCHECK_EQ(proto.property_type(), proto::PropertyTree::Transform);
+ // Verify that the property tree is empty.
+ DCHECK_EQ(static_cast<int>(size()), TransformTree::kRootNodeId);
PropertyTree::FromProtobuf(proto, node_id_to_index_map);
const proto::TransformTreeData& data = proto.transform_tree_data();
@@ -1433,10 +1435,10 @@ void TransformTree::FromProtobuf(
nodes_affected_by_outer_viewport_bounds_delta_.push_back(
data.nodes_affected_by_outer_viewport_bounds_delta(i));
}
-
DCHECK_EQ(static_cast<int>(cached_data_.size()), 1);
- cached_data_.back().FromProtobuf(data.cached_data(0));
- for (int i = 1; i < data.cached_data_size(); ++i) {
+ cached_data_.clear();
+ // cached_data_.back().FromProtobuf(data.cached_data(0));
ajuma 2016/06/22 13:29:51 <_<
+ for (int i = 0; i < data.cached_data_size(); ++i) {
cached_data_.push_back(TransformCachedNodeData());
cached_data_.back().FromProtobuf(data.cached_data(i));
}
@@ -1591,7 +1593,7 @@ void EffectTree::TakeCopyRequestsAndTransformToSurface(
// the surface to the space of the surface itself.
int destination_id = effect_node->data.transform_id;
int source_id;
- if (effect_node->parent_id != -1) {
+ if (effect_node->parent_id != EffectTree::kInvalidNodeId) {
// For non-root surfaces, transform only by sub-layer scale.
source_id = destination_id;
} else {
@@ -1639,7 +1641,7 @@ bool EffectTree::ContributesToDrawnSurface(int id) {
}
void EffectTree::ResetChangeTracking() {
- for (int id = 1; id < static_cast<int>(size()); ++id) {
+ for (int id = EffectTree::kRootNodeId; id < static_cast<int>(size()); ++id) {
EffectNode* node = Node(id);
node->data.effect_changed = false;
}
@@ -1659,7 +1661,7 @@ void TransformTree::UpdateNodeAndAncestorsHaveIntegerTranslations(
void ClipTree::SetViewportClip(gfx::RectF viewport_rect) {
if (size() < 2)
ajuma 2016/06/22 17:06:57 I think this should be 1 now that the viewport has
return;
- ClipNode* node = Node(1);
+ ClipNode* node = Node(ClipTree::kViewportNodeId);
if (viewport_rect == node->data.clip)
return;
node->data.clip = viewport_rect;
@@ -1669,7 +1671,7 @@ void ClipTree::SetViewportClip(gfx::RectF viewport_rect) {
gfx::RectF ClipTree::ViewportClip() {
const unsigned long min_size = 1;
DCHECK_GT(size(), min_size);
ajuma 2016/06/22 17:06:57 And this should be a DCHECK_GE now.
- return Node(1)->data.clip;
+ return Node(ClipTree::kViewportNodeId)->data.clip;
}
bool ClipTree::operator==(const ClipTree& other) const {
@@ -1688,6 +1690,8 @@ void ClipTree::FromProtobuf(
std::unordered_map<int, int>* node_id_to_index_map) {
DCHECK(proto.has_property_type());
DCHECK_EQ(proto.property_type(), proto::PropertyTree::Clip);
+ // Verify that the property tree is empty.
+ DCHECK_EQ(static_cast<int>(size()), ClipTree::kRootNodeId);
PropertyTree::FromProtobuf(proto, node_id_to_index_map);
}
@@ -1721,6 +1725,8 @@ void EffectTree::FromProtobuf(
std::unordered_map<int, int>* node_id_to_index_map) {
DCHECK(proto.has_property_type());
DCHECK_EQ(proto.property_type(), proto::PropertyTree::Effect);
+ // Verify that the property tree is empty.
+ DCHECK_EQ(static_cast<int>(size()), EffectTree::kRootNodeId);
PropertyTree::FromProtobuf(proto, node_id_to_index_map);
const proto::EffectTreeData& data = proto.effect_tree_data();
@@ -1796,6 +1802,8 @@ void ScrollTree::FromProtobuf(
std::unordered_map<int, int>* node_id_to_index_map) {
DCHECK(proto.has_property_type());
DCHECK_EQ(proto.property_type(), proto::PropertyTree::Scroll);
+ // Verify that the property tree is empty.
+ DCHECK_EQ(static_cast<int>(size()), ScrollTree::kRootNodeId);
PropertyTree::FromProtobuf(proto, node_id_to_index_map);
const proto::ScrollTreeData& data = proto.scroll_tree_data();
@@ -2149,6 +2157,7 @@ PropertyTrees::PropertyTrees()
effect_tree.SetPropertyTrees(this);
clip_tree.SetPropertyTrees(this);
scroll_tree.SetPropertyTrees(this);
+ transform_id_to_index_map[Layer::INVALID_ID] = TransformTree::kDeviceNodeId;
}
PropertyTrees::~PropertyTrees() {}
@@ -2315,12 +2324,14 @@ bool PropertyTrees::IsInIdToIndexMap(TreeType tree_type, int id) {
}
void PropertyTrees::UpdateChangeTracking() {
- for (int id = 1; id < static_cast<int>(effect_tree.size()); ++id) {
+ for (int id = EffectTree::kRootNodeId;
+ id < static_cast<int>(effect_tree.size()); ++id) {
EffectNode* node = effect_tree.Node(id);
EffectNode* parent_node = effect_tree.parent(node);
effect_tree.UpdateEffectChanged(node, parent_node);
}
- for (int i = 1; i < static_cast<int>(transform_tree.size()); ++i) {
+ for (int i = TransformTree::kRootNodeId;
+ i < static_cast<int>(transform_tree.size()); ++i) {
TransformNode* node = transform_tree.Node(i);
TransformNode* parent_node = transform_tree.parent(node);
TransformNode* source_node = transform_tree.Node(node->data.source_node_id);
@@ -2329,14 +2340,16 @@ void PropertyTrees::UpdateChangeTracking() {
}
void PropertyTrees::PushChangeTrackingTo(PropertyTrees* tree) {
- for (int id = 1; id < static_cast<int>(effect_tree.size()); ++id) {
+ for (int id = EffectTree::kRootNodeId;
+ id < static_cast<int>(effect_tree.size()); ++id) {
EffectNode* node = effect_tree.Node(id);
if (node->data.effect_changed) {
EffectNode* target_node = tree->effect_tree.Node(node->id);
target_node->data.effect_changed = true;
}
}
- for (int id = 1; id < static_cast<int>(transform_tree.size()); ++id) {
+ for (int id = TransformTree::kRootNodeId;
+ id < static_cast<int>(transform_tree.size()); ++id) {
TransformNode* node = transform_tree.Node(id);
if (node->data.transform_changed) {
TransformNode* target_node = tree->transform_tree.Node(node->id);
« cc/trees/property_tree.h ('K') | « cc/trees/property_tree.h ('k') | cc/trees/property_tree_builder.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698