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

Author: stagadish

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 @@ class SurfaceFlinger;
 struct CompositionInfo;
 struct DisplayDeviceCreationArgs;
 
-namespace compositionengine {
-class Display;
-class DisplaySurface;
-} // namespace compositionengine
-
 namespace display {
 class DisplaySnapshot;
 } // namespace display
@@ -123,6 +120,12 @@ class DisplayDevice : public RefBase {
 
     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());

services/surfaceflinger/RegionSamplingThread.cpp

@@ -348,7 +348,7 @@ void RegionSamplingThread::captureSample() {
 
     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;

services/surfaceflinger/ScreenCaptureOutput.cpp

@@ -30,11 +30,12 @@ namespace android {
 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,21 +60,21 @@ std::shared_ptr<ScreenCaptureOutput> createScreenCaptureOutput(ScreenCaptureOutp
 
     {
         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);
     }
     return output;
 }
 
 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 @@ ScreenCaptureOutput::generateLuts() {
         }
 
         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()) {

services/surfaceflinger/ScreenCaptureOutput.h

@@ -30,7 +30,7 @@ struct ScreenCaptureOutputArgs {
     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 @@ struct ScreenCaptureOutputArgs {
 // 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 @@ class ScreenCaptureOutput : public compositionengine::impl::Output {
 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;

services/surfaceflinger/SurfaceFlinger.cpp

@@ -7340,7 +7340,7 @@ void SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args,
     }
 
     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 @@ void SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args,
             return;
         }
         displayWeak = display;
-        displayId = display->getId();
+        displayIdVariantOpt = display->getDisplayIdVariant();
         layerStack = display->getLayerStack();
         displayIsSecure = display->isSecure();
 
@@ -7384,7 +7384,7 @@ void SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args,
 
     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 @@ void SurfaceFlinger::captureDisplay(DisplayId displayId, const CaptureArgs& args
                                     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 @@ void SurfaceFlinger::captureDisplay(DisplayId displayId, const CaptureArgs& args
         }
 
         displayWeak = display;
+        displayIdVariantOpt = display->getDisplayIdVariant();
         layerStack = display->getLayerStack();
         layerStackSpaceRect = display->getLayerStackSpaceRect();
         size = display->getLayerStackSpaceRect().getSize();
@@ -7452,7 +7454,7 @@ void SurfaceFlinger::captureDisplay(DisplayId displayId, const CaptureArgs& args
 
     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 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl(
                                         .layerStack = layerStack,
                                         .sourceCrop = args.sourceCrop,
                                         .buffer = std::move(buffer),
-                                        .displayId = args.displayId,
+                                        .displayIdVariant = args.displayIdVariant,
                                         .reqBufferSize = args.reqSize,
                                         .sdrWhitePointNits = args.sdrWhitePointNits,
                                         .displayBrightnessNits = args.displayBrightnessNits,

services/surfaceflinger/SurfaceFlinger.h

@@ -876,7 +876,7 @@ class SurfaceFlinger : public BnSurfaceComposer,
         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};

services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h

@@ -481,7 +481,7 @@ class TestableSurfaceFlinger {
 
         SurfaceFlinger::ScreenshotArgs screenshotArgs;
         screenshotArgs.captureTypeVariant = display;
-        screenshotArgs.displayId = std::nullopt;
+        screenshotArgs.displayIdVariant = std::nullopt;
         screenshotArgs.sourceCrop = sourceCrop;
         screenshotArgs.reqSize = sourceCrop.getSize();
         screenshotArgs.dataspace = dataspace;