SF: Remove *DisplayId::tryCast usage from VirtualDisplaySurface
Work towards DisplayId opaqueness by eliminating call-sites to APIs that
parse the display ID values directly. One such site is
VirtualDisplaySurface.
Replace all calls to *DisplayId::tryCast with a VirtualDisplayIdVariant
guard.
Flag: com.android.graphics.surfaceflinger.flags.stable_edid_ids
Bug: 390690584
Test: libsurfaceflinger_unittest
Change-Id: I7ae9e838547c31ce09349e15f7d23e99f313646b
diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
index 384f7b2..56ed11f 100644
--- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
+++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
@@ -47,7 +47,8 @@
namespace android {
-VirtualDisplaySurface::VirtualDisplaySurface(HWComposer& hwc, VirtualDisplayId displayId,
+VirtualDisplaySurface::VirtualDisplaySurface(HWComposer& hwc,
+ VirtualDisplayIdVariant virtualIdVariant,
const sp<IGraphicBufferProducer>& sink,
const sp<IGraphicBufferProducer>& bqProducer,
const sp<IGraphicBufferConsumer>& bqConsumer,
@@ -58,7 +59,7 @@
: ConsumerBase(bqConsumer),
#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ)
mHwc(hwc),
- mDisplayId(displayId),
+ mVirtualIdVariant(virtualIdVariant),
mDisplayName(name),
mSource{},
mDefaultOutputFormat(HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED),
@@ -119,7 +120,7 @@
}
status_t VirtualDisplaySurface::beginFrame(bool mustRecompose) {
- if (GpuVirtualDisplayId::tryCast(mDisplayId)) {
+ if (isBackedByGpu()) {
return NO_ERROR;
}
@@ -133,7 +134,7 @@
}
status_t VirtualDisplaySurface::prepareFrame(CompositionType compositionType) {
- if (GpuVirtualDisplayId::tryCast(mDisplayId)) {
+ if (isBackedByGpu()) {
return NO_ERROR;
}
@@ -181,7 +182,10 @@
}
status_t VirtualDisplaySurface::advanceFrame(float hdrSdrRatio) {
- if (GpuVirtualDisplayId::tryCast(mDisplayId)) {
+ const auto halVirtualDisplayId = ftl::match(
+ mVirtualIdVariant, [](HalVirtualDisplayId id) { return ftl::Optional(id); },
+ [](auto) { return ftl::Optional<HalVirtualDisplayId>(); });
+ if (!halVirtualDisplayId) {
return NO_ERROR;
}
@@ -212,11 +216,8 @@
VDS_LOGV("%s: fb=%d(%p) out=%d(%p)", __func__, mFbProducerSlot, fbBuffer.get(),
mOutputProducerSlot, outBuffer.get());
- const auto halDisplayId = HalVirtualDisplayId::tryCast(mDisplayId);
- LOG_FATAL_IF(!halDisplayId);
- // At this point we know the output buffer acquire fence,
- // so update HWC state with it.
- mHwc.setOutputBuffer(*halDisplayId, mOutputFence, outBuffer);
+ // At this point we know the output buffer acquire fence, so update HWC state with it.
+ mHwc.setOutputBuffer(*halVirtualDisplayId, mOutputFence, outBuffer);
status_t result = NO_ERROR;
if (fbBuffer != nullptr) {
@@ -227,7 +228,7 @@
hwcBuffer = fbBuffer; // HWC hasn't previously seen this buffer in this slot
}
// TODO: Correctly propagate the dataspace from GL composition
- result = mHwc.setClientTarget(*halDisplayId, mFbProducerSlot, mFbFence, hwcBuffer,
+ result = mHwc.setClientTarget(*halVirtualDisplayId, mFbProducerSlot, mFbFence, hwcBuffer,
ui::Dataspace::UNKNOWN, hdrSdrRatio);
}
@@ -235,8 +236,8 @@
}
void VirtualDisplaySurface::onFrameCommitted() {
- const auto halDisplayId = HalVirtualDisplayId::tryCast(mDisplayId);
- if (!halDisplayId) {
+ const auto halDisplayId = asHalDisplayId(mVirtualIdVariant);
+ if (!halDisplayId.has_value()) {
return;
}
@@ -250,8 +251,7 @@
Mutex::Autolock lock(mMutex);
int sslot = mapProducer2SourceSlot(SOURCE_SCRATCH, mFbProducerSlot);
VDS_LOGV("%s: release scratch sslot=%d", __func__, sslot);
- addReleaseFenceLocked(sslot, mProducerBuffers[mFbProducerSlot],
- retireFence);
+ addReleaseFenceLocked(sslot, mProducerBuffers[mFbProducerSlot], retireFence);
releaseBufferLocked(sslot, mProducerBuffers[mFbProducerSlot]);
}
@@ -299,7 +299,7 @@
status_t VirtualDisplaySurface::requestBuffer(int pslot,
sp<GraphicBuffer>* outBuf) {
- if (GpuVirtualDisplayId::tryCast(mDisplayId)) {
+ if (isBackedByGpu()) {
return mSource[SOURCE_SINK]->requestBuffer(pslot, outBuf);
}
@@ -321,7 +321,7 @@
status_t VirtualDisplaySurface::dequeueBuffer(Source source,
PixelFormat format, uint64_t usage, int* sslot, sp<Fence>* fence) {
- LOG_ALWAYS_FATAL_IF(GpuVirtualDisplayId::tryCast(mDisplayId).has_value());
+ LOG_ALWAYS_FATAL_IF(isBackedByGpu());
status_t result =
mSource[source]->dequeueBuffer(sslot, fence, mSinkBufferWidth, mSinkBufferHeight,
@@ -373,7 +373,7 @@
PixelFormat format, uint64_t usage,
uint64_t* outBufferAge,
FrameEventHistoryDelta* outTimestamps) {
- if (GpuVirtualDisplayId::tryCast(mDisplayId)) {
+ if (isBackedByGpu()) {
return mSource[SOURCE_SINK]->dequeueBuffer(pslot, fence, w, h, format, usage, outBufferAge,
outTimestamps);
}
@@ -459,7 +459,7 @@
status_t VirtualDisplaySurface::queueBuffer(int pslot,
const QueueBufferInput& input, QueueBufferOutput* output) {
- if (GpuVirtualDisplayId::tryCast(mDisplayId)) {
+ if (isBackedByGpu()) {
return mSource[SOURCE_SINK]->queueBuffer(pslot, input, output);
}
@@ -517,7 +517,7 @@
status_t VirtualDisplaySurface::cancelBuffer(int pslot,
const sp<Fence>& fence) {
- if (GpuVirtualDisplayId::tryCast(mDisplayId)) {
+ if (isBackedByGpu()) {
return mSource[SOURCE_SINK]->cancelBuffer(mapProducer2SourceSlot(SOURCE_SINK, pslot), fence);
}
@@ -621,7 +621,10 @@
}
status_t VirtualDisplaySurface::refreshOutputBuffer() {
- LOG_ALWAYS_FATAL_IF(GpuVirtualDisplayId::tryCast(mDisplayId).has_value());
+ const auto halVirtualDisplayId = ftl::match(
+ mVirtualIdVariant, [](HalVirtualDisplayId id) { return ftl::Optional(id); },
+ [](auto) { return ftl::Optional<HalVirtualDisplayId>(); });
+ LOG_ALWAYS_FATAL_IF(!halVirtualDisplayId);
if (mOutputProducerSlot >= 0) {
mSource[SOURCE_SINK]->cancelBuffer(
@@ -640,14 +643,16 @@
// until after GPU calls queueBuffer(). So here we just set the buffer
// (for use in HWC prepare) but not the fence; we'll call this again with
// the proper fence once we have it.
- const auto halDisplayId = HalVirtualDisplayId::tryCast(mDisplayId);
- LOG_FATAL_IF(!halDisplayId);
- result = mHwc.setOutputBuffer(*halDisplayId, Fence::NO_FENCE,
+ result = mHwc.setOutputBuffer(*halVirtualDisplayId, Fence::NO_FENCE,
mProducerBuffers[mOutputProducerSlot]);
return result;
}
+bool VirtualDisplaySurface::isBackedByGpu() const {
+ return std::holds_alternative<GpuVirtualDisplayId>(mVirtualIdVariant);
+}
+
// This slot mapping function is its own inverse, so two copies are unnecessary.
// Both are kept to make the intent clear where the function is called, and for
// the (unlikely) chance that we switch to a different mapping function.
diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h
index 90426f7..cb65c79 100644
--- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h
+++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h
@@ -75,7 +75,8 @@
public BnGraphicBufferProducer,
private ConsumerBase {
public:
- VirtualDisplaySurface(HWComposer&, VirtualDisplayId, const sp<IGraphicBufferProducer>& sink,
+ VirtualDisplaySurface(HWComposer&, VirtualDisplayIdVariant,
+ const sp<IGraphicBufferProducer>& sink,
const sp<IGraphicBufferProducer>& bqProducer,
const sp<IGraphicBufferConsumer>& bqConsumer, const std::string& name);
@@ -145,6 +146,7 @@
void updateQueueBufferOutput(QueueBufferOutput&&);
void resetPerFrameState();
status_t refreshOutputBuffer();
+ bool isBackedByGpu() const;
// Both the sink and scratch buffer pools have their own set of slots
// ("source slots", or "sslot"). We have to merge these into the single
@@ -159,7 +161,7 @@
// Immutable after construction
//
HWComposer& mHwc;
- const VirtualDisplayId mDisplayId;
+ const VirtualDisplayIdVariant mVirtualIdVariant;
const std::string mDisplayName;
sp<IGraphicBufferProducer> mSource[2]; // indexed by SOURCE_*
uint32_t mDefaultOutputFormat;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index ab468c9..d6b51ae 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -659,15 +659,15 @@
}
}
-void SurfaceFlinger::acquireVirtualDisplay(ui::Size resolution, ui::PixelFormat format,
- const std::string& uniqueId,
- compositionengine::DisplayCreationArgsBuilder& builder) {
+std::optional<VirtualDisplayIdVariant> SurfaceFlinger::acquireVirtualDisplay(
+ ui::Size resolution, ui::PixelFormat format, const std::string& uniqueId,
+ compositionengine::DisplayCreationArgsBuilder& builder) {
if (auto& generator = mVirtualDisplayIdGenerators.hal) {
if (const auto id = generator->generateId()) {
if (getHwComposer().allocateVirtualDisplay(*id, resolution, &format)) {
acquireVirtualDisplaySnapshot(*id, uniqueId);
builder.setId(*id);
- return;
+ return *id;
}
generator->releaseId(*id);
@@ -682,6 +682,7 @@
LOG_ALWAYS_FATAL_IF(!id, "Failed to generate ID for GPU virtual display");
acquireVirtualDisplaySnapshot(*id, uniqueId);
builder.setId(*id);
+ return *id;
}
void SurfaceFlinger::releaseVirtualDisplay(VirtualDisplayId displayId) {
@@ -4007,10 +4008,12 @@
}
compositionengine::DisplayCreationArgsBuilder builder;
+ std::optional<VirtualDisplayIdVariant> virtualDisplayIdVariantOpt;
if (const auto& physical = state.physical) {
builder.setId(physical->id);
} else {
- acquireVirtualDisplay(resolution, pixelFormat, state.uniqueId, builder);
+ virtualDisplayIdVariantOpt =
+ acquireVirtualDisplay(resolution, pixelFormat, state.uniqueId, builder);
}
builder.setPixels(resolution);
@@ -4030,10 +4033,10 @@
getFactory().createBufferQueue(&bqProducer, &bqConsumer, /*consumerIsSurfaceFlinger =*/false);
if (state.isVirtual()) {
- const auto displayId = VirtualDisplayId::tryCast(compositionDisplay->getId());
- LOG_FATAL_IF(!displayId);
- auto surface = sp<VirtualDisplaySurface>::make(getHwComposer(), *displayId, state.surface,
- bqProducer, bqConsumer, state.displayName);
+ LOG_FATAL_IF(!virtualDisplayIdVariantOpt);
+ auto surface = sp<VirtualDisplaySurface>::make(getHwComposer(), *virtualDisplayIdVariantOpt,
+ state.surface, bqProducer, bqConsumer,
+ state.displayName);
displaySurface = surface;
producer = std::move(surface);
} else {
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 2d81738..1114aff 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -1126,8 +1126,10 @@
void enableHalVirtualDisplays(bool);
// Virtual display lifecycle for ID generation and HAL allocation.
- void acquireVirtualDisplay(ui::Size, ui::PixelFormat, const std::string& uniqueId,
- compositionengine::DisplayCreationArgsBuilder&) REQUIRES(mStateLock);
+ std::optional<VirtualDisplayIdVariant> acquireVirtualDisplay(
+ ui::Size, ui::PixelFormat, const std::string& uniqueId,
+ compositionengine::DisplayCreationArgsBuilder&) REQUIRES(mStateLock);
+
template <typename ID>
void acquireVirtualDisplaySnapshot(ID displayId, const std::string& uniqueId) {
std::lock_guard lock(mVirtualDisplaysMutex);