Mozart: Improve internal scene graph representation.

services/gfx/compositor/graph/scene_content.cc

Index: services/gfx/compositor/graph/scene_content.cc
diff --git a/services/gfx/compositor/graph/scene_content.cc b/services/gfx/compositor/graph/scene_content.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f5ce9b55cc5666591616f95341f1ae3fc2d8f1aa
--- /dev/null
+++ b/services/gfx/compositor/graph/scene_content.cc
@@ -0,0 +1,135 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "services/gfx/compositor/graph/scene_content.h"
+
+#include <ostream>
+
+#include "base/logging.h"
+#include "services/gfx/compositor/graph/scene_def.h"
+
+namespace compositor {
+
+SceneContent::SceneContent(const SceneLabel& label,
+                           uint32_t version,
+                           size_t max_resources,
+                           size_t max_nodes)
+    : label_(label),
+      version_(version),
+      resources_(max_resources),
+      nodes_(max_nodes) {}
+
+SceneContent::~SceneContent() {}
+
+const ResourceDef* SceneContent::GetResource(
+    uint32_t resource_id,
+    ResourceDef::Type resource_type) const {
+  auto it = resources_.find(resource_id);
+  DCHECK(it != resources_.end());
+  DCHECK(it->second->type() == resource_type);
+  return it->second.get();
+}
+
+const NodeDef* SceneContent::GetNode(uint32_t node_id) const {
+  auto it = nodes_.find(node_id);
+  DCHECK(it != nodes_.end());
+  return it->second.get();
+}
+
+const NodeDef* SceneContent::GetRootNodeIfExists() const {
+  auto it = nodes_.find(mojo::gfx::composition::kSceneRootNodeId);
+  return it != nodes_.end() ? it->second.get() : nullptr;
+}
+
+SceneContentBuilder::SceneContentBuilder(const SceneDef* scene,
+                                         uint32_t version,
+                                         std::ostream& err,
+                                         size_t max_resources,
+                                         size_t max_nodes)
+    : content_(
+          new SceneContent(scene->label(), version, max_resources, max_nodes)),
+      scene_(scene),
+      err_(err) {
+  DCHECK(scene);
+}
+
+SceneContentBuilder::~SceneContentBuilder() {}
+
+const ResourceDef* SceneContentBuilder::RequireResource(
+    uint32_t resource_id,
+    ResourceDef::Type resource_type,
+    uint32_t referrer_node_id) {
+  DCHECK(content_);
+
+  auto it = content_->resources_.find(resource_id);
+  if (it != content_->resources_.end())
+    return it->second.get();  // found

[abarth, 2016/03/01 17:21:26]

I'd skip this comment.  It's a common pattern for finding things in a map.

[jeffbrown, 2016/03/02 00:32:16]

Done.

+
+  const ResourceDef* resource = scene_->FindResource(resource_id);
+  if (!resource) {
+    err_ << "Missing resource " << resource_id << " referenced from node "
+         << content_->FormattedLabelForNode(referrer_node_id);
+    return nullptr;
+  }
+
+  if (resource->type() != resource_type) {
+    err_ << "Resource " << resource_id << " referenced from node "
+         << content_->FormattedLabelForNode(referrer_node_id)
+         << " has incorrect type for its intended usage";
+    return nullptr;
+  }
+
+  content_->resources_.emplace(std::make_pair(resource_id, resource));
+  return resource;
+}
+
+const NodeDef* SceneContentBuilder::RequireNode(uint32_t node_id,
+                                                uint32_t referrer_node_id) {
+  DCHECK(content_);
+
+  auto it = content_->nodes_.find(node_id);
+  if (it != content_->nodes_.end()) {
+    if (it->second)
+      return it->second.get();  // already added
+    err_ << "Cycle detected at node " << node_id << " referenced from node "
+         << content_->FormattedLabelForNode(referrer_node_id);
+    return nullptr;
+  }
+
+  const NodeDef* node = scene_->FindNode(node_id);
+  if (!node) {
+    err_ << "Missing node " << node_id << " referenced from node "
+         << content_->FormattedLabelForNode(referrer_node_id);
+    return nullptr;
+  }
+
+  return AddNode(node) ? node : nullptr;
+}
+
+bool SceneContentBuilder::AddNode(const NodeDef* node) {
+  DCHECK(content_);
+  DCHECK(node);
+
+  // Reserve a spot in the table to mark the node recording in progress.
+  content_->nodes_.emplace(std::make_pair(node->node_id(), nullptr));
+
+  // Record the node's content.
+  // This performs a depth first search of the node.  If it succeeds, we
+  // will know that this part of the graph has no cycles.
+  if (!node->RecordContent(this))
+    return false;
+
+  // Store the node in the table.
+  content_->nodes_[node->node_id()] = node;

[abarth, 2016/03/01 17:21:26]

There's a pattern for doing this without doing two hash lookups.  It looks like
emplace returns an iterator, which you can hold in a local and then use to set
the slot in the map.

[jeffbrown, 2016/03/02 00:32:16]

You're correct that we can do this but the reasons are subtle.  Note that
|RecordContent| may recurse into |AddNode| to add more nodes.  Ordinarily we
might worry about the iterator being invalidated but we took care to size the
map at creation time to avoid any rehashing, so it's ok.

+  return true;
+}
+
+scoped_refptr<const SceneContent> SceneContentBuilder::Build() {
+  DCHECK(content_);
+
+  const NodeDef* root = scene_->FindRootNode();
+  return !root || AddNode(root) ? std::move(content_) : nullptr;
+}
+
+}  // namespace compositor