SF: Remove *DisplayId::tryCast usage from ScreenCaptureOutput
Work towards DisplayId opaqueness by eliminating call-sites to APIs that
parse the display ID values directly. One such site is
ScreenCaptureOutput.
Replace all calls to *DisplayId::tryCast with local calls to cached
display variant.
Flag: com.android.graphics.surfaceflinger.flags.stable_edid_ids
Bug: 390690584
Test: libcompositionengine_test && libsurfaceflinger_unittest
Change-Id: Ic83a2b2bfa6fc98b1d0fccc60f450d1587c2cc34
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index 02522a3..b4667c9 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -23,6 +23,8 @@
#include <android-base/thread_annotations.h>
#include <android/native_window.h>
#include <binder/IBinder.h>
+#include <compositionengine/Display.h>
+#include <compositionengine/DisplaySurface.h>
#include <gui/LayerState.h>
#include <math/mat4.h>
#include <renderengine/RenderEngine.h>
@@ -61,11 +63,6 @@
struct CompositionInfo;
struct DisplayDeviceCreationArgs;
-namespace compositionengine {
-class Display;
-class DisplaySurface;
-} // namespace compositionengine
-
namespace display {
class DisplaySnapshot;
} // namespace display
@@ -123,6 +120,12 @@
DisplayId getId() const;
+ DisplayIdVariant getDisplayIdVariant() const {
+ const auto idVariant = mCompositionDisplay->getDisplayIdVariant();
+ LOG_FATAL_IF(!idVariant);
+ return *idVariant;
+ }
+
// Shorthand to upcast the ID of a display whose type is known as a precondition.
PhysicalDisplayId getPhysicalId() const {
const auto id = PhysicalDisplayId::tryCast(getId());
diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp
index 514adac..615492a 100644
--- a/services/surfaceflinger/RegionSamplingThread.cpp
+++ b/services/surfaceflinger/RegionSamplingThread.cpp
@@ -348,7 +348,7 @@
SurfaceFlinger::ScreenshotArgs screenshotArgs;
screenshotArgs.captureTypeVariant = displayWeak;
- screenshotArgs.displayId = std::nullopt;
+ screenshotArgs.displayIdVariant = std::nullopt;
screenshotArgs.sourceCrop = sampledBounds.isEmpty() ? layerStackSpaceRect : sampledBounds;
screenshotArgs.reqSize = sampledBounds.getSize();
screenshotArgs.dataspace = ui::Dataspace::V0_SRGB;
diff --git a/services/surfaceflinger/ScreenCaptureOutput.cpp b/services/surfaceflinger/ScreenCaptureOutput.cpp
index af6d4d3..2906bbd 100644
--- a/services/surfaceflinger/ScreenCaptureOutput.cpp
+++ b/services/surfaceflinger/ScreenCaptureOutput.cpp
@@ -30,11 +30,12 @@
std::shared_ptr<ScreenCaptureOutput> createScreenCaptureOutput(ScreenCaptureOutputArgs args) {
std::shared_ptr<ScreenCaptureOutput> output = compositionengine::impl::createOutputTemplated<
ScreenCaptureOutput, compositionengine::CompositionEngine,
- /* sourceCrop */ const Rect, std::optional<DisplayId>,
+ /* sourceCrop */ const Rect, ftl::Optional<DisplayIdVariant>,
const compositionengine::Output::ColorProfile&,
/* layerAlpha */ float,
- /* regionSampling */ bool>(args.compositionEngine, args.sourceCrop, args.displayId,
- args.colorProfile, args.layerAlpha, args.regionSampling,
+ /* regionSampling */ bool>(args.compositionEngine, args.sourceCrop,
+ args.displayIdVariant, args.colorProfile, args.layerAlpha,
+ args.regionSampling,
args.dimInGammaSpaceForEnhancedScreenshots,
args.enableLocalTonemapping);
output->editState().isSecure = args.isSecure;
@@ -59,8 +60,8 @@
{
std::string name = args.regionSampling ? "RegionSampling" : "ScreenCaptureOutput";
- if (args.displayId) {
- base::StringAppendF(&name, " for %" PRIu64, args.displayId.value().value);
+ if (const auto id = args.displayIdVariant.and_then(asDisplayIdOfType<DisplayId>)) {
+ base::StringAppendF(&name, " for %" PRIu64, id->value);
}
output->setName(name);
}
@@ -68,12 +69,12 @@
}
ScreenCaptureOutput::ScreenCaptureOutput(
- const Rect sourceCrop, std::optional<DisplayId> displayId,
+ const Rect sourceCrop, ftl::Optional<DisplayIdVariant> displayIdVariant,
const compositionengine::Output::ColorProfile& colorProfile, float layerAlpha,
bool regionSampling, bool dimInGammaSpaceForEnhancedScreenshots,
bool enableLocalTonemapping)
: mSourceCrop(sourceCrop),
- mDisplayId(displayId),
+ mDisplayIdVariant(displayIdVariant),
mColorProfile(colorProfile),
mLayerAlpha(layerAlpha),
mRegionSampling(regionSampling),
@@ -137,12 +138,9 @@
}
std::vector<aidl::android::hardware::graphics::composer3::Luts> luts;
- if (mDisplayId) {
- const auto id = PhysicalDisplayId::tryCast(mDisplayId.value());
- if (id) {
- auto& hwc = getCompositionEngine().getHwComposer();
- hwc.getLuts(*id, buffers, &luts);
- }
+ if (const auto physicalDisplayId = mDisplayIdVariant.and_then(asPhysicalDisplayId)) {
+ auto& hwc = getCompositionEngine().getHwComposer();
+ hwc.getLuts(*physicalDisplayId, buffers, &luts);
}
if (buffers.size() == luts.size()) {
diff --git a/services/surfaceflinger/ScreenCaptureOutput.h b/services/surfaceflinger/ScreenCaptureOutput.h
index b3e98b1..d4e20fc 100644
--- a/services/surfaceflinger/ScreenCaptureOutput.h
+++ b/services/surfaceflinger/ScreenCaptureOutput.h
@@ -30,7 +30,7 @@
ui::LayerStack layerStack;
Rect sourceCrop;
std::shared_ptr<renderengine::ExternalTexture> buffer;
- std::optional<DisplayId> displayId;
+ ftl::Optional<DisplayIdVariant> displayIdVariant;
ui::Size reqBufferSize;
float sdrWhitePointNits;
float displayBrightnessNits;
@@ -51,7 +51,7 @@
// SurfaceFlinger::captureLayers and SurfaceFlinger::captureDisplay.
class ScreenCaptureOutput : public compositionengine::impl::Output {
public:
- ScreenCaptureOutput(const Rect sourceCrop, std::optional<DisplayId> displayId,
+ ScreenCaptureOutput(const Rect sourceCrop, ftl::Optional<DisplayIdVariant> displayIdVariant,
const compositionengine::Output::ColorProfile& colorProfile,
float layerAlpha, bool regionSampling,
bool dimInGammaSpaceForEnhancedScreenshots, bool enableLocalTonemapping);
@@ -70,7 +70,7 @@
private:
std::unordered_map<int32_t, aidl::android::hardware::graphics::composer3::Luts> generateLuts();
const Rect mSourceCrop;
- const std::optional<DisplayId> mDisplayId;
+ const ftl::Optional<DisplayIdVariant> mDisplayIdVariant;
const compositionengine::Output::ColorProfile& mColorProfile;
const float mLayerAlpha;
const bool mRegionSampling;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index d6b51ae..3f063c9 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -7340,7 +7340,7 @@
}
wp<const DisplayDevice> displayWeak;
- DisplayId displayId;
+ ftl::Optional<DisplayIdVariant> displayIdVariantOpt;
ui::LayerStack layerStack;
ui::Size reqSize(args.width, args.height);
std::unordered_set<uint32_t> excludeLayerIds;
@@ -7356,7 +7356,7 @@
return;
}
displayWeak = display;
- displayId = display->getId();
+ displayIdVariantOpt = display->getDisplayIdVariant();
layerStack = display->getLayerStack();
displayIsSecure = display->isSecure();
@@ -7384,7 +7384,7 @@
ScreenshotArgs screenshotArgs;
screenshotArgs.captureTypeVariant = displayWeak;
- screenshotArgs.displayId = displayId;
+ screenshotArgs.displayIdVariant = displayIdVariantOpt;
screenshotArgs.sourceCrop = gui::aidl_utils::fromARect(captureArgs.sourceCrop);
if (screenshotArgs.sourceCrop.isEmpty()) {
screenshotArgs.sourceCrop = layerStackSpaceRect;
@@ -7403,6 +7403,7 @@
const sp<IScreenCaptureListener>& captureListener) {
ui::LayerStack layerStack;
wp<const DisplayDevice> displayWeak;
+ ftl::Optional<DisplayIdVariant> displayIdVariantOpt;
ui::Size size;
Rect layerStackSpaceRect;
bool displayIsSecure;
@@ -7418,6 +7419,7 @@
}
displayWeak = display;
+ displayIdVariantOpt = display->getDisplayIdVariant();
layerStack = display->getLayerStack();
layerStackSpaceRect = display->getLayerStackSpaceRect();
size = display->getLayerStackSpaceRect().getSize();
@@ -7452,7 +7454,7 @@
ScreenshotArgs screenshotArgs;
screenshotArgs.captureTypeVariant = displayWeak;
- screenshotArgs.displayId = displayId;
+ screenshotArgs.displayIdVariant = displayIdVariantOpt;
screenshotArgs.sourceCrop = layerStackSpaceRect;
screenshotArgs.reqSize = size;
screenshotArgs.dataspace = static_cast<ui::Dataspace>(args.dataspace);
@@ -7944,7 +7946,7 @@
.layerStack = layerStack,
.sourceCrop = args.sourceCrop,
.buffer = std::move(buffer),
- .displayId = args.displayId,
+ .displayIdVariant = args.displayIdVariant,
.reqBufferSize = args.reqSize,
.sdrWhitePointNits = args.sdrWhitePointNits,
.displayBrightnessNits = args.displayBrightnessNits,
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 1114aff..278aa6f 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -876,7 +876,7 @@
std::variant<int32_t, wp<const DisplayDevice>> captureTypeVariant;
// Display ID of the display the result will be on
- std::optional<DisplayId> displayId{std::nullopt};
+ ftl::Optional<DisplayIdVariant> displayIdVariant{std::nullopt};
// If true, transform is inverted from the parent layer snapshot
bool childrenOnly{false};
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 42e7ab9..4f86d86 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -481,7 +481,7 @@
SurfaceFlinger::ScreenshotArgs screenshotArgs;
screenshotArgs.captureTypeVariant = display;
- screenshotArgs.displayId = std::nullopt;
+ screenshotArgs.displayIdVariant = std::nullopt;
screenshotArgs.sourceCrop = sourceCrop;
screenshotArgs.reqSize = sourceCrop.getSize();
screenshotArgs.dataspace = dataspace;