DVR API: Implement support for deleting surface attributes.
- Implement support for deleting surface attributes using the "NONE"
attribute type.
- Add tests for attribute events and attribute deletion.
Bug: 62456002
Test: dvr_api-test passes.
Change-Id: I66b6edbd35077596d89e85829bcbe7c52829ef5b
diff --git a/libs/vr/libdvr/dvr_surface.cpp b/libs/vr/libdvr/dvr_surface.cpp
index c44f8a6..30f25e3 100644
--- a/libs/vr/libdvr/dvr_surface.cpp
+++ b/libs/vr/libdvr/dvr_surface.cpp
@@ -2,6 +2,7 @@
#include <inttypes.h>
+#include <pdx/rpc/variant.h>
#include <private/android/AHardwareBufferHelpers.h>
#include <private/dvr/display_client.h>
@@ -14,6 +15,7 @@
using android::dvr::display::SurfaceAttributes;
using android::dvr::display::SurfaceAttributeValue;
using android::dvr::CreateDvrReadBufferFromBufferConsumer;
+using android::pdx::rpc::EmptyVariant;
namespace {
@@ -51,6 +53,9 @@
case DVR_SURFACE_ATTRIBUTE_TYPE_FLOAT16:
value = attributes[i].value.float16_value;
break;
+ case DVR_SURFACE_ATTRIBUTE_TYPE_NONE:
+ value = EmptyVariant{};
+ break;
default:
*error_index = i;
return false;
diff --git a/libs/vr/libdvr/tests/dvr_display_manager-test.cpp b/libs/vr/libdvr/tests/dvr_display_manager-test.cpp
index 726949d..f1614d6 100644
--- a/libs/vr/libdvr/tests/dvr_display_manager-test.cpp
+++ b/libs/vr/libdvr/tests/dvr_display_manager-test.cpp
@@ -41,6 +41,13 @@
return attribute;
}
+DvrSurfaceAttribute GetAttribute(DvrSurfaceAttributeKey key, nullptr_t) {
+ DvrSurfaceAttribute attribute;
+ attribute.key = key;
+ attribute.value.type = DVR_SURFACE_ATTRIBUTE_TYPE_NONE;
+ return attribute;
+}
+
Status<UniqueDvrSurface> CreateApplicationSurface(bool visible = false,
int32_t z_order = 0) {
DvrSurface* surface = nullptr;
@@ -101,13 +108,14 @@
return {};
}
- Status<void> WaitForUpdate() {
+ enum : int { kTimeoutMs = 10000 }; // 10s
+
+ Status<void> WaitForUpdate(int timeout_ms = kTimeoutMs) {
if (display_manager_event_fd_ < 0)
return ErrorStatus(-display_manager_event_fd_);
- const int kTimeoutMs = 10000; // 10s
pollfd pfd = {display_manager_event_fd_, POLLIN, 0};
- const int count = poll(&pfd, 1, kTimeoutMs);
+ const int count = poll(&pfd, 1, timeout_ms);
if (count < 0)
return ErrorStatus(errno);
else if (count == 0)
@@ -471,19 +479,118 @@
auto attributes = attribute_status.take();
EXPECT_GE(attributes.size(), 2u);
- const std::set<int32_t> expected_keys = {DVR_SURFACE_ATTRIBUTE_Z_ORDER,
- DVR_SURFACE_ATTRIBUTE_VISIBLE};
+ std::set<int32_t> actual_keys;
+ std::set<int32_t> expected_keys = {DVR_SURFACE_ATTRIBUTE_Z_ORDER,
+ DVR_SURFACE_ATTRIBUTE_VISIBLE};
// Collect all the keys in attributes that match the expected keys.
- std::set<int32_t> actual_keys;
- std::for_each(attributes.begin(), attributes.end(),
- [&expected_keys, &actual_keys](const auto& attribute) {
- if (expected_keys.find(attribute.key) != expected_keys.end())
- actual_keys.emplace(attribute.key);
- });
+ auto compare_keys = [](const auto& attributes, const auto& expected_keys) {
+ std::set<int32_t> keys;
+ for (const auto& attribute : attributes) {
+ if (expected_keys.find(attribute.key) != expected_keys.end())
+ keys.emplace(attribute.key);
+ }
+ return keys;
+ };
// If the sets match then attributes contained at least the expected keys,
// even if other keys were also present.
+ actual_keys = compare_keys(attributes, expected_keys);
+ EXPECT_EQ(expected_keys, actual_keys);
+
+ std::vector<DvrSurfaceAttribute> attributes_to_set = {
+ GetAttribute(DVR_SURFACE_ATTRIBUTE_Z_ORDER, 0)};
+
+ // Test invalid args.
+ EXPECT_EQ(-EINVAL, dvrSurfaceSetAttributes(nullptr, attributes_to_set.data(),
+ attributes_to_set.size()));
+ EXPECT_EQ(-EINVAL, dvrSurfaceSetAttributes(surface.get(), nullptr,
+ attributes_to_set.size()));
+
+ // Test attribute change events.
+ ASSERT_EQ(0, dvrSurfaceSetAttributes(surface.get(), attributes_to_set.data(),
+ attributes_to_set.size()));
+ ASSERT_STATUS_OK(manager_->WaitForUpdate());
+
+ // Verify the attributes changed flag is set.
+ auto check_flags = [](const auto& value) {
+ return value & DVR_SURFACE_UPDATE_FLAGS_ATTRIBUTES_CHANGED;
+ };
+ EXPECT_STATUS_PRED(check_flags, manager_->GetUpdateFlags(0));
+
+ attribute_status = manager_->GetAttributes(0);
+ ASSERT_STATUS_OK(attribute_status);
+ attributes = attribute_status.take();
+ EXPECT_GE(attributes.size(), 2u);
+
+ expected_keys = {DVR_SURFACE_ATTRIBUTE_Z_ORDER,
+ DVR_SURFACE_ATTRIBUTE_VISIBLE};
+
+ actual_keys.clear();
+ actual_keys = compare_keys(attributes, expected_keys);
+ EXPECT_EQ(expected_keys, actual_keys);
+
+ // Test setting and then deleting an attribute.
+ const DvrSurfaceAttributeKey kUserKey = 1;
+ attributes_to_set = {GetAttribute(kUserKey, 1024)};
+
+ ASSERT_EQ(0, dvrSurfaceSetAttributes(surface.get(), attributes_to_set.data(),
+ attributes_to_set.size()));
+ ASSERT_STATUS_OK(manager_->WaitForUpdate());
+ EXPECT_STATUS_PRED(check_flags, manager_->GetUpdateFlags(0));
+
+ attribute_status = manager_->GetAttributes(0);
+ ASSERT_STATUS_OK(attribute_status);
+ attributes = attribute_status.take();
+ EXPECT_GE(attributes.size(), 2u);
+
+ expected_keys = {DVR_SURFACE_ATTRIBUTE_Z_ORDER, DVR_SURFACE_ATTRIBUTE_VISIBLE,
+ kUserKey};
+
+ actual_keys.clear();
+ actual_keys = compare_keys(attributes, expected_keys);
+ EXPECT_EQ(expected_keys, actual_keys);
+
+ // Delete the attribute.
+ attributes_to_set = {GetAttribute(kUserKey, nullptr)};
+
+ ASSERT_EQ(0, dvrSurfaceSetAttributes(surface.get(), attributes_to_set.data(),
+ attributes_to_set.size()));
+ ASSERT_STATUS_OK(manager_->WaitForUpdate());
+ EXPECT_STATUS_PRED(check_flags, manager_->GetUpdateFlags(0));
+
+ attribute_status = manager_->GetAttributes(0);
+ ASSERT_STATUS_OK(attribute_status);
+ attributes = attribute_status.take();
+ EXPECT_GE(attributes.size(), 2u);
+
+ expected_keys = {DVR_SURFACE_ATTRIBUTE_Z_ORDER, DVR_SURFACE_ATTRIBUTE_VISIBLE,
+ kUserKey};
+
+ actual_keys.clear();
+ actual_keys = compare_keys(attributes, expected_keys);
+ EXPECT_NE(expected_keys, actual_keys);
+
+ // Test deleting a reserved attribute.
+ attributes_to_set = {GetAttribute(DVR_SURFACE_ATTRIBUTE_VISIBLE, nullptr)};
+
+ EXPECT_EQ(0, dvrSurfaceSetAttributes(surface.get(), attributes_to_set.data(),
+ attributes_to_set.size()));
+
+ // Failed attribute operations should not trigger update events.
+ const int kTimeoutMs = 100; // 0.1s
+ EXPECT_STATUS_ERROR_VALUE(ETIMEDOUT, manager_->WaitForUpdate(kTimeoutMs));
+
+ attribute_status = manager_->GetAttributes(0);
+ ASSERT_STATUS_OK(attribute_status);
+ attributes = attribute_status.take();
+ EXPECT_GE(attributes.size(), 2u);
+
+ expected_keys = {DVR_SURFACE_ATTRIBUTE_Z_ORDER,
+ DVR_SURFACE_ATTRIBUTE_VISIBLE};
+
+ actual_keys.clear();
+ actual_keys = compare_keys(attributes, expected_keys);
EXPECT_EQ(expected_keys, actual_keys);
}
diff --git a/libs/vr/libvrflinger/display_surface.cpp b/libs/vr/libvrflinger/display_surface.cpp
index 330b455..6e18781 100644
--- a/libs/vr/libvrflinger/display_surface.cpp
+++ b/libs/vr/libvrflinger/display_surface.cpp
@@ -68,7 +68,7 @@
display::SurfaceUpdateFlags update_flags;
for (const auto& attribute : attributes) {
- const auto& key = attribute.first;
+ const auto key = attribute.first;
const auto* variant = &attribute.second;
bool invalid_value = false;
bool visibility_changed = false;
@@ -95,14 +95,23 @@
break;
}
+ // Only update the attribute map with valid values. This check also has the
+ // effect of preventing special attributes handled above from being deleted
+ // by an empty value.
if (invalid_value) {
ALOGW(
"DisplaySurface::OnClientSetAttributes: Failed to set display "
"surface attribute '%d' because of incompatible type: %d",
key, variant->index());
} else {
- // Only update the attribute map with valid values.
- attributes_[attribute.first] = attribute.second;
+ // An empty value indicates the attribute should be deleted.
+ if (variant->empty()) {
+ auto search = attributes_.find(key);
+ if (search != attributes_.end())
+ attributes_.erase(search);
+ } else {
+ attributes_[key] = *variant;
+ }
// All attribute changes generate a notification, even if the value
// doesn't change. Visibility attributes set a flag only if the value