Merge "Replace use of deprecated function with_min_level" into main
diff --git a/include/ftl/expected.h b/include/ftl/expected.h
new file mode 100644
index 0000000..12b6102
--- /dev/null
+++ b/include/ftl/expected.h
@@ -0,0 +1,65 @@
+/*
+ * Copyright 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <android-base/expected.h>
+#include <ftl/optional.h>
+
+#include <utility>
+
+namespace android::ftl {
+
+// Superset of base::expected<T, E> with monadic operations.
+//
+// TODO: Extend std::expected<T, E> in C++23.
+//
+template <typename T, typename E>
+struct Expected final : base::expected<T, E> {
+ using Base = base::expected<T, E>;
+ using Base::expected;
+
+ using Base::error;
+ using Base::has_value;
+ using Base::value;
+
+ template <typename P>
+ constexpr bool has_error(P predicate) const {
+ return !has_value() && predicate(error());
+ }
+
+ constexpr Optional<T> value_opt() const& {
+ return has_value() ? Optional(value()) : std::nullopt;
+ }
+
+ constexpr Optional<T> value_opt() && {
+ return has_value() ? Optional(std::move(value())) : std::nullopt;
+ }
+
+ // Delete new for this class. Its base doesn't have a virtual destructor, and
+ // if it got deleted via base class pointer, it would cause undefined
+ // behavior. There's not a good reason to allocate this object on the heap
+ // anyway.
+ static void* operator new(size_t) = delete;
+ static void* operator new[](size_t) = delete;
+};
+
+template <typename E>
+constexpr auto Unexpected(E&& error) {
+ return base::unexpected(std::forward<E>(error));
+}
+
+} // namespace android::ftl
diff --git a/libs/ftl/Android.bp b/libs/ftl/Android.bp
index 918680d..5ac965f 100644
--- a/libs/ftl/Android.bp
+++ b/libs/ftl/Android.bp
@@ -10,11 +10,15 @@
cc_test {
name: "ftl_test",
test_suites: ["device-tests"],
+ header_libs: [
+ "libbase_headers",
+ ],
srcs: [
"algorithm_test.cpp",
"cast_test.cpp",
"concat_test.cpp",
"enum_test.cpp",
+ "expected_test.cpp",
"fake_guard_test.cpp",
"flags_test.cpp",
"function_test.cpp",
diff --git a/libs/ftl/expected_test.cpp b/libs/ftl/expected_test.cpp
new file mode 100644
index 0000000..8cb07e4
--- /dev/null
+++ b/libs/ftl/expected_test.cpp
@@ -0,0 +1,77 @@
+/*
+ * Copyright 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <ftl/expected.h>
+#include <gtest/gtest.h>
+
+#include <string>
+#include <system_error>
+
+namespace android::test {
+
+using IntExp = ftl::Expected<int, std::errc>;
+using StringExp = ftl::Expected<std::string, std::errc>;
+
+using namespace std::string_literals;
+
+TEST(Expected, Construct) {
+ // Default value.
+ EXPECT_TRUE(IntExp().has_value());
+ EXPECT_EQ(IntExp(), IntExp(0));
+
+ EXPECT_TRUE(StringExp().has_value());
+ EXPECT_EQ(StringExp(), StringExp(""));
+
+ // Value.
+ ASSERT_TRUE(IntExp(42).has_value());
+ EXPECT_EQ(42, IntExp(42).value());
+
+ ASSERT_TRUE(StringExp("test").has_value());
+ EXPECT_EQ("test"s, StringExp("test").value());
+
+ // Error.
+ const auto exp = StringExp(ftl::Unexpected(std::errc::invalid_argument));
+ ASSERT_FALSE(exp.has_value());
+ EXPECT_EQ(std::errc::invalid_argument, exp.error());
+}
+
+TEST(Expected, HasError) {
+ EXPECT_FALSE(IntExp(123).has_error([](auto) { return true; }));
+ EXPECT_FALSE(IntExp(ftl::Unexpected(std::errc::io_error)).has_error([](auto) { return false; }));
+
+ EXPECT_TRUE(StringExp(ftl::Unexpected(std::errc::permission_denied)).has_error([](auto e) {
+ return e == std::errc::permission_denied;
+ }));
+}
+
+TEST(Expected, ValueOpt) {
+ EXPECT_EQ(ftl::Optional(-1), IntExp(-1).value_opt());
+ EXPECT_EQ(std::nullopt, IntExp(ftl::Unexpected(std::errc::broken_pipe)).value_opt());
+
+ {
+ const StringExp exp("foo"s);
+ EXPECT_EQ(ftl::Optional('f'),
+ exp.value_opt().transform([](const auto& s) { return s.front(); }));
+ EXPECT_EQ("foo"s, exp.value());
+ }
+ {
+ StringExp exp("foobar"s);
+ EXPECT_EQ(ftl::Optional(6), std::move(exp).value_opt().transform(&std::string::length));
+ EXPECT_TRUE(exp.value().empty());
+ }
+}
+
+} // namespace android::test
diff --git a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
index 9e35717..575a30e 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
@@ -97,13 +97,13 @@
MOCK_CONST_METHOD1(isConnected, bool(PhysicalDisplayId));
MOCK_CONST_METHOD2(getModes,
std::vector<HWComposer::HWCDisplayMode>(PhysicalDisplayId, int32_t));
- MOCK_CONST_METHOD1(getActiveMode, std::optional<hal::HWConfigId>(PhysicalDisplayId));
+ MOCK_CONST_METHOD1(getActiveMode, ftl::Expected<hal::HWConfigId, status_t>(PhysicalDisplayId));
MOCK_CONST_METHOD1(getColorModes, std::vector<ui::ColorMode>(PhysicalDisplayId));
MOCK_METHOD3(setActiveColorMode, status_t(PhysicalDisplayId, ui::ColorMode, ui::RenderIntent));
MOCK_CONST_METHOD0(isUsingVrComposer, bool());
MOCK_CONST_METHOD1(getDisplayConnectionType, ui::DisplayConnectionType(PhysicalDisplayId));
MOCK_CONST_METHOD1(isVsyncPeriodSwitchSupported, bool(PhysicalDisplayId));
- MOCK_CONST_METHOD2(getDisplayVsyncPeriod, status_t(PhysicalDisplayId, nsecs_t*));
+ MOCK_CONST_METHOD1(getDisplayVsyncPeriod, ftl::Expected<nsecs_t, status_t>(PhysicalDisplayId));
MOCK_METHOD4(setActiveModeWithConstraints,
status_t(PhysicalDisplayId, hal::HWConfigId,
const hal::VsyncPeriodChangeConstraints&,
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 4f81482..5f20cd9 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -239,10 +239,8 @@
return 0;
}
- nsecs_t vsyncPeriod;
- const auto status = mHwComposer.getDisplayVsyncPeriod(physicalId, &vsyncPeriod);
- if (status == NO_ERROR) {
- return vsyncPeriod;
+ if (const auto vsyncPeriodOpt = mHwComposer.getDisplayVsyncPeriod(physicalId).value_opt()) {
+ return *vsyncPeriodOpt;
}
return refreshRateSelector().getActiveMode().modePtr->getVsyncRate().getPeriodNsecs();
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index cf1c3c1..bac24c7 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -343,14 +343,18 @@
return modes;
}
-std::optional<hal::HWConfigId> HWComposer::getActiveMode(PhysicalDisplayId displayId) const {
- RETURN_IF_INVALID_DISPLAY(displayId, std::nullopt);
+ftl::Expected<hal::HWConfigId, status_t> HWComposer::getActiveMode(
+ PhysicalDisplayId displayId) const {
+ RETURN_IF_INVALID_DISPLAY(displayId, ftl::Unexpected(BAD_INDEX));
const auto hwcId = *fromPhysicalDisplayId(displayId);
hal::HWConfigId configId;
const auto error = static_cast<hal::Error>(mComposer->getActiveConfig(hwcId, &configId));
+ if (error == hal::Error::BAD_CONFIG) {
+ return ftl::Unexpected(NO_INIT);
+ }
- RETURN_IF_HWC_ERROR_FOR("getActiveConfig", error, displayId, std::nullopt);
+ RETURN_IF_HWC_ERROR_FOR("getActiveConfig", error, displayId, ftl::Unexpected(UNKNOWN_ERROR));
return configId;
}
@@ -376,20 +380,20 @@
return mDisplayData.at(displayId).hwcDisplay->isVsyncPeriodSwitchSupported();
}
-status_t HWComposer::getDisplayVsyncPeriod(PhysicalDisplayId displayId,
- nsecs_t* outVsyncPeriod) const {
- RETURN_IF_INVALID_DISPLAY(displayId, 0);
+ftl::Expected<nsecs_t, status_t> HWComposer::getDisplayVsyncPeriod(
+ PhysicalDisplayId displayId) const {
+ RETURN_IF_INVALID_DISPLAY(displayId, ftl::Unexpected(BAD_INDEX));
if (!isVsyncPeriodSwitchSupported(displayId)) {
- return INVALID_OPERATION;
+ return ftl::Unexpected(INVALID_OPERATION);
}
+
const auto hwcId = *fromPhysicalDisplayId(displayId);
Hwc2::VsyncPeriodNanos vsyncPeriodNanos = 0;
- auto error =
+ const auto error =
static_cast<hal::Error>(mComposer->getDisplayVsyncPeriod(hwcId, &vsyncPeriodNanos));
- RETURN_IF_HWC_ERROR(error, displayId, 0);
- *outVsyncPeriod = static_cast<nsecs_t>(vsyncPeriodNanos);
- return NO_ERROR;
+ RETURN_IF_HWC_ERROR(error, displayId, ftl::Unexpected(UNKNOWN_ERROR));
+ return static_cast<nsecs_t>(vsyncPeriodNanos);
}
std::vector<ui::ColorMode> HWComposer::getColorModes(PhysicalDisplayId displayId) const {
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index fb32ff4..7fbbb1a 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -25,6 +25,7 @@
#include <vector>
#include <android-base/thread_annotations.h>
+#include <ftl/expected.h>
#include <ftl/future.h>
#include <ui/DisplayIdentification.h>
#include <ui/FenceTime.h>
@@ -237,7 +238,7 @@
virtual std::vector<HWCDisplayMode> getModes(PhysicalDisplayId,
int32_t maxFrameIntervalNs) const = 0;
- virtual std::optional<hal::HWConfigId> getActiveMode(PhysicalDisplayId) const = 0;
+ virtual ftl::Expected<hal::HWConfigId, status_t> getActiveMode(PhysicalDisplayId) const = 0;
virtual std::vector<ui::ColorMode> getColorModes(PhysicalDisplayId) const = 0;
@@ -247,8 +248,7 @@
// Composer 2.4
virtual ui::DisplayConnectionType getDisplayConnectionType(PhysicalDisplayId) const = 0;
virtual bool isVsyncPeriodSwitchSupported(PhysicalDisplayId) const = 0;
- virtual status_t getDisplayVsyncPeriod(PhysicalDisplayId displayId,
- nsecs_t* outVsyncPeriod) const = 0;
+ virtual ftl::Expected<nsecs_t, status_t> getDisplayVsyncPeriod(PhysicalDisplayId) const = 0;
virtual status_t setActiveModeWithConstraints(PhysicalDisplayId, hal::HWConfigId,
const hal::VsyncPeriodChangeConstraints&,
hal::VsyncPeriodChangeTimeline* outTimeline) = 0;
@@ -424,7 +424,7 @@
std::vector<HWCDisplayMode> getModes(PhysicalDisplayId,
int32_t maxFrameIntervalNs) const override;
- std::optional<hal::HWConfigId> getActiveMode(PhysicalDisplayId) const override;
+ ftl::Expected<hal::HWConfigId, status_t> getActiveMode(PhysicalDisplayId) const override;
std::vector<ui::ColorMode> getColorModes(PhysicalDisplayId) const override;
@@ -435,8 +435,7 @@
// Composer 2.4
ui::DisplayConnectionType getDisplayConnectionType(PhysicalDisplayId) const override;
bool isVsyncPeriodSwitchSupported(PhysicalDisplayId) const override;
- status_t getDisplayVsyncPeriod(PhysicalDisplayId displayId,
- nsecs_t* outVsyncPeriod) const override;
+ ftl::Expected<nsecs_t, status_t> getDisplayVsyncPeriod(PhysicalDisplayId) const override;
status_t setActiveModeWithConstraints(PhysicalDisplayId, hal::HWConfigId,
const hal::VsyncPeriodChangeConstraints&,
hal::VsyncPeriodChangeTimeline* outTimeline) override;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 776eccb..d86de84 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3293,7 +3293,7 @@
std::pair<DisplayModes, DisplayModePtr> SurfaceFlinger::loadDisplayModes(
PhysicalDisplayId displayId) const {
std::vector<HWComposer::HWCDisplayMode> hwcModes;
- std::optional<hal::HWDisplayId> activeModeHwcId;
+ std::optional<hal::HWConfigId> activeModeHwcIdOpt;
int attempt = 0;
constexpr int kMaxAttempts = 3;
@@ -3301,10 +3301,10 @@
hwcModes = getHwComposer().getModes(displayId,
scheduler::RefreshRateSelector::kMinSupportedFrameRate
.getPeriodNsecs());
- activeModeHwcId = getHwComposer().getActiveMode(displayId);
+ activeModeHwcIdOpt = getHwComposer().getActiveMode(displayId).value_opt();
- const auto isActiveMode = [activeModeHwcId](const HWComposer::HWCDisplayMode& mode) {
- return mode.hwcId == activeModeHwcId;
+ const auto isActiveMode = [activeModeHwcIdOpt](const HWComposer::HWCDisplayMode& mode) {
+ return mode.hwcId == activeModeHwcIdOpt;
};
if (std::any_of(hwcModes.begin(), hwcModes.end(), isActiveMode)) {
@@ -3314,7 +3314,7 @@
if (attempt == kMaxAttempts) {
const std::string activeMode =
- activeModeHwcId ? std::to_string(*activeModeHwcId) : "unknown"s;
+ activeModeHwcIdOpt ? std::to_string(*activeModeHwcIdOpt) : "unknown"s;
ALOGE("HWC failed to report an active mode that is supported: activeModeHwcId=%s, "
"hwcModes={%s}",
activeMode.c_str(), base::Join(hwcModes, ", ").c_str());
@@ -3358,8 +3358,8 @@
// Keep IDs if modes have not changed.
const auto& modes = sameModes ? oldModes : newModes;
const DisplayModePtr activeMode =
- std::find_if(modes.begin(), modes.end(), [activeModeHwcId](const auto& pair) {
- return pair.second->getHwcId() == activeModeHwcId;
+ std::find_if(modes.begin(), modes.end(), [activeModeHwcIdOpt](const auto& pair) {
+ return pair.second->getHwcId() == activeModeHwcIdOpt;
})->second;
return {modes, activeMode};
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_displayhardware_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_displayhardware_fuzzer.cpp
index 68237c8..682079f 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_displayhardware_fuzzer.cpp
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_displayhardware_fuzzer.cpp
@@ -143,7 +143,6 @@
void invokeComposerHal2_2(Hwc2::AidlComposer*, Display, Hwc2::V2_4::hal::Layer);
void invokeComposerHal2_3(Hwc2::AidlComposer*, Display, Hwc2::V2_4::hal::Layer);
void invokeComposerHal2_4(Hwc2::AidlComposer*, Display, Hwc2::V2_4::hal::Layer);
- void getDisplayVsyncPeriod();
void setActiveModeWithConstraints();
void getDisplayIdentificationData();
void dumpHwc();
@@ -202,11 +201,6 @@
return display;
}
-void DisplayHardwareFuzzer::getDisplayVsyncPeriod() {
- nsecs_t outVsyncPeriod;
- mHwc.getDisplayVsyncPeriod(mPhysicalDisplayId, &outVsyncPeriod);
-}
-
void DisplayHardwareFuzzer::setActiveModeWithConstraints() {
hal::VsyncPeriodChangeTimeline outTimeline;
mHwc.setActiveModeWithConstraints(mPhysicalDisplayId, kActiveConfig, {} /*constraints*/,
@@ -617,8 +611,7 @@
mHwc.getDisplayConnectionType(mPhysicalDisplayId);
mHwc.isVsyncPeriodSwitchSupported(mPhysicalDisplayId);
-
- getDisplayVsyncPeriod();
+ mHwc.getDisplayVsyncPeriod(mPhysicalDisplayId);
setActiveModeWithConstraints();
diff --git a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
index a5c0657..a25804c 100644
--- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
@@ -104,7 +104,7 @@
TEST_F(HWComposerTest, getActiveMode) {
// Unknown display.
- EXPECT_EQ(mHwc.getActiveMode(PhysicalDisplayId::fromPort(0)), std::nullopt);
+ EXPECT_EQ(mHwc.getActiveMode(PhysicalDisplayId::fromPort(0)), ftl::Unexpected(BAD_INDEX));
constexpr hal::HWDisplayId kHwcDisplayId = 2;
expectHotplugConnect(kHwcDisplayId);
@@ -117,14 +117,20 @@
EXPECT_CALL(*mHal, getActiveConfig(kHwcDisplayId, _))
.WillOnce(Return(HalError::BAD_DISPLAY));
- EXPECT_EQ(mHwc.getActiveMode(info->id), std::nullopt);
+ EXPECT_EQ(mHwc.getActiveMode(info->id), ftl::Unexpected(UNKNOWN_ERROR));
+ }
+ {
+ EXPECT_CALL(*mHal, getActiveConfig(kHwcDisplayId, _))
+ .WillOnce(Return(HalError::BAD_CONFIG));
+
+ EXPECT_EQ(mHwc.getActiveMode(info->id), ftl::Unexpected(NO_INIT));
}
{
constexpr hal::HWConfigId kConfigId = 42;
EXPECT_CALL(*mHal, getActiveConfig(kHwcDisplayId, _))
.WillOnce(DoAll(SetArgPointee<1>(kConfigId), Return(HalError::NONE)));
- EXPECT_EQ(mHwc.getActiveMode(info->id), kConfigId);
+ EXPECT_EQ(mHwc.getActiveMode(info->id).value_opt(), kConfigId);
}
}