[flatland] VkBufferCollectionImporter
Update the renderer interface to inherit from
BufferCollectionImporter and by extension also
update the VkRenderer and NullRenderer to do so
as well.
This allows us to completely remove the renderer
dependency from flatland and streamline the
buffer importation process within the flatland code.
Testing: New unit tests added to flatland_unittests.
All unittests are also modified to account for the
change in dependencies and continue to pass.
Change-Id: Idc23fb710a62bc6d12620f8fcc88f3d495567833
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/436743
Commit-Queue: Chris Fontas <cfontas@google.com>
Testability-Review: Chris Fontas <cfontas@google.com>
Reviewed-by: Adam Gousetis <adamgousetis@google.com>
diff --git a/src/ui/scenic/lib/flatland/engine/engine.cc b/src/ui/scenic/lib/flatland/engine/engine.cc
index 6105974..1460a81 100644
--- a/src/ui/scenic/lib/flatland/engine/engine.cc
+++ b/src/ui/scenic/lib/flatland/engine/engine.cc
@@ -145,7 +145,7 @@
FX_DCHECK(uber_struct_system_);
}
-void Engine::ImportBufferCollection(
+bool Engine::ImportBufferCollection(
sysmem_util::GlobalBufferCollectionId collection_id,
fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) {
@@ -160,7 +160,7 @@
std::unique_lock<std::mutex> lock(lock_);
auto result = scenic_impl::ImportBufferCollection(collection_id, *display_controller_.get(),
std::move(sync_token), image_config);
- FX_DCHECK(result);
+ return result;
}
}
@@ -377,7 +377,9 @@
FX_DCHECK(result);
// Register the buffer collection with the display controller.
- ImportBufferCollection(renderer_collection_id, sysmem_allocator, std::move(display_token));
+ result =
+ ImportBufferCollection(renderer_collection_id, sysmem_allocator, std::move(display_token));
+ FX_DCHECK(result);
// Finally set the engine constraints.
SetClientConstraintsAndWaitForAllocated(sysmem_allocator, std::move(engine_token), num_vmos,
diff --git a/src/ui/scenic/lib/flatland/engine/engine.h b/src/ui/scenic/lib/flatland/engine/engine.h
index 3508275..8d90e60 100644
--- a/src/ui/scenic/lib/flatland/engine/engine.h
+++ b/src/ui/scenic/lib/flatland/engine/engine.h
@@ -21,7 +21,7 @@
const std::shared_ptr<UberStructSystem>& uber_struct_system);
// |BufferCollectionImporter|
- void ImportBufferCollection(
+ bool ImportBufferCollection(
sysmem_util::GlobalBufferCollectionId collection_id,
fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) override;
diff --git a/src/ui/scenic/lib/flatland/engine/tests/engine_unittest.cc b/src/ui/scenic/lib/flatland/engine/tests/engine_unittest.cc
index 34e56c1..3142be9 100644
--- a/src/ui/scenic/lib/flatland/engine/tests/engine_unittest.cc
+++ b/src/ui/scenic/lib/flatland/engine/tests/engine_unittest.cc
@@ -343,10 +343,10 @@
const uint64_t kDisplayImageId = 70;
EXPECT_CALL(*mock_display_controller_.get(),
ImportImage(_, kGlobalBufferCollectionId, kVmoIdx, _))
- .WillRepeatedly(testing::Invoke([](fuchsia::hardware::display::ImageConfig image_config,
- uint64_t collection_id, uint32_t index,
- MockDisplayController::ImportImageCallback callback) {
- callback(ZX_OK, /*display_image_id*/kDisplayImageId);
+ .WillOnce(testing::Invoke([](fuchsia::hardware::display::ImageConfig image_config,
+ uint64_t collection_id, uint32_t index,
+ MockDisplayController::ImportImageCallback callback) {
+ callback(ZX_OK, /*display_image_id*/ kDisplayImageId);
}));
auto result = engine_->ImportImage(metadata);
EXPECT_TRUE(result);
@@ -358,9 +358,9 @@
// Make sure that the engine returns false if the display controller returns an error
EXPECT_CALL(*mock_display_controller_.get(),
ImportImage(_, kGlobalBufferCollectionId, kVmoIdx, _))
- .WillRepeatedly(testing::Invoke([](fuchsia::hardware::display::ImageConfig image_config,
- uint64_t collection_id, uint32_t index,
- MockDisplayController::ImportImageCallback callback) {
+ .WillOnce(testing::Invoke([](fuchsia::hardware::display::ImageConfig image_config,
+ uint64_t collection_id, uint32_t index,
+ MockDisplayController::ImportImageCallback callback) {
callback(ZX_ERR_INVALID_ARGS, /*display_image_id*/ 0);
}));
result = engine_->ImportImage(metadata);
diff --git a/src/ui/scenic/lib/flatland/flatland.cc b/src/ui/scenic/lib/flatland/flatland.cc
index 98b3e62a..f22c2b9 100644
--- a/src/ui/scenic/lib/flatland/flatland.cc
+++ b/src/ui/scenic/lib/flatland/flatland.cc
@@ -40,13 +40,12 @@
Flatland::Flatland(
scheduling::SessionId session_id, const std::shared_ptr<FlatlandPresenter>& flatland_presenter,
- const std::shared_ptr<Renderer>& renderer, const std::shared_ptr<LinkSystem>& link_system,
+ const std::shared_ptr<LinkSystem>& link_system,
const std::shared_ptr<UberStructSystem::UberStructQueue>& uber_struct_queue,
const std::vector<std::shared_ptr<BufferCollectionImporter>>& buffer_collection_importers,
fuchsia::sysmem::AllocatorSyncPtr sysmem_allocator)
: session_id_(session_id),
flatland_presenter_(flatland_presenter),
- renderer_(renderer),
link_system_(link_system),
uber_struct_queue_(uber_struct_queue),
buffer_collection_importers_(buffer_collection_importers),
@@ -84,10 +83,10 @@
// The buffer collection metadata referenced by the image must still be alive. Decrement
// its usage count, which may trigger garbage collection if the collection has been
// released.
- auto buffer_data_kv = buffer_collections_.find(image_kv->second.collection_id);
- FX_DCHECK(buffer_data_kv != buffer_collections_.end());
+ auto buffer_data_kv = buffer_usage_counts_.find(image_kv->second.collection_id);
+ FX_DCHECK(buffer_data_kv != buffer_usage_counts_.end());
- --buffer_data_kv->second.image_count;
+ --buffer_data_kv->second;
// The importers will release the images in this vector at the same time they release
// their buffer collections.
@@ -98,19 +97,19 @@
}
// Collect the list of deregistered buffer collections that are unreferenced by any Images,
- // meaning they can be released from the Renderer.
+ // meaning they can be released from the BufferCollectionImporters.
std::vector<sysmem_util::GlobalBufferCollectionId> buffers_to_release;
for (auto released_iter = released_buffer_collection_ids_.begin();
released_iter != released_buffer_collection_ids_.end();) {
const auto global_collection_id = *released_iter;
- auto buffer_data_kv = buffer_collections_.find(global_collection_id);
- FX_DCHECK(buffer_data_kv != buffer_collections_.end());
+ auto buffer_data_kv = buffer_usage_counts_.find(global_collection_id);
+ FX_DCHECK(buffer_data_kv != buffer_usage_counts_.end());
- if (buffer_data_kv->second.image_count == 0) {
+ if (buffer_data_kv->second == 0) {
buffers_to_release.push_back(global_collection_id);
// Delete local references to the sysmem_util::GlobalBufferCollectionId.
- buffer_collections_.erase(buffer_data_kv);
+ buffer_usage_counts_.erase(buffer_data_kv);
released_iter = released_buffer_collection_ids_.erase(released_iter);
} else {
++released_iter;
@@ -129,19 +128,19 @@
zx_status_t status = zx::event::create(0, &buffer_collection_and_image_release_fence);
FX_DCHECK(status == ZX_OK);
- // Use a self-referencing async::WaitOnce to perform Renderer deregistration. This is
- // primarily so the handler does not have to live in the Flatland instance, which may be
- // destroyed before the release fence is signaled. WaitOnce moves the handler to the stack
+ // Use a self-referencing async::WaitOnce to perform BufferCollectionImporter deregistration.
+ // This is primarily so the handler does not have to live in the Flatland instance, which may
+ // be destroyed before the release fence is signaled. WaitOnce moves the handler to the stack
// prior to invoking it, so it is safe for the handler to delete the WaitOnce on exit.
// Specifically, we move the wait object into the lambda function via |copy_ref = wait| to
// ensure that the wait object lives. The callback will not trigger without this.
auto wait = std::make_shared<async::WaitOnce>(buffer_collection_and_image_release_fence.get(),
ZX_EVENT_SIGNALED);
status = wait->Begin(
- dispatcher, [copy_ref = wait, renderer_ref = renderer_,
- importer_ref = buffer_collection_importers_, buffers_to_release,
- images_to_release](async_dispatcher_t*, async::WaitOnce*, zx_status_t status,
- const zx_packet_signal_t* /*signal*/) mutable {
+ dispatcher,
+ [copy_ref = wait, importer_ref = buffer_collection_importers_, buffers_to_release,
+ images_to_release](async_dispatcher_t*, async::WaitOnce*, zx_status_t status,
+ const zx_packet_signal_t* /*signal*/) mutable {
FX_DCHECK(status == ZX_OK);
// Release images first, since they need to be released before we release their
@@ -154,7 +153,6 @@
// Now we can release the buffer collections.
for (const auto& global_collection_id : buffers_to_release) {
- renderer_ref->DeregisterCollection(global_collection_id);
for (auto& importer : importer_ref) {
importer->ReleaseBufferCollection(global_collection_id);
}
@@ -307,7 +305,7 @@
matrices_.clear();
// List all global buffer collection IDs as "released", which will trigger cleanup in Present().
- for (const auto& [collection_id, buffer_collection] : buffer_collections_) {
+ for (const auto& [collection_id, buffer_collection] : buffer_usage_counts_) {
released_buffer_collection_ids_.insert(collection_id);
}
@@ -555,8 +553,6 @@
void Flatland::RegisterBufferCollection(
BufferCollectionId collection_id,
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) {
- FX_DCHECK(renderer_);
-
if (collection_id == 0) {
FX_LOGS(ERROR) << "RegisterBufferCollection called with collection_id 0";
ReportError();
@@ -578,52 +574,55 @@
// Grab a new unique global buffer collection id.
auto global_collection_id = sysmem_util::GenerateUniqueBufferCollectionId();
+ FX_DCHECK(!buffer_usage_counts_.count(global_collection_id));
- // For every external service we have, duplicate the token and store it in the
- // |extra_tokens| vector. We will then pass the tokens along to the importers
- // after we've received the |global_collection_id| from sysmem_util, so that
- // all handles use the same ID.
- std::vector<fuchsia::sysmem::BufferCollectionTokenSyncPtr> extra_tokens;
- for (auto& buffer_importer : buffer_collection_importers_) {
+ // Create a token for each of the buffer collection importers and stick all
+ // of the tokens into an std::vector.
+ fuchsia::sysmem::BufferCollectionTokenSyncPtr sync_token = token.BindSync();
+ std::vector<fuchsia::sysmem::BufferCollectionTokenSyncPtr> tokens;
+ for (uint32_t i = 0; i < buffer_collection_importers_.size() - 1; i++) {
fuchsia::sysmem::BufferCollectionTokenSyncPtr extra_token;
- // TODO(fxbug.dev/51213): See if this can become asynchronous.
- fuchsia::sysmem::BufferCollectionTokenSyncPtr sync_token = token.BindSync();
zx_status_t status =
sync_token->Duplicate(std::numeric_limits<uint32_t>::max(), extra_token.NewRequest());
FX_DCHECK(status == ZX_OK);
- token = sync_token.Unbind();
- extra_tokens.push_back(std::move(extra_token));
+ tokens.push_back(std::move(extra_token));
+ }
+ tokens.push_back(std::move(sync_token));
+
+ // Loop over each of the importers and provide each of them with a token from the vector
+ // we created above. We declare the iterator |i| outside the loop to aid in cleanup if
+ // importing fails.
+ uint32_t i = 0;
+ for (i = 0; i < buffer_collection_importers_.size(); i++) {
+ auto importer = buffer_collection_importers_[i];
+ auto result = importer->ImportBufferCollection(global_collection_id, sysmem_allocator_.get(),
+ std::move(tokens[i]));
+ // Exit the loop early if an importer fails to import the buffer collection.
+ if (!result) {
+ break;
+ }
}
- // Register the texture collection immediately since the client may block on buffers being
- // allocated before calling Present().
- auto result = renderer_->RegisterTextureCollection(global_collection_id, sysmem_allocator_.get(),
- std::move(token));
+ // If the iterator |i| isn't equal to the number of importers than we know that one of the
+ // importers has failed.
+ if (i < buffer_collection_importers_.size()) {
+ // We have to clean up the buffer collection from the importers where importation was
+ // successful.
+ for (uint32_t j = 0; j < i; j++) {
+ buffer_collection_importers_[j]->ReleaseBufferCollection(global_collection_id);
+ }
- if (!result) {
- FX_LOGS(ERROR)
- << "RegisterBufferCollection failed to register the sysmem token with the renderer.";
+ FX_LOGS(ERROR) << "Failed to import the buffer collection to the BufferCollectionImporter.";
ReportError();
return;
}
- // Now we can set the constraints on each of the importers.
- for (uint32_t i = 0; i < buffer_collection_importers_.size(); i++) {
- auto& buffer_importer = buffer_collection_importers_[i];
- auto extra_token = std::move(extra_tokens[i]);
- // Pass along extra token to the external service that needs the buffer collection.
- buffer_importer->ImportBufferCollection(global_collection_id, sysmem_allocator_.get(),
- std::move(extra_token));
- }
-
buffer_collection_ids_[collection_id] = global_collection_id;
- buffer_collections_[global_collection_id] = BufferCollectionData();
+ buffer_usage_counts_[global_collection_id] = 0u;
}
void Flatland::CreateImage(ContentId image_id, BufferCollectionId collection_id, uint32_t vmo_index,
ImageProperties properties) {
- FX_DCHECK(renderer_);
-
if (image_id == 0) {
FX_LOGS(ERROR) << "CreateImage called with image_id 0";
ReportError();
@@ -646,33 +645,10 @@
const auto global_collection_id = buffer_id_kv->second;
- auto buffer_collection_kv = buffer_collections_.find(global_collection_id);
- FX_DCHECK(buffer_collection_kv != buffer_collections_.end());
+ auto buffer_collection_kv = buffer_usage_counts_.find(global_collection_id);
+ FX_DCHECK(buffer_collection_kv != buffer_usage_counts_.end());
- auto& buffer_data = buffer_collection_kv->second;
-
- // If the buffer hasn't been validated yet, try to validate it. If validation fails, it will be
- // impossible to render this image.
- if (!buffer_data.metadata.has_value()) {
- auto metadata = renderer_->Validate(global_collection_id);
- if (!metadata.has_value()) {
- FX_LOGS(ERROR) << "CreateImage failed, collection_id " << collection_id
- << " has not been allocated yet";
- ReportError();
- return;
- }
-
- buffer_data.metadata = std::move(metadata.value());
- }
-
- if (vmo_index >= buffer_data.metadata->vmo_count) {
- FX_LOGS(ERROR) << "CreateImage failed, vmo_index " << vmo_index
- << " must be less than vmo_count " << buffer_data.metadata->vmo_count;
- ReportError();
- return;
- }
-
- const auto& image_constraints = buffer_data.metadata->image_constraints;
+ auto& buffer_usage_count = buffer_collection_kv->second;
if (!properties.has_width()) {
FX_LOGS(ERROR) << "CreateImage failed, ImageProperties did not specify a width";
@@ -680,39 +656,23 @@
return;
}
- const uint32_t width = properties.width();
- if (width < image_constraints.min_coded_width || width > image_constraints.max_coded_width) {
- FX_LOGS(ERROR) << "CreateImage failed, width " << width << " is not within valid range ["
- << image_constraints.min_coded_width << "," << image_constraints.max_coded_width
- << "]";
- ReportError();
- return;
- }
-
if (!properties.has_height()) {
FX_LOGS(ERROR) << "CreateImage failed, ImageProperties did not specify a height";
ReportError();
return;
}
- const uint32_t height = properties.height();
- if (height < image_constraints.min_coded_height || height > image_constraints.max_coded_height) {
- FX_LOGS(ERROR) << "CreateImage failed, height " << height << " is not within valid range ["
- << image_constraints.min_coded_height << ","
- << image_constraints.max_coded_height << "]";
- ReportError();
- return;
- }
-
ImageMetadata metadata;
metadata.identifier = GenerateUniqueImageId();
metadata.collection_id = global_collection_id;
metadata.vmo_idx = vmo_index;
- metadata.width = width;
- metadata.height = height;
+ metadata.width = properties.width();
+ metadata.height = properties.height();
for (uint32_t i = 0; i < buffer_collection_importers_.size(); i++) {
auto& importer = buffer_collection_importers_[i];
+
+ // TODO(62240): Give more detailed errors.
auto result = importer->ImportImage(metadata);
if (!result) {
// If this importer fails, we need to release the image from
@@ -737,7 +697,7 @@
image_metadatas_[handle] = metadata;
// Increment the buffer's usage count.
- ++buffer_data.image_count;
+ ++buffer_usage_count;
}
void Flatland::SetContentOnTransform(ContentId content_id, TransformId transform_id) {
@@ -949,7 +909,7 @@
}
auto global_collection_id = buffer_id_kv->second;
- FX_DCHECK(buffer_collections_.count(global_collection_id));
+ FX_DCHECK(buffer_usage_counts_.count(global_collection_id));
// Erase the user-facing mapping of the ID and queue the global ID for garbage collection. The
// actual buffer collection data will be cleared once all Images reference the collection are
diff --git a/src/ui/scenic/lib/flatland/flatland.h b/src/ui/scenic/lib/flatland/flatland.h
index d1aae6e..7270fb8 100644
--- a/src/ui/scenic/lib/flatland/flatland.h
+++ b/src/ui/scenic/lib/flatland/flatland.h
@@ -25,7 +25,6 @@
#include "src/ui/scenic/lib/flatland/link_system.h"
#include "src/ui/scenic/lib/flatland/renderer/buffer_collection_importer.h"
-#include "src/ui/scenic/lib/flatland/renderer/renderer.h"
#include "src/ui/scenic/lib/flatland/flatland_presenter.h"
#include "src/ui/scenic/lib/flatland/transform_graph.h"
#include "src/ui/scenic/lib/flatland/transform_handle.h"
@@ -50,7 +49,7 @@
explicit Flatland(
scheduling::SessionId session_id,
const std::shared_ptr<FlatlandPresenter>& flatland_presenter,
- const std::shared_ptr<Renderer>& renderer, const std::shared_ptr<LinkSystem>& link_system,
+ const std::shared_ptr<LinkSystem>& link_system,
const std::shared_ptr<UberStructSystem::UberStructQueue>& uber_struct_queue,
const std::vector<std::shared_ptr<BufferCollectionImporter>>& buffer_collection_importers,
fuchsia::sysmem::AllocatorSyncPtr sysmem_allocator);
@@ -149,10 +148,6 @@
// PresentIds when publishing to the UberStructSystem.
std::shared_ptr<FlatlandPresenter> flatland_presenter_;
- // A Renderer shared between Flatland instances. Flatland registers buffer collections with the
- // Renderer and references them by ID when submitting data in an UberStruct.
- std::shared_ptr<Renderer> renderer_;
-
// A link system shared between Flatland instances, so that links can be made between them.
std::shared_ptr<LinkSystem> link_system_;
@@ -257,17 +252,10 @@
std::unordered_map<BufferCollectionId, sysmem_util::GlobalBufferCollectionId>
buffer_collection_ids_;
- // The metadata associated with a particular buffer collection and the number of Images that
- // currently reference that buffer collection.
- struct BufferCollectionData {
- std::optional<BufferCollectionMetadata> metadata;
- size_t image_count = 0;
- };
-
- // A mapping from global buffer collection ID to the data associated with each
- // collection.
- std::unordered_map<sysmem_util::GlobalBufferCollectionId, BufferCollectionData>
- buffer_collections_;
+ // A mapping from global buffer collection ID to a size_t indicating how many images are currently
+ // using the given buffer. This count is used to determine in part when the buffer collection
+ // should be garbage collected.
+ std::unordered_map<sysmem_util::GlobalBufferCollectionId, size_t> buffer_usage_counts_;
// The set of sysmem_util::GlobalBufferCollectionIds associated with released BufferCollectionIds
// that have not yet been garbage collected.
diff --git a/src/ui/scenic/lib/flatland/flatland_manager.cc b/src/ui/scenic/lib/flatland/flatland_manager.cc
index c90d0d0..81b6b4f 100644
--- a/src/ui/scenic/lib/flatland/flatland_manager.cc
+++ b/src/ui/scenic/lib/flatland/flatland_manager.cc
@@ -13,12 +13,10 @@
FlatlandManager::FlatlandManager(
const std::shared_ptr<FlatlandPresenter>& flatland_presenter,
- const std::shared_ptr<Renderer>& renderer,
const std::shared_ptr<UberStructSystem>& uber_struct_system,
const std::shared_ptr<LinkSystem>& link_system,
const std::vector<std::shared_ptr<BufferCollectionImporter>>& buffer_collection_importers)
: flatland_presenter_(flatland_presenter),
- renderer_(renderer),
uber_struct_system_(uber_struct_system),
link_system_(link_system),
buffer_collection_importers_(buffer_collection_importers) {}
@@ -33,10 +31,9 @@
sysmem_allocator.NewRequest().TakeChannel().release());
FX_DCHECK(status == ZX_OK);
- auto flatland =
- std::make_unique<Flatland>(id, flatland_presenter_, renderer_, link_system_,
- uber_struct_system_->AllocateQueueForSession(id),
- buffer_collection_importers_, std::move(sysmem_allocator));
+ auto flatland = std::make_unique<Flatland>(
+ id, flatland_presenter_, link_system_, uber_struct_system_->AllocateQueueForSession(id),
+ buffer_collection_importers_, std::move(sysmem_allocator));
auto result = flatland_instances_.emplace(
id, std::make_unique<FlatlandInstance>(request.channel(), std::move(flatland)));
diff --git a/src/ui/scenic/lib/flatland/flatland_manager.h b/src/ui/scenic/lib/flatland/flatland_manager.h
index 58ce335..06b6e5a 100644
--- a/src/ui/scenic/lib/flatland/flatland_manager.h
+++ b/src/ui/scenic/lib/flatland/flatland_manager.h
@@ -18,7 +18,6 @@
#include "src/ui/scenic/lib/flatland/flatland.h"
#include "src/ui/scenic/lib/flatland/flatland_presenter.h"
#include "src/ui/scenic/lib/flatland/link_system.h"
-#include "src/ui/scenic/lib/flatland/renderer/renderer.h"
#include "src/ui/scenic/lib/flatland/uber_struct_system.h"
#include "src/ui/scenic/lib/scheduling/id.h"
@@ -28,7 +27,6 @@
public:
FlatlandManager(
const std::shared_ptr<FlatlandPresenter>& flatland_presenter,
- const std::shared_ptr<Renderer>& renderer,
const std::shared_ptr<UberStructSystem>& uber_struct_system,
const std::shared_ptr<LinkSystem>& link_system,
const std::vector<std::shared_ptr<BufferCollectionImporter>>& buffer_collection_importers);
@@ -43,7 +41,6 @@
void RemoveFlatlandInstance(scheduling::SessionId session_id);
std::shared_ptr<FlatlandPresenter> flatland_presenter_;
- std::shared_ptr<Renderer> renderer_;
std::shared_ptr<UberStructSystem> uber_struct_system_;
std::shared_ptr<LinkSystem> link_system_;
std::vector<std::shared_ptr<BufferCollectionImporter>> buffer_collection_importers_;
diff --git a/src/ui/scenic/lib/flatland/global_image_data.h b/src/ui/scenic/lib/flatland/global_image_data.h
index eb57927..3d1ca19 100644
--- a/src/ui/scenic/lib/flatland/global_image_data.h
+++ b/src/ui/scenic/lib/flatland/global_image_data.h
@@ -6,7 +6,7 @@
#define SRC_UI_SCENIC_LIB_FLATLAND_GLOBAL_IMAGE_DATA_H_
#include "src/ui/scenic/lib/flatland/global_topology_data.h"
-#include "src/ui/scenic/lib/flatland/renderer/renderer.h"
+#include "src/ui/scenic/lib/flatland/renderer/buffer_collection_importer.h"
#include "src/ui/scenic/lib/flatland/uber_struct.h"
namespace flatland {
diff --git a/src/ui/scenic/lib/flatland/renderer/buffer_collection_importer.h b/src/ui/scenic/lib/flatland/renderer/buffer_collection_importer.h
index 2e8d15b..dba2b57 100644
--- a/src/ui/scenic/lib/flatland/renderer/buffer_collection_importer.h
+++ b/src/ui/scenic/lib/flatland/renderer/buffer_collection_importer.h
@@ -5,10 +5,41 @@
#ifndef SRC_UI_SCENIC_LIB_FLATLAND_RENDERER_BUFFER_COLLECTION_IMPORTER_H_
#define SRC_UI_SCENIC_LIB_FLATLAND_RENDERER_BUFFER_COLLECTION_IMPORTER_H_
-#include "src/ui/scenic/lib/flatland/renderer/renderer.h"
+#include <fuchsia/sysmem/cpp/fidl.h>
+
+#include "src/ui/scenic/lib/sysmem/id.h"
namespace flatland {
+// Used to reference a particular flatland image.
+using GlobalImageId = uint64_t;
+
+// Struct representing the data needed to extract an image from a buffer collection.
+// All pixel information is stored within the Vmo of the collection so this struct
+// only needs information regarding which collection and which vmo to point to, and
+// the overall size of the image. Only supports fuchsia::sysmem::PixelFormatType::BGRA32
+// as the image format type.
+struct ImageMetadata {
+ // The unique id of the buffer collection this image is backed by.
+ sysmem_util::GlobalBufferCollectionId collection_id;
+
+ // The unique ID for this particular image.
+ GlobalImageId identifier;
+
+ // A single buffer collection may have several vmos. This tells the importer
+ // which vmo in the collection specified by |collection_id| to use as the memory
+ // for this image. This value must be less than BufferCollectionMetadata::vmo_count.
+ uint32_t vmo_idx;
+
+ // The dimensions of the image in pixels.
+ uint32_t width, height;
+
+ bool operator==(const ImageMetadata& meta) const {
+ return collection_id == meta.collection_id && vmo_idx == meta.vmo_idx && width == meta.width &&
+ height == meta.height;
+ }
+};
+
// This interface is used for importing Flatland buffer collections
// and images to external services that would like to also have access
// to the collection and set their own constraints. This interface allows
@@ -17,8 +48,10 @@
class BufferCollectionImporter {
public:
// Allows the service to set its own constraints on the buffer collection. Must be set before
- // the buffer collection is fully allocated/validated.
- virtual void ImportBufferCollection(
+ // the buffer collection is fully allocated/validated. The return value indicates successful
+ // importation via |true| and a failed importation via |false|. The collection_id can be
+ // reused if the importation fails.
+ virtual bool ImportBufferCollection(
sysmem_util::GlobalBufferCollectionId collection_id,
fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) = 0;
@@ -30,6 +63,8 @@
// Has the service create an image for itself from the provided buffer collection. Returns
// true upon a successful import and false otherwise.
+ //
+ // TODO(62240): Give more detailed errors.
virtual bool ImportImage(const ImageMetadata& meta_data) = 0;
// Deregisters the provided image from the service.
diff --git a/src/ui/scenic/lib/flatland/renderer/null_renderer.cc b/src/ui/scenic/lib/flatland/renderer/null_renderer.cc
index 0948a28..34e8338 100644
--- a/src/ui/scenic/lib/flatland/renderer/null_renderer.cc
+++ b/src/ui/scenic/lib/flatland/renderer/null_renderer.cc
@@ -9,21 +9,19 @@
namespace flatland {
-bool NullRenderer::RegisterTextureCollection(
- sysmem_util::GlobalBufferCollectionId collection_id,
- fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
- fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) {
- return RegisterCollection(collection_id, sysmem_allocator, std::move(token));
-}
-
bool NullRenderer::RegisterRenderTargetCollection(
sysmem_util::GlobalBufferCollectionId collection_id,
fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) {
- return RegisterCollection(collection_id, sysmem_allocator, std::move(token));
+ return ImportBufferCollection(collection_id, sysmem_allocator, std::move(token));
}
-bool NullRenderer::RegisterCollection(
+void NullRenderer::DeregisterRenderTargetCollection(
+ sysmem_util::GlobalBufferCollectionId collection_id) {
+ ReleaseBufferCollection(collection_id);
+}
+
+bool NullRenderer::ImportBufferCollection(
sysmem_util::GlobalBufferCollectionId collection_id,
fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) {
@@ -56,7 +54,7 @@
return true;
}
-void NullRenderer::DeregisterCollection(sysmem_util::GlobalBufferCollectionId collection_id) {
+void NullRenderer::ReleaseBufferCollection(sysmem_util::GlobalBufferCollectionId collection_id) {
// Multiple threads may be attempting to read/write from the various maps,
// lock this function here.
// TODO(fxbug.dev/44335): Convert this to a lock-free structure.
@@ -77,6 +75,43 @@
collection_metadata_map_.erase(collection_id);
}
+bool NullRenderer::ImportImage(const ImageMetadata& meta_data) {
+ auto buffer_metadata = Validate(meta_data.collection_id);
+ if (!buffer_metadata.has_value()) {
+ FX_LOGS(ERROR) << "CreateImage failed, collection_id " << meta_data.collection_id
+ << " has not been allocated yet";
+ return false;
+ }
+
+ if (meta_data.vmo_idx >= buffer_metadata->vmo_count) {
+ FX_LOGS(ERROR) << "CreateImage failed, vmo_index " << meta_data.vmo_idx
+ << " must be less than vmo_count " << buffer_metadata->vmo_count;
+ return false;
+ }
+
+ const auto& image_constraints = buffer_metadata->image_constraints;
+
+ if (meta_data.width < image_constraints.min_coded_width ||
+ meta_data.width > image_constraints.max_coded_width) {
+ FX_LOGS(ERROR) << "CreateImage failed, width " << meta_data.width
+ << " is not within valid range [" << image_constraints.min_coded_width << ","
+ << image_constraints.max_coded_width << "]";
+ return false;
+ }
+
+ if (meta_data.height < image_constraints.min_coded_height ||
+ meta_data.height > image_constraints.max_coded_height) {
+ FX_LOGS(ERROR) << "CreateImage failed, height " << meta_data.height
+ << " is not within valid range [" << image_constraints.min_coded_height << ","
+ << image_constraints.max_coded_height << "]";
+ return false;
+ }
+
+ return true;
+}
+
+void NullRenderer::ReleaseImage(GlobalImageId image_id) {}
+
std::optional<BufferCollectionMetadata> NullRenderer::Validate(
sysmem_util::GlobalBufferCollectionId collection_id) {
// TODO(fxbug.dev/44335): Convert this to a lock-free structure. This is trickier than in the
diff --git a/src/ui/scenic/lib/flatland/renderer/null_renderer.h b/src/ui/scenic/lib/flatland/renderer/null_renderer.h
index d0d61d0a..727a8c0 100644
--- a/src/ui/scenic/lib/flatland/renderer/null_renderer.h
+++ b/src/ui/scenic/lib/flatland/renderer/null_renderer.h
@@ -15,16 +15,25 @@
// A renderer implementation used for validation. It does everything a standard
// renderer implementation does except for actually rendering.
-class NullRenderer : public Renderer {
+class NullRenderer final : public Renderer {
public:
~NullRenderer() override = default;
- // |Renderer|.
- bool RegisterTextureCollection(
+ // |BufferCollectionImporter|
+ bool ImportBufferCollection(
sysmem_util::GlobalBufferCollectionId collection_id,
fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) override;
+ // |BufferCollectionImporter|
+ void ReleaseBufferCollection(sysmem_util::GlobalBufferCollectionId collection_id) override;
+
+ // |BufferCollectionImporter|
+ bool ImportImage(const ImageMetadata& meta_data) override;
+
+ // |BufferCollectionImporter|
+ void ReleaseImage(GlobalImageId image_id) override;
+
// |Renderer|.
bool RegisterRenderTargetCollection(
sysmem_util::GlobalBufferCollectionId collection_id,
@@ -32,7 +41,8 @@
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) override;
// |Renderer|.
- void DeregisterCollection(sysmem_util::GlobalBufferCollectionId collection_id) override;
+ void DeregisterRenderTargetCollection(
+ sysmem_util::GlobalBufferCollectionId collection_id) override;
// |Renderer|.
std::optional<BufferCollectionMetadata> Validate(
@@ -44,10 +54,6 @@
const std::vector<zx::event>& release_fences = {}) override;
private:
- bool RegisterCollection(sysmem_util::GlobalBufferCollectionId collection_id,
- fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
- fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token);
-
// This mutex is used to protect access to |collection_map_| and |collection_metadata_map_|.
std::mutex lock_;
std::unordered_map<sysmem_util::GlobalBufferCollectionId, BufferCollectionInfo> collection_map_;
diff --git a/src/ui/scenic/lib/flatland/renderer/renderer.h b/src/ui/scenic/lib/flatland/renderer/renderer.h
index 2f3b026..bb90d0a 100644
--- a/src/ui/scenic/lib/flatland/renderer/renderer.h
+++ b/src/ui/scenic/lib/flatland/renderer/renderer.h
@@ -5,11 +5,12 @@
#ifndef SRC_UI_SCENIC_LIB_FLATLAND_RENDERER_RENDERER_H_
#define SRC_UI_SCENIC_LIB_FLATLAND_RENDERER_RENDERER_H_
-#include <fuchsia/sysmem/cpp/fidl.h>
#include <lib/zx/event.h>
#include <optional>
+#include "src/ui/scenic/lib/flatland/renderer/buffer_collection_importer.h"
+
// clang-format off
#include "src/ui/lib/glm_workaround/glm_workaround.h"
// clang-format on
@@ -19,16 +20,11 @@
#include <glm/vec4.hpp>
#include "src/ui/lib/escher/geometry/types.h"
-#include "src/ui/scenic/lib/sysmem/id.h"
-
namespace flatland {
// Typedef to a flatland type.
using Rectangle2D = escher::Rectangle2D;
-// Used to reference a particular flatland image.
-using GlobalImageId = uint64_t;
-
// Contains information regarding the constraints of a particular buffer collection. This
// should be used by the Flatland instance to check that images it wishes to create are
// valid. More information may be added to this struct over time.
@@ -41,64 +37,12 @@
fuchsia::sysmem::ImageFormatConstraints image_constraints;
};
-// Struct representing the data needed to extract an image from a buffer collection.
-// All pixel information is stored within the Vmo of the collection so this struct
-// only needs information regarding which collection and which vmo to point to, and
-// the overall size of the image. Only supports fuchsia::sysmem::PixelFormatType::BGRA32
-// as the image format type.
-struct ImageMetadata {
- // The unique id of the buffer collection to refer to. This id should match an
- // an ID generated by a call to |Renderer::RegisterBufferCollection| below.
- sysmem_util::GlobalBufferCollectionId collection_id;
-
- // The unique ID for this particular image.
- GlobalImageId identifier;
-
- // A single buffer collection may have several vmos. This tells the renderer
- // which vmo in the collection specified by |collection_id| to use as the memory
- // for this image. This value must be less than BufferCollectionMetadata::vmo_count.
- uint32_t vmo_idx;
-
- // The dimensions of the image in pixels.
- uint32_t width, height;
-
- bool operator==(const ImageMetadata& meta) const {
- return collection_id == meta.collection_id && vmo_idx == meta.vmo_idx && width == meta.width &&
- height == meta.height;
- }
-};
-
// This is the main renderer interface used by the Flatland System. Since Flatland is
// agnostic to the implementation of the renderer, it is declared here as a virtual
// interface, whose concrete implementation is to be injected into Flatland.
-class Renderer {
+class Renderer : public BufferCollectionImporter {
public:
- // This function registers a buffer collection with the renderer and is intended to be
- // called by a Flatland instance on that instance's thread, not the render thread. It is
- // thread safe, and so multiple Flatland instances can call it simultaneously. The
- // buffer collection associated with an image must be registered with the renderer before
- // that image can be used during rendering. Concrete implementations of this class may
- // potentially set their own additional constraints on the buffer collection, including
- // negotiating with the display controller.
- //
- // This function returns true upon a successful registration and false otherwise.
- // Registration may fail if the passed in token is invalid or if it is a valid
- // channel but not a valid buffer collection.
- //
- // It is the client's responsibility to provide a GlobalBufferCollectionID and to hold onto
- // that ID to reference the buffer collection in subsequent calls. IDs must be positive,
- // so an ID value of 0 will result in a failed registration.
- //
- // Passing in an ID that is already in use by another collection will also result in a return
- // value of false. An ID can be reused if the buffer collection associated with the ID has
- // been removed from the renderer via a call to |DeregisterBufferCollection|.
- virtual bool RegisterTextureCollection(
- sysmem_util::GlobalBufferCollectionId collection_id,
- fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
- fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) = 0;
-
- // This function operates exactly like the function |RegisterTextureCollection|, but is
- // specifically for registering collections that contain render targets. In order for an image
+ // This function is for registering collections that contain render targets. In order for an image
// to be used as a render target in the Render() function below, the buffer collection it
// is associated with must have been registered with this function.
//
@@ -109,18 +53,10 @@
fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) = 0;
- // Removes the buffer collection provided from the renderer. All images created using
- // this buffer collection become invalid afterwards and no new images can be created using
- // this collection id. Buffer collections can only be safely deregisted after all pending
- // render work involving the collection has been completed. Deregistering before this point
- // results in undefined behavior and may crash the process. To avoid this, it is possible
- // to synchronize with rendering by passing a release fence in the form of a zx::event to
- // Render() and waiting for it to signal before deregistering. See Render() for more details.
- // This function is threadsafe.
- //
- // Once a buffer collection has been deregistered, the collection_id associated with it will
- // be free to be reused again for a new collection.
- virtual void DeregisterCollection(sysmem_util::GlobalBufferCollectionId collection_id) = 0;
+ // Removes a buffer collection used for render targets from the renderer. Once done, the
+ // collection_id can be reused for another buffer collection.
+ virtual void DeregisterRenderTargetCollection(
+ sysmem_util::GlobalBufferCollectionId collection_id) = 0;
// This function validates if the buffer collection referenced by |collection_id| is ready to
// be used in rendering operations and must be called before that buffer collection is used
diff --git a/src/ui/scenic/lib/flatland/renderer/tests/display_unittest.cc b/src/ui/scenic/lib/flatland/renderer/tests/display_unittest.cc
index f0d70cf..d540076 100644
--- a/src/ui/scenic/lib/flatland/renderer/tests/display_unittest.cc
+++ b/src/ui/scenic/lib/flatland/renderer/tests/display_unittest.cc
@@ -112,8 +112,8 @@
// Register the collection with the renderer, which sets the vk constraints.
auto renderer_collection_id = sysmem_util::GenerateUniqueBufferCollectionId();
- auto result = renderer.RegisterTextureCollection(renderer_collection_id, sysmem_allocator_.get(),
- std::move(tokens.dup_token));
+ auto result = renderer.ImportBufferCollection(renderer_collection_id, sysmem_allocator_.get(),
+ std::move(tokens.dup_token));
EXPECT_TRUE(result);
// Validating should fail, because we've only set the renderer constraints.
diff --git a/src/ui/scenic/lib/flatland/renderer/tests/renderer_unittest.cc b/src/ui/scenic/lib/flatland/renderer/tests/renderer_unittest.cc
index b850d97..2830605 100644
--- a/src/ui/scenic/lib/flatland/renderer/tests/renderer_unittest.cc
+++ b/src/ui/scenic/lib/flatland/renderer/tests/renderer_unittest.cc
@@ -52,8 +52,8 @@
// First id should be valid.
auto bcid = sysmem_util::GenerateUniqueBufferCollectionId();
- auto result =
- renderer->RegisterTextureCollection(bcid, sysmem_allocator, std::move(tokens.local_token));
+ auto result = renderer->RegisterRenderTargetCollection(bcid, sysmem_allocator,
+ std::move(tokens.local_token));
EXPECT_TRUE(result);
}
@@ -75,14 +75,14 @@
// First id should be valid.
auto bcid = sysmem_util::GenerateUniqueBufferCollectionId();
- auto result =
- renderer->RegisterTextureCollection(bcid, sysmem_allocator, std::move(tokens.local_token));
+ auto result = renderer->RegisterRenderTargetCollection(bcid, sysmem_allocator,
+ std::move(tokens.local_token));
EXPECT_TRUE(result);
// Second id should be valid.
auto bcid2 = sysmem_util::GenerateUniqueBufferCollectionId();
- result =
- renderer->RegisterTextureCollection(bcid2, sysmem_allocator, std::move(tokens.dup_token));
+ result = renderer->RegisterRenderTargetCollection(bcid2, sysmem_allocator,
+ std::move(tokens.dup_token));
EXPECT_TRUE(result);
// Set the client constraints.
@@ -106,7 +106,7 @@
void BadTokenTest(Renderer* renderer, fuchsia::sysmem::Allocator_Sync* sysmem_allocator) {
// Null token should fail.
auto bcid = sysmem_util::GenerateUniqueBufferCollectionId();
- auto result = renderer->RegisterTextureCollection(bcid, sysmem_allocator, nullptr);
+ auto result = renderer->RegisterRenderTargetCollection(bcid, sysmem_allocator, nullptr);
EXPECT_FALSE(result);
// A valid channel that isn't a buffer collection should also fail.
@@ -117,7 +117,7 @@
ASSERT_TRUE(handle.is_valid());
bcid = sysmem_util::GenerateUniqueBufferCollectionId();
- result = renderer->RegisterTextureCollection(bcid, sysmem_allocator, std::move(handle));
+ result = renderer->RegisterRenderTargetCollection(bcid, sysmem_allocator, std::move(handle));
EXPECT_FALSE(result);
}
@@ -129,7 +129,7 @@
auto bcid = sysmem_util::GenerateUniqueBufferCollectionId();
auto result =
- renderer->RegisterTextureCollection(bcid, sysmem_allocator, std::move(tokens.dup_token));
+ renderer->RegisterRenderTargetCollection(bcid, sysmem_allocator, std::move(tokens.dup_token));
EXPECT_TRUE(result);
// The buffer collection should not be valid here.
@@ -145,7 +145,7 @@
EXPECT_EQ(validate_result->vmo_count, 1U);
}
-// Simple deregistration test that calls DeregisterCollection() directly without
+// Simple deregistration test that calls ReleaseBufferCollection() directly without
// any zx::events just to make sure that the method's functionality itself is
// working as intented.
void DeregistrationTest(Renderer* renderer, fuchsia::sysmem::Allocator_Sync* sysmem_allocator) {
@@ -153,7 +153,7 @@
auto bcid = sysmem_util::GenerateUniqueBufferCollectionId();
auto result =
- renderer->RegisterTextureCollection(bcid, sysmem_allocator, std::move(tokens.dup_token));
+ renderer->RegisterRenderTargetCollection(bcid, sysmem_allocator, std::move(tokens.dup_token));
EXPECT_TRUE(result);
// The buffer collection should not be valid here.
@@ -169,7 +169,7 @@
EXPECT_EQ(validate_result->vmo_count, 1U);
// Now deregister the collection.
- renderer->DeregisterCollection(bcid);
+ renderer->DeregisterRenderTargetCollection(bcid);
// After deregistration, calling validate should return false.
EXPECT_FALSE(renderer->Validate(bcid));
@@ -184,7 +184,7 @@
std::set<sysmem_util::GlobalBufferCollectionId> bcid_set;
std::mutex lock;
- auto register_and_validate_function = [&renderer, &bcid_set, &lock](bool register_texture) {
+ auto register_and_validate_function = [&renderer, &bcid_set, &lock]() {
// Make a test loop.
async::TestLoop loop;
@@ -195,14 +195,8 @@
auto tokens = CreateSysmemTokens(sysmem_allocator.get());
auto bcid = sysmem_util::GenerateUniqueBufferCollectionId();
- bool result = false;
- if (register_texture) {
- result = renderer->RegisterTextureCollection(bcid, sysmem_allocator.get(),
- std::move(tokens.local_token));
- } else {
- result = renderer->RegisterRenderTargetCollection(bcid, sysmem_allocator.get(),
- std::move(tokens.local_token));
- }
+ bool result = renderer->RegisterRenderTargetCollection(bcid, sysmem_allocator.get(),
+ std::move(tokens.local_token));
EXPECT_TRUE(result);
@@ -228,7 +222,7 @@
// and threads that register render target collections.
std::vector<std::thread> threads;
for (uint32_t i = 0; i < kNumThreads; i++) {
- threads.push_back(std::thread(register_and_validate_function, static_cast<bool>(i % 2)));
+ threads.push_back(std::thread(register_and_validate_function));
}
for (auto&& thread : threads) {
@@ -306,7 +300,7 @@
wait->Begin(dispatcher);
// The call to Render() will signal the release fence, triggering the wait object to
- // call DeregisterCollection().
+ // call its handler function.
std::vector<zx::event> fences;
fences.push_back(std::move(release_fence));
renderer->Render(render_target, {}, {}, fences);
@@ -440,8 +434,8 @@
// Register and validate the collection with the renderer.
auto collection_id = sysmem_util::GenerateUniqueBufferCollectionId();
- auto result = renderer.RegisterTextureCollection(collection_id, sysmem_allocator_.get(),
- std::move(tokens.dup_token));
+ auto result = renderer.ImportBufferCollection(collection_id, sysmem_allocator_.get(),
+ std::move(tokens.dup_token));
EXPECT_TRUE(result);
// Create a client-side handle to the buffer collection and set the client constraints.
@@ -593,8 +587,8 @@
// Register and validate the collection with the renderer.
auto collection_id = sysmem_util::GenerateUniqueBufferCollectionId();
- auto result = renderer.RegisterTextureCollection(collection_id, sysmem_allocator_.get(),
- std::move(tokens.dup_token));
+ auto result = renderer.ImportBufferCollection(collection_id, sysmem_allocator_.get(),
+ std::move(tokens.dup_token));
EXPECT_TRUE(result);
// Create a client-side handle to the buffer collection and set the client constraints.
diff --git a/src/ui/scenic/lib/flatland/renderer/vk_renderer.cc b/src/ui/scenic/lib/flatland/renderer/vk_renderer.cc
index b9b2573..c13c3ff 100644
--- a/src/ui/scenic/lib/flatland/renderer/vk_renderer.cc
+++ b/src/ui/scenic/lib/flatland/renderer/vk_renderer.cc
@@ -38,15 +38,7 @@
}
}
-bool VkRenderer::RegisterRenderTargetCollection(
- sysmem_util::GlobalBufferCollectionId collection_id,
- fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
- fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) {
- return RegisterCollection(collection_id, sysmem_allocator, std::move(token),
- escher::RectangleCompositor::kRenderTargetUsageFlags);
-}
-
-bool VkRenderer::RegisterTextureCollection(
+bool VkRenderer::ImportBufferCollection(
sysmem_util::GlobalBufferCollectionId collection_id,
fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) {
@@ -54,6 +46,37 @@
escher::RectangleCompositor::kTextureUsageFlags);
}
+void VkRenderer::ReleaseBufferCollection(sysmem_util::GlobalBufferCollectionId collection_id) {
+ // Multiple threads may be attempting to read/write from the various maps,
+ // lock this function here.
+ // TODO(fxbug.dev/44335): Convert this to a lock-free structure.
+ std::unique_lock<std::mutex> lock(lock_);
+
+ auto collection_itr = collection_map_.find(collection_id);
+
+ // If the collection is not in the map, then there's nothing to do.
+ if (collection_itr == collection_map_.end()) {
+ return;
+ }
+
+ // Erase the sysmem collection from the map.
+ collection_map_.erase(collection_id);
+
+ // Grab the vulkan collection and DCHECK since there should always be
+ // a vk collection to correspond with the buffer collection. We then
+ // delete it using the vk device.
+ auto vk_itr = vk_collection_map_.find(collection_id);
+ FX_DCHECK(vk_itr != vk_collection_map_.end());
+ auto vk_device = escher_->vk_device();
+ auto vk_loader = escher_->device()->dispatch_loader();
+ vk_device.destroyBufferCollectionFUCHSIA(vk_itr->second, nullptr, vk_loader);
+ vk_collection_map_.erase(collection_id);
+
+ // Erase the metadata. There may not actually be any metadata if the collection was
+ // never validated, but there's no need to check as erasing a non-existent key is valid.
+ collection_metadata_map_.erase(collection_id);
+}
+
bool VkRenderer::RegisterCollection(
sysmem_util::GlobalBufferCollectionId collection_id,
fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
@@ -68,12 +91,7 @@
// vulkan token.
if (!token.is_valid()) {
FX_LOGS(ERROR) << "Token is invalid.";
- return false;
- }
-
- if (collection_map_.find(collection_id) != collection_map_.end()) {
- FX_LOGS(ERROR) << "Duplicate GlobalBufferCollectionID: " << collection_id;
- return false;
+ return sysmem_util::kInvalidId;
}
auto vk_constraints =
@@ -121,35 +139,49 @@
return true;
}
-void VkRenderer::DeregisterCollection(sysmem_util::GlobalBufferCollectionId collection_id) {
- // Multiple threads may be attempting to read/write from the various maps,
- // lock this function here.
- // TODO(fxbug.dev/44335): Convert this to a lock-free structure.
- std::unique_lock<std::mutex> lock(lock_);
-
- auto collection_itr = collection_map_.find(collection_id);
-
- // If the collection is not in the map, then there's nothing to do.
- if (collection_itr == collection_map_.end()) {
- return;
+bool VkRenderer::ImportImage(const ImageMetadata& meta_data) {
+ auto buffer_metadata = Validate(meta_data.collection_id);
+ if (!buffer_metadata.has_value()) {
+ FX_LOGS(ERROR) << "CreateImage failed, collection_id " << meta_data.collection_id
+ << " has not been allocated yet";
+ return false;
}
- // Erase the sysmem collection from the map.
- collection_map_.erase(collection_id);
+ if (meta_data.vmo_idx >= buffer_metadata->vmo_count) {
+ FX_LOGS(ERROR) << "CreateImage failed, vmo_index " << meta_data.vmo_idx
+ << " must be less than vmo_count " << buffer_metadata->vmo_count;
+ return false;
+ }
- // Grab the vulkan collection and DCHECK since there should always be
- // a vk collection to correspond with the buffer collection. We then
- // delete it using the vk device.
- auto vk_itr = vk_collection_map_.find(collection_id);
- FX_DCHECK(vk_itr != vk_collection_map_.end());
- auto vk_device = escher_->vk_device();
- auto vk_loader = escher_->device()->dispatch_loader();
- vk_device.destroyBufferCollectionFUCHSIA(vk_itr->second, nullptr, vk_loader);
- vk_collection_map_.erase(collection_id);
+ const auto& image_constraints = buffer_metadata->image_constraints;
- // Erase the metadata. There may not actually be any metadata if the collection was
- // never validated, but there's no need to check as erasing a non-existent key is valid.
- collection_metadata_map_.erase(collection_id);
+ if (meta_data.width < image_constraints.min_coded_width ||
+ meta_data.width > image_constraints.max_coded_width) {
+ FX_LOGS(ERROR) << "CreateImage failed, width " << meta_data.width
+ << " is not within valid range [" << image_constraints.min_coded_width << ","
+ << image_constraints.max_coded_width << "]";
+ return false;
+ }
+
+ if (meta_data.height < image_constraints.min_coded_height ||
+ meta_data.height > image_constraints.max_coded_height) {
+ FX_LOGS(ERROR) << "CreateImage failed, height " << meta_data.height
+ << " is not within valid range [" << image_constraints.min_coded_height << ","
+ << image_constraints.max_coded_height << "]";
+ return false;
+ }
+
+ // TODO(46708): Actually create and cache the image at this point. Currently we just recreate
+ // the images we need every time RenderFrame() is called, so there's nothing here to
+ // do at the moment other than return true since we have successfully validated the
+ // buffer collection and checked that the ImageMetatada struct has good data.
+ return true;
+}
+
+void VkRenderer::ReleaseImage(GlobalImageId image_id) {
+ // TODO(46708): Fill out this function once we actually start caching images. Currently
+ // we just recreate them every time |RenderFrame| is called, and let them go out
+ // of scope afterwards, so there is nothing here to release.
}
std::optional<BufferCollectionMetadata> VkRenderer::Validate(
@@ -189,26 +221,6 @@
return result;
}
-escher::ImagePtr VkRenderer::ExtractRenderTarget(escher::CommandBuffer* command_buffer,
- ImageMetadata metadata) {
- const vk::ImageLayout kRenderTargetLayout = vk::ImageLayout::eColorAttachmentOptimal;
- auto render_target =
- ExtractImage(command_buffer, metadata, escher::RectangleCompositor::kRenderTargetUsageFlags,
- kRenderTargetLayout);
- render_target->set_swapchain_layout(kRenderTargetLayout);
- return render_target;
-}
-
-escher::TexturePtr VkRenderer::ExtractTexture(escher::CommandBuffer* command_buffer,
- ImageMetadata metadata) {
- auto image =
- ExtractImage(command_buffer, metadata, escher::RectangleCompositor::kTextureUsageFlags,
- vk::ImageLayout::eShaderReadOnlyOptimal);
- auto texture = escher::Texture::New(escher_->resource_recycler(), image, vk::Filter::eNearest);
- FX_DCHECK(texture);
- return texture;
-}
-
escher::ImagePtr VkRenderer::ExtractImage(escher::CommandBuffer* command_buffer,
ImageMetadata metadata, vk::ImageUsageFlags usage,
vk::ImageLayout layout) {
@@ -248,6 +260,39 @@
return image_ptr;
}
+bool VkRenderer::RegisterRenderTargetCollection(
+ sysmem_util::GlobalBufferCollectionId collection_id,
+ fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
+ fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) {
+ return RegisterCollection(collection_id, sysmem_allocator, std::move(token),
+ escher::RectangleCompositor::kRenderTargetUsageFlags);
+}
+
+void VkRenderer::DeregisterRenderTargetCollection(
+ sysmem_util::GlobalBufferCollectionId collection_id) {
+ ReleaseBufferCollection(collection_id);
+}
+
+escher::ImagePtr VkRenderer::ExtractRenderTarget(escher::CommandBuffer* command_buffer,
+ ImageMetadata metadata) {
+ const vk::ImageLayout kRenderTargetLayout = vk::ImageLayout::eColorAttachmentOptimal;
+ auto render_target =
+ ExtractImage(command_buffer, metadata, escher::RectangleCompositor::kRenderTargetUsageFlags,
+ kRenderTargetLayout);
+ render_target->set_swapchain_layout(kRenderTargetLayout);
+ return render_target;
+}
+
+escher::TexturePtr VkRenderer::ExtractTexture(escher::CommandBuffer* command_buffer,
+ ImageMetadata metadata) {
+ auto image =
+ ExtractImage(command_buffer, metadata, escher::RectangleCompositor::kTextureUsageFlags,
+ vk::ImageLayout::eShaderReadOnlyOptimal);
+ auto texture = escher::Texture::New(escher_->resource_recycler(), image, vk::Filter::eNearest);
+ FX_DCHECK(texture);
+ return texture;
+}
+
void VkRenderer::Render(const ImageMetadata& render_target,
const std::vector<Rectangle2D>& rectangles,
const std::vector<ImageMetadata>& images,
diff --git a/src/ui/scenic/lib/flatland/renderer/vk_renderer.h b/src/ui/scenic/lib/flatland/renderer/vk_renderer.h
index 05fcef9..16b6376 100644
--- a/src/ui/scenic/lib/flatland/renderer/vk_renderer.h
+++ b/src/ui/scenic/lib/flatland/renderer/vk_renderer.h
@@ -8,7 +8,6 @@
#include <unordered_map>
#include "src/ui/lib/escher/flatland/rectangle_compositor.h"
-#include "src/ui/lib/escher/resources/resource_recycler.h"
#include "src/ui/scenic/lib/flatland/renderer/gpu_mem.h"
#include "src/ui/scenic/lib/flatland/renderer/renderer.h"
@@ -21,12 +20,21 @@
explicit VkRenderer(std::unique_ptr<escher::Escher> escher);
~VkRenderer() override;
- // |Renderer|.
- bool RegisterTextureCollection(
+ // |BufferCollectionImporter|
+ bool ImportBufferCollection(
sysmem_util::GlobalBufferCollectionId collection_id,
fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) override;
+ // |BufferCollectionImporter|
+ void ReleaseBufferCollection(sysmem_util::GlobalBufferCollectionId collection_id) override;
+
+ // |BufferCollectionImporter|
+ bool ImportImage(const ImageMetadata& meta_data) override;
+
+ // |BufferCollectionImporter|
+ void ReleaseImage(GlobalImageId image_id) override;
+
// |Renderer|.
bool RegisterRenderTargetCollection(
sysmem_util::GlobalBufferCollectionId collection_id,
@@ -34,7 +42,8 @@
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token) override;
// |Renderer|.
- void DeregisterCollection(sysmem_util::GlobalBufferCollectionId collection_id) override;
+ void DeregisterRenderTargetCollection(
+ sysmem_util::GlobalBufferCollectionId collection_id) override;
// |Renderer|.
std::optional<BufferCollectionMetadata> Validate(
@@ -49,12 +58,16 @@
void WaitIdle();
private:
+ // Generic helper function used by both |ImportBufferCollection| and |RegisterTextureCollection|.
bool RegisterCollection(sysmem_util::GlobalBufferCollectionId collection_id,
fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token,
vk::ImageUsageFlags usage);
// The function ExtractImage() creates an escher Image from a sysmem collection vmo.
+ escher::ImagePtr ExtractImage(escher::CommandBuffer* command_buffer, ImageMetadata metadata,
+ vk::ImageUsageFlags usage, vk::ImageLayout layout);
+
// ExtractRenderTarget() and ExtractTexture() are wrapper functions to ExtractImage()
// that provide specific usage flags for color attachments and shader textures
// respectively. All three functions are thread safe. Additionally, they only get
@@ -62,11 +75,11 @@
escher::TexturePtr ExtractTexture(escher::CommandBuffer* command_buffer, ImageMetadata metadata);
escher::ImagePtr ExtractRenderTarget(escher::CommandBuffer* command_buffer,
ImageMetadata metadata);
- escher::ImagePtr ExtractImage(escher::CommandBuffer* command_buffer, ImageMetadata metadata,
- vk::ImageUsageFlags usage, vk::ImageLayout layout);
- // Vulkan rendering components.
+ // Escher is how we access Vulkan.
std::unique_ptr<escher::Escher> escher_;
+
+ // Vulkan rendering component.
escher::RectangleCompositor compositor_;
// This mutex is used to protect access to |collection_map_|, |collection_metadata_map_|,
diff --git a/src/ui/scenic/lib/flatland/tests/flatland_manager_unittest.cc b/src/ui/scenic/lib/flatland/tests/flatland_manager_unittest.cc
index e73f335..a549bee 100644
--- a/src/ui/scenic/lib/flatland/tests/flatland_manager_unittest.cc
+++ b/src/ui/scenic/lib/flatland/tests/flatland_manager_unittest.cc
@@ -10,10 +10,8 @@
#include "fuchsia/ui/scenic/internal/cpp/fidl.h"
#include "lib/gtest/real_loop_fixture.h"
-#include "src/ui/scenic/lib/flatland/renderer/renderer.h"
#include "src/ui/scenic/lib/flatland/tests/mock_buffer_collection_importer.h"
#include "src/ui/scenic/lib/flatland/tests/mock_flatland_presenter.h"
-#include "src/ui/scenic/lib/flatland/tests/mock_renderer.h"
#include "src/ui/scenic/lib/scheduling/frame_scheduler.h"
#include "src/ui/scenic/lib/scheduling/id.h"
@@ -24,8 +22,6 @@
using flatland::FlatlandPresenter;
using flatland::LinkSystem;
using flatland::MockFlatlandPresenter;
-using flatland::MockRenderer;
-using flatland::Renderer;
using flatland::UberStructSystem;
using fuchsia::ui::scenic::internal::Flatland;
using fuchsia::ui::scenic::internal::Flatland_Present_Result;
@@ -111,10 +107,8 @@
flatland_presenter_ = std::shared_ptr<FlatlandPresenter>(mock_flatland_presenter_);
- mock_renderer_ = new ::testing::StrictMock<MockRenderer>();
- renderer_ = std::shared_ptr<Renderer>(mock_renderer_);
manager_ = std::make_unique<FlatlandManager>(
- flatland_presenter_, renderer_, uber_struct_system_, link_system_,
+ flatland_presenter_, uber_struct_system_, link_system_,
std::vector<std::shared_ptr<flatland::BufferCollectionImporter>>());
}
@@ -131,7 +125,6 @@
EXPECT_TRUE(snapshot.empty());
manager_.reset();
- renderer_.reset();
flatland_presenter_.reset();
gtest::RealLoopFixture::TearDown();
@@ -151,14 +144,12 @@
protected:
::testing::StrictMock<MockFlatlandPresenter>* mock_flatland_presenter_;
- ::testing::StrictMock<MockRenderer>* mock_renderer_;
const std::shared_ptr<UberStructSystem> uber_struct_system_;
std::unique_ptr<FlatlandManager> manager_;
private:
std::shared_ptr<FlatlandPresenter> flatland_presenter_;
- std::shared_ptr<Renderer> renderer_;
const std::shared_ptr<LinkSystem> link_system_;
// Storage for |mock_flatland_presenter_|.
diff --git a/src/ui/scenic/lib/flatland/tests/flatland_unittest.cc b/src/ui/scenic/lib/flatland/tests/flatland_unittest.cc
index d74da41..c717c3f 100644
--- a/src/ui/scenic/lib/flatland/tests/flatland_unittest.cc
+++ b/src/ui/scenic/lib/flatland/tests/flatland_unittest.cc
@@ -17,7 +17,6 @@
#include "src/lib/fsl/handles/object_info.h"
#include "src/ui/scenic/lib/flatland/global_matrix_data.h"
#include "src/ui/scenic/lib/flatland/global_topology_data.h"
-#include "src/ui/scenic/lib/flatland/renderer/renderer.h"
#include "src/ui/scenic/lib/flatland/tests/mock_buffer_collection_importer.h"
#include "src/ui/scenic/lib/flatland/tests/mock_flatland_presenter.h"
#include "src/ui/scenic/lib/flatland/tests/mock_renderer.h"
@@ -42,8 +41,6 @@
using flatland::LinkSystem;
using flatland::MockBufferCollectionImporter;
using flatland::MockFlatlandPresenter;
-using flatland::MockRenderer;
-using flatland::Renderer;
using flatland::TransformGraph;
using flatland::TransformHandle;
using flatland::UberStruct;
@@ -241,9 +238,6 @@
flatland_presenter_ = std::shared_ptr<FlatlandPresenter>(mock_flatland_presenter_);
- mock_renderer_ = new ::testing::StrictMock<MockRenderer>();
- renderer_ = std::shared_ptr<Renderer>(mock_renderer_);
-
mock_buffer_collection_importer_ = new MockBufferCollectionImporter();
buffer_collection_importer_ =
std::shared_ptr<BufferCollectionImporter>(mock_buffer_collection_importer_);
@@ -255,7 +249,6 @@
auto link_topologies = link_system_->GetResolvedTopologyLinks();
EXPECT_TRUE(link_topologies.empty());
- renderer_.reset();
buffer_collection_importer_.reset();
flatland_presenter_.reset();
@@ -267,7 +260,7 @@
Flatland CreateFlatland() {
fuchsia::sysmem::AllocatorSyncPtr sysmem_allocator;
auto session_id = scheduling::GetNextSessionId();
- return Flatland(session_id, flatland_presenter_, renderer_, link_system_,
+ return Flatland(session_id, flatland_presenter_, link_system_,
uber_struct_system_->AllocateQueueForSession(session_id),
{buffer_collection_importer_}, std::move(sysmem_allocator));
}
@@ -407,7 +400,7 @@
GlobalIdPair CreateImage(Flatland* flatland, ContentId image_id, BufferCollectionId collection_id,
ImageProperties properties) {
sysmem_util::GlobalBufferCollectionId global_collection_id = sysmem_util::kInvalidId;
- EXPECT_CALL(*mock_renderer_, RegisterTextureCollection(_, _, _))
+ EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _))
.WillOnce(testing::Invoke(
[&global_collection_id](
sysmem_util::GlobalBufferCollectionId collection_id,
@@ -417,8 +410,6 @@
return true;
}));
- EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _)).Times(1);
-
flatland->RegisterBufferCollection(collection_id, CreateToken());
EXPECT_NE(global_collection_id, sysmem_util::kInvalidId);
@@ -436,7 +427,6 @@
metadata.image_constraints.max_coded_height = properties.height();
flatland::GlobalImageId global_image_id;
- EXPECT_CALL(*mock_renderer_, Validate(global_collection_id)).WillOnce(Return(metadata));
EXPECT_CALL(*mock_buffer_collection_importer_, ImportImage(_))
.WillOnce(testing::Invoke([&global_image_id](const ImageMetadata& meta_data) {
global_image_id = meta_data.identifier;
@@ -451,9 +441,7 @@
protected:
::testing::StrictMock<MockFlatlandPresenter>* mock_flatland_presenter_;
- ::testing::StrictMock<MockRenderer>* mock_renderer_;
MockBufferCollectionImporter* mock_buffer_collection_importer_;
- std::shared_ptr<Renderer> renderer_;
std::shared_ptr<BufferCollectionImporter> buffer_collection_importer_;
const std::shared_ptr<UberStructSystem> uber_struct_system_;
std::shared_ptr<FlatlandPresenter> flatland_presenter_;
@@ -2826,10 +2814,11 @@
PRESENT(flatland, false);
}
- // The Renderer registration call can fail.
+ // The buffer importer call can fail.
{
- // Mock the Renderer call to fail.
- EXPECT_CALL(*mock_renderer_, RegisterTextureCollection(_, _, _)).WillOnce(Return(false));
+ // Mock the importer call to fail.
+ EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _))
+ .WillOnce(Return(false));
flatland.RegisterBufferCollection(1, CreateToken());
PRESENT(flatland, false);
@@ -2839,8 +2828,8 @@
{
const BufferCollectionId kId = 2;
- EXPECT_CALL(*mock_renderer_, RegisterTextureCollection(_, _, _)).WillOnce(Return(true));
- EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _)).Times(1);
+ EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _))
+ .WillOnce(Return(true));
flatland.RegisterBufferCollection(kId, CreateToken());
PRESENT(flatland, true);
@@ -2850,16 +2839,15 @@
}
}
-// Tests that Flatland passes the Sysmem token to the Renderer even if the client has not called
+// Tests that Flatland passes the Sysmem token to the importer even if the client has not called
// Present(). This is necessary since the client may block on buffers being allocated before
// presenting.
-TEST_F(FlatlandTest, RendererGetsSysmemTokenBeforePresent) {
+TEST_F(FlatlandTest, BufferImporterGetsSysmemTokenBeforePresent) {
Flatland flatland = CreateFlatland();
- // Register a buffer collection and expect the mock Renderer call, even without presenting.
+ // Register a buffer collection and expect the mock buffer importer call, even without presenting.
const BufferCollectionId kId = 2;
- EXPECT_CALL(*mock_renderer_, RegisterTextureCollection(_, _, _)).WillOnce(Return(true));
EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _)).Times(1);
flatland.RegisterBufferCollection(kId, CreateToken());
}
@@ -2875,7 +2863,7 @@
// Setup a valid buffer collection.
const BufferCollectionId kBufferCollectionId = 1;
sysmem_util::GlobalBufferCollectionId global_collection_id;
- EXPECT_CALL(*mock_renderer_, RegisterTextureCollection(_, _, _))
+ EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _))
.WillOnce(
testing::Invoke([&global_collection_id](
sysmem_util::GlobalBufferCollectionId collection_id,
@@ -2885,8 +2873,6 @@
return true;
}));
- EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _)).Times(1);
-
flatland.RegisterBufferCollection(kBufferCollectionId, CreateToken());
PRESENT(flatland, true);
@@ -2904,83 +2890,22 @@
PRESENT(flatland, false);
}
- // The buffer collection can fail to validate.
+ // The buffer collection can fail to create an image.
{
- EXPECT_CALL(*mock_renderer_, Validate(global_collection_id)).WillOnce(Return(std::nullopt));
-
ImageProperties properties;
flatland.CreateImage(1, kBufferCollectionId, kDefaultVmoIndex, std::move(properties));
PRESENT(flatland, false);
}
- // The remaining error cases tested below occur after the BufferCollection is validated. Because
- // the results of validation is cached, we set up valid ranges for all properties once here.
- BufferCollectionMetadata metadata;
- metadata.vmo_count = 2;
- metadata.image_constraints.min_coded_width = 50;
- metadata.image_constraints.max_coded_width = 150;
- metadata.image_constraints.min_coded_height = 500;
- metadata.image_constraints.max_coded_height = 1500;
- EXPECT_CALL(*mock_renderer_, Validate(global_collection_id)).WillOnce(Return(metadata));
-
- // The vmo index must be less than the vmo count.
+ // Check to make sure that if the BufferCollectionImporter returns false, then the call
+ // to Flatland::CreateImage() also returns false.
{
- ImageProperties properties;
- flatland.CreateImage(1, kBufferCollectionId, /*vmo_idx*/ 3, std::move(properties));
- PRESENT(flatland, false);
- }
-
- // The width must be set.
- {
- ImageProperties properties;
- properties.set_height(kDefaultHeight);
-
- flatland.CreateImage(1, kBufferCollectionId, kDefaultVmoIndex, std::move(properties));
- PRESENT(flatland, false);
- }
-
- // The width must be within the valid range.
- {
- ImageProperties properties;
- properties.set_width(10);
- properties.set_height(kDefaultHeight);
-
- flatland.CreateImage(1, kBufferCollectionId, kDefaultVmoIndex, std::move(properties));
- PRESENT(flatland, false);
- }
- {
- ImageProperties properties;
- properties.set_width(1000);
- properties.set_height(kDefaultHeight);
-
- flatland.CreateImage(1, kBufferCollectionId, kDefaultVmoIndex, std::move(properties));
- PRESENT(flatland, false);
- }
-
- // The height must be set.
- {
+ const ContentId kId = 100;
ImageProperties properties;
properties.set_width(kDefaultWidth);
-
- flatland.CreateImage(1, kBufferCollectionId, kDefaultVmoIndex, std::move(properties));
- PRESENT(flatland, false);
- }
-
- // The height must be within the valid range.
- {
- ImageProperties properties;
- properties.set_width(kDefaultWidth);
- properties.set_height(100);
-
- flatland.CreateImage(1, kBufferCollectionId, kDefaultVmoIndex, std::move(properties));
- PRESENT(flatland, false);
- }
- {
- ImageProperties properties;
- properties.set_width(kDefaultWidth);
- properties.set_height(10000);
-
- flatland.CreateImage(1, kBufferCollectionId, kDefaultVmoIndex, std::move(properties));
+ properties.set_height(kDefaultHeight);
+ EXPECT_CALL(*mock_buffer_collection_importer_, ImportImage(_)).WillOnce(Return(false));
+ flatland.CreateImage(kId, kBufferCollectionId, kDefaultVmoIndex, std::move(properties));
PRESENT(flatland, false);
}
@@ -3210,7 +3135,7 @@
// A buffer collection cannot be deregistered twice.
sysmem_util::GlobalBufferCollectionId global_collection_id;
- EXPECT_CALL(*mock_renderer_, RegisterTextureCollection(_, _, _))
+ EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _))
.WillOnce(
testing::Invoke([&global_collection_id](
sysmem_util::GlobalBufferCollectionId collection_id,
@@ -3219,15 +3144,13 @@
global_collection_id = collection_id;
return true;
}));
- EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _)).Times(1);
const BufferCollectionId kBufferCollectionId = 3;
flatland.RegisterBufferCollection(kBufferCollectionId, CreateToken());
PRESENT(flatland, true);
- // The PRESENT macro triggers release fences, which in turn triggers the Renderer call.
+ // The PRESENT macro triggers release fences, which in turn triggers the buffer importer call.
flatland.DeregisterBufferCollection(kBufferCollectionId);
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id)).Times(1);
EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id))
.Times(1);
PRESENT(flatland, true);
@@ -3241,7 +3164,7 @@
// Register the first buffer collection.
sysmem_util::GlobalBufferCollectionId global_collection_id_1;
- EXPECT_CALL(*mock_renderer_, RegisterTextureCollection(_, _, _))
+ EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _))
.WillOnce(
testing::Invoke([&global_collection_id_1](
sysmem_util::GlobalBufferCollectionId collection_id,
@@ -3250,14 +3173,13 @@
global_collection_id_1 = collection_id;
return true;
}));
- EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _)).Times(1);
const BufferCollectionId kBufferCollectionId1 = 2;
{ flatland.RegisterBufferCollection(kBufferCollectionId1, CreateToken()); }
// Register the second buffer collection.
sysmem_util::GlobalBufferCollectionId global_collection_id_2;
- EXPECT_CALL(*mock_renderer_, RegisterTextureCollection(_, _, _))
+ EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _))
.WillOnce(
testing::Invoke([&global_collection_id_2](
sysmem_util::GlobalBufferCollectionId collection_id,
@@ -3266,7 +3188,6 @@
global_collection_id_2 = collection_id;
return true;
}));
- EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _)).Times(1);
const BufferCollectionId kBufferCollectionId2 = 4;
{ flatland.RegisterBufferCollection(kBufferCollectionId2, CreateToken()); }
@@ -3277,17 +3198,21 @@
flatland.DeregisterBufferCollection(kBufferCollectionId1);
flatland.DeregisterBufferCollection(kBufferCollectionId2);
- // Skip session updates to test that release fences are what trigger the Renderer calls.
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id_1)).Times(0);
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id_2)).Times(0);
+ // Skip session updates to test that release fences are what trigger the buffer importer calls.
+ EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id_1))
+ .Times(0);
+ EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id_2))
+ .Times(0);
PresentArgs args;
args.skip_session_update_and_release_fences = true;
PRESENT_WITH_ARGS(flatland, std::move(args), true);
// Signal the release fence.
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id_1)).Times(1);
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id_2)).Times(1);
+ EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id_1))
+ .Times(1);
+ EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id_2))
+ .Times(1);
ApplySessionUpdatesAndSignalFences();
RunLoopUntilIdle();
}
@@ -3297,7 +3222,7 @@
// Create a valid BufferCollectionId.
sysmem_util::GlobalBufferCollectionId global_collection_id_1;
- EXPECT_CALL(*mock_renderer_, RegisterTextureCollection(_, _, _))
+ EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _))
.WillOnce(
testing::Invoke([&global_collection_id_1](
sysmem_util::GlobalBufferCollectionId collection_id,
@@ -3306,7 +3231,6 @@
global_collection_id_1 = collection_id;
return true;
}));
- EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _)).Times(1);
const BufferCollectionId kBufferCollectionId = 2;
{
@@ -3318,8 +3242,9 @@
flatland.DeregisterBufferCollection(kBufferCollectionId);
{
- // Skip session updates to test that release fences are what trigger the Renderer calls.
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id_1)).Times(0);
+ // Skip session updates to test that release fences are what trigger the buffer importer calls.
+ EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id_1))
+ .Times(0);
PresentArgs args;
args.skip_session_update_and_release_fences = true;
@@ -3328,7 +3253,7 @@
// Register another buffer collection with that same ID.
sysmem_util::GlobalBufferCollectionId global_collection_id_2;
- EXPECT_CALL(*mock_renderer_, RegisterTextureCollection(_, _, _))
+ EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _))
.WillOnce(
testing::Invoke([&global_collection_id_2](
sysmem_util::GlobalBufferCollectionId collection_id,
@@ -3337,37 +3262,36 @@
global_collection_id_2 = collection_id;
return true;
}));
- EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _)).Times(1);
{
flatland.RegisterBufferCollection(kBufferCollectionId, CreateToken());
- // Skip session updates to test that release fences are what trigger the Renderer calls.
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id_1)).Times(0);
+ // Skip session updates to test that release fences are what trigger the buffer importer calls.
+ EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id_1))
+ .Times(0);
PresentArgs args;
args.skip_session_update_and_release_fences = true;
PRESENT_WITH_ARGS(flatland, std::move(args), true);
}
- // Signal the release fences, which should result deregister the first one from the renderer.
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id_1)).Times(1);
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id_2)).Times(0);
+ // Signal the release fences, which should result deregister the first one from the importer.
EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id_1))
.Times(1);
EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id_2))
- .Times(1);
+ .Times(0);
ApplySessionUpdatesAndSignalFences();
RunLoopUntilIdle();
// Deregister the second one, signal the release fences, and verify the second global ID was
// deregistered.
flatland.DeregisterBufferCollection(kBufferCollectionId);
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id_2)).Times(1);
+ EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id_2))
+ .Times(1);
PRESENT(flatland, true);
}
-// Tests that a buffer collection is not deregistered from the Renderer until it is not referenced
+// Tests that a buffer collection is not deregistered from the importer until it is not referenced
// by any active Image (including released Images still on Transforms) and the release fence is
// signaled.
TEST_F(FlatlandTest, DeregisterBufferCollectionWaitsForReleaseFence) {
@@ -3392,9 +3316,8 @@
flatland.SetContentOnTransform(kImageId, kTransformId);
PRESENT(flatland, true);
- // Deregister the buffer collection, but ensure that the deregistration call on the Renderer has
+ // Deregister the buffer collection, but ensure that the deregistration call on the importer has
// not happened.
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id)).Times(0);
EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id))
.Times(0);
flatland.DeregisterBufferCollection(kBufferCollectionId);
@@ -3402,7 +3325,6 @@
// Release the Image that referenced the buffer collection. Because the Image is still attached
// to a Transform, the deregestration call should still not happen.
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id)).Times(0);
EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id))
.Times(0);
flatland.ReleaseImage(kImageId);
@@ -3410,8 +3332,7 @@
// Remove the Image from the transform. This triggers the creation of the release fence, but
// still does not result in a deregestration call. Skip session updates to test that release
- // fences are what trigger the Renderer calls.
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id)).Times(0);
+ // fences are what trigger the importer calls.
EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id))
.Times(0);
flatland.SetContentOnTransform(0, kTransformId);
@@ -3421,7 +3342,6 @@
PRESENT_WITH_ARGS(flatland, std::move(args), true);
// Signal the release fences, which triggers the deregistration call.
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id)).Times(1);
EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id))
.Times(1);
ApplySessionUpdatesAndSignalFences();
@@ -3450,8 +3370,7 @@
// Deregister the buffer collection.
flatland.DeregisterBufferCollection(kBufferCollectionId);
- // Skip session updates to test that release fences are what trigger the Renderer calls.
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id)).Times(0);
+ // Skip session updates to test that release fences are what trigger the importer calls.
EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id))
.Times(0);
EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseImage(global_image_id)).Times(0);
@@ -3462,12 +3381,12 @@
// |flatland| falls out of scope.
}
- // Reset the last known reference to the Renderer to demonstrate that the Wait keeps it alive.
- renderer_.reset();
+ // Reset the last known reference to the BufferImporter to demonstrate that the Wait keeps it
+ // alive.
+ buffer_collection_importer_.reset();
// Signal the release fences, which triggers the deregistration call, even though the Flatland
// instance and Renderer associated with the call have been cleaned up.
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id)).Times(1);
EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id))
.Times(1);
EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseImage(global_image_id)).Times(1);
@@ -3504,6 +3423,36 @@
}
// If we have multiple BufferCollectionImporters, some of them may properly import
+// a buffer collection while others do not. We have to therefore make sure that if
+// improter A properly imports a buffer collection and then importer B fails, that
+// Flatland automatically releases the buffer collection from importer A.
+TEST_F(FlatlandTest, BufferCollectionImportPassesAndFailsOnDifferentImportersTest) {
+ // Create a second buffer collection importer.
+ auto local_mock_buffer_collection_importer = new MockBufferCollectionImporter();
+ auto local_buffer_collection_importer =
+ std::shared_ptr<BufferCollectionImporter>(local_mock_buffer_collection_importer);
+
+ // Create a flatland instance that has two BufferCollectionImporters.
+ fuchsia::sysmem::AllocatorSyncPtr sysmem_allocator;
+ auto session_id = scheduling::GetNextSessionId();
+ auto flatland = Flatland(session_id, flatland_presenter_, link_system_,
+ uber_struct_system_->AllocateQueueForSession(session_id),
+ {buffer_collection_importer_, local_buffer_collection_importer},
+ std::move(sysmem_allocator));
+
+ EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*local_mock_buffer_collection_importer, ImportBufferCollection(_, _, _))
+ .WillOnce(Return(false));
+
+ EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(_)).Times(1);
+ EXPECT_CALL(*local_mock_buffer_collection_importer, ReleaseBufferCollection(_)).Times(0);
+
+ const auto kCollectionId = 2;
+ flatland.RegisterBufferCollection(kCollectionId, CreateToken());
+}
+
+// If we have multiple BufferCollectionImporters, some of them may properly import
// an image while others do not. We have to therefore make sure that if importer A
// properly imports an image and then importer B fails, that Flatland automatically
// releases the image from importer A.
@@ -3516,13 +3465,13 @@
// Create a flatland instance that has
fuchsia::sysmem::AllocatorSyncPtr sysmem_allocator;
auto session_id = scheduling::GetNextSessionId();
- auto flatland = Flatland(session_id, flatland_presenter_, renderer_, link_system_,
+ auto flatland = Flatland(session_id, flatland_presenter_, link_system_,
uber_struct_system_->AllocateQueueForSession(session_id),
{buffer_collection_importer_, local_buffer_collection_importer},
std::move(sysmem_allocator));
sysmem_util::GlobalBufferCollectionId global_collection_id;
- EXPECT_CALL(*mock_renderer_, RegisterTextureCollection(_, _, _))
+ EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _))
.WillOnce(
testing::Invoke([&global_collection_id](
sysmem_util::GlobalBufferCollectionId collection_id,
@@ -3531,9 +3480,8 @@
global_collection_id = collection_id;
return true;
}));
-
- EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _)).Times(1);
- EXPECT_CALL(*local_mock_buffer_collection_importer, ImportBufferCollection(_, _, _)).Times(1);
+ EXPECT_CALL(*local_mock_buffer_collection_importer, ImportBufferCollection(_, _, _))
+ .WillOnce(Return(true));
const auto kCollectionId = 2;
flatland.RegisterBufferCollection(kCollectionId, CreateToken());
@@ -3542,16 +3490,6 @@
properties.set_width(100);
properties.set_height(200);
- // Ensure all buffer constraints are valid for the desired image by generating constraints based
- // on the image properties.
- BufferCollectionMetadata metadata;
- metadata.vmo_count = 1;
- metadata.image_constraints.min_coded_width = properties.width();
- metadata.image_constraints.max_coded_width = properties.width();
- metadata.image_constraints.min_coded_height = properties.height();
- metadata.image_constraints.max_coded_height = properties.height();
- EXPECT_CALL(*mock_renderer_, Validate(global_collection_id)).WillOnce(Return(metadata));
-
// We have the first importer return true, signifying a successful import, and the second one
// returning false. This should trigger the first importer to call ReleaseImage().
EXPECT_CALL(*mock_buffer_collection_importer_, ImportImage(_)).WillOnce(Return(true));
@@ -3565,10 +3503,9 @@
TEST_F(FlatlandTest, BufferImporterImportImageReturnsFalseTest) {
Flatland flatland = CreateFlatland();
- EXPECT_CALL(*mock_renderer_, RegisterTextureCollection(_, _, _)).WillOnce(Return(true));
- EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _)).Times(1);
-
const auto kCollectionId = 3;
+ EXPECT_CALL(*mock_buffer_collection_importer_, ImportBufferCollection(_, _, _))
+ .WillOnce(Return(true));
flatland.RegisterBufferCollection(kCollectionId, CreateToken());
// Create a proper properties struct.
@@ -3576,15 +3513,6 @@
properties.set_width(150);
properties.set_height(175);
- // We need the mock renderer to return valid buffer metadata so that we can make it down
- // to the importer later in the call to CreateImage().
- BufferCollectionMetadata metadata;
- metadata.vmo_count = 1;
- metadata.image_constraints.min_coded_width = 100;
- metadata.image_constraints.max_coded_width = 200;
- metadata.image_constraints.min_coded_height = 100;
- metadata.image_constraints.max_coded_height = 200;
- EXPECT_CALL(*mock_renderer_, Validate(_)).WillOnce(Return(metadata));
EXPECT_CALL(*mock_buffer_collection_importer_, ImportImage(_)).WillOnce(Return(true));
// We've imported a proper image and we have the importer returning true, so
@@ -3836,7 +3764,8 @@
// Clear the graph, then signal the release fence and ensure the buffer collection is released.
flatland.ClearGraph();
- EXPECT_CALL(*mock_renderer_, DeregisterCollection(global_collection_id1)).Times(1);
+ EXPECT_CALL(*mock_buffer_collection_importer_, ReleaseBufferCollection(global_collection_id1))
+ .Times(1);
PRESENT(flatland, true);
// The buffer collection and Image ID should be available for re-use.
diff --git a/src/ui/scenic/lib/flatland/tests/mock_buffer_collection_importer.h b/src/ui/scenic/lib/flatland/tests/mock_buffer_collection_importer.h
index c1b7291..a9ac81b 100644
--- a/src/ui/scenic/lib/flatland/tests/mock_buffer_collection_importer.h
+++ b/src/ui/scenic/lib/flatland/tests/mock_buffer_collection_importer.h
@@ -15,7 +15,7 @@
class MockBufferCollectionImporter : public BufferCollectionImporter {
public:
MOCK_METHOD3(ImportBufferCollection,
- void(sysmem_util::GlobalBufferCollectionId, fuchsia::sysmem::Allocator_Sync*,
+ bool(sysmem_util::GlobalBufferCollectionId, fuchsia::sysmem::Allocator_Sync*,
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken>));
MOCK_METHOD1(ReleaseBufferCollection, void(sysmem_util::GlobalBufferCollectionId));
diff --git a/src/ui/scenic/lib/flatland/tests/mock_renderer.h b/src/ui/scenic/lib/flatland/tests/mock_renderer.h
index 589c524..43775d5 100644
--- a/src/ui/scenic/lib/flatland/tests/mock_renderer.h
+++ b/src/ui/scenic/lib/flatland/tests/mock_renderer.h
@@ -14,17 +14,11 @@
// Mock class of Renderer for Flatland API testing.
class MockRenderer : public Renderer {
public:
- MOCK_METHOD3(RegisterTextureCollection,
- bool(sysmem_util::GlobalBufferCollectionId collection_id,
- fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
- fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token));
MOCK_METHOD3(RegisterRenderTargetCollection,
bool(sysmem_util::GlobalBufferCollectionId collection_id,
fuchsia::sysmem::Allocator_Sync* sysmem_allocator,
fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> token));
- MOCK_METHOD1(DeregisterCollection, void(sysmem_util::GlobalBufferCollectionId collection_id));
- MOCK_METHOD1(Validate, std::optional<BufferCollectionMetadata>(
- sysmem_util::GlobalBufferCollectionId collection_id));
+
MOCK_METHOD4(Render,
void(const ImageMetadata& render_target, const std::vector<Rectangle2D>& rectangles,
const std::vector<ImageMetadata>& images,
diff --git a/src/ui/scenic/lib/flatland/uber_struct.h b/src/ui/scenic/lib/flatland/uber_struct.h
index a4f470a..3435e4b 100644
--- a/src/ui/scenic/lib/flatland/uber_struct.h
+++ b/src/ui/scenic/lib/flatland/uber_struct.h
@@ -16,7 +16,7 @@
#include <glm/mat3x3.hpp>
-#include "src/ui/scenic/lib/flatland/renderer/renderer.h"
+#include "src/ui/scenic/lib/flatland/renderer/buffer_collection_importer.h"
#include "src/ui/scenic/lib/flatland/transform_graph.h"
#include "src/ui/scenic/lib/flatland/transform_handle.h"
