SF: Clean up DisplayDeviceState
The DisplayDevice::DisplayType in DisplayDeviceState will be replaced
with a DisplayId in a follow-up CL, so this CL removes unnecessary
constructors and renames displayId to sequenceId.
Bug: 74619554
Test: Build
Change-Id: I994042188299d59a62a368c12455d04891d07298
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 33e57cd..b7fa6cb 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -507,18 +507,6 @@
result.append(surfaceDump);
}
-std::atomic<int32_t> DisplayDeviceState::nextDisplayId(1);
-
-DisplayDeviceState::DisplayDeviceState(DisplayDevice::DisplayType type, bool isSecure)
- : type(type),
- layerStack(DisplayDevice::NO_LAYER_STACK),
- orientation(0),
- width(0),
- height(0),
- isSecure(isSecure)
-{
- viewport.makeInvalid();
- frame.makeInvalid();
-}
+std::atomic<int32_t> DisplayDeviceState::sNextSequenceId(1);
} // namespace android
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index d7c5b26..dd69404 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -267,15 +267,10 @@
};
struct DisplayDeviceState {
- DisplayDeviceState() = default;
- DisplayDeviceState(DisplayDevice::DisplayType type, bool isSecure);
-
bool isValid() const { return type >= 0; }
- bool isMainDisplay() const { return type == DisplayDevice::DISPLAY_PRIMARY; }
- bool isVirtualDisplay() const { return type >= DisplayDevice::DISPLAY_VIRTUAL; }
+ bool isVirtual() const { return type >= DisplayDevice::DISPLAY_VIRTUAL; }
- static std::atomic<int32_t> nextDisplayId;
- int32_t displayId = nextDisplayId++;
+ int32_t sequenceId = sNextSequenceId++;
DisplayDevice::DisplayType type = DisplayDevice::DISPLAY_ID_INVALID;
sp<IGraphicBufferProducer> surface;
uint32_t layerStack = DisplayDevice::NO_LAYER_STACK;
@@ -286,6 +281,9 @@
uint32_t height = 0;
std::string displayName;
bool isSecure = false;
+
+private:
+ static std::atomic<int32_t> sNextSequenceId;
};
class DisplayRenderArea : public RenderArea {
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 2c95e14..40a7460 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -408,8 +408,10 @@
sp<BBinder> token = new DisplayToken(this);
Mutex::Autolock _l(mStateLock);
- DisplayDeviceState info(DisplayDevice::DISPLAY_VIRTUAL, secure);
+ DisplayDeviceState info;
+ info.type = DisplayDevice::DISPLAY_VIRTUAL;
info.displayName = displayName;
+ info.isSecure = secure;
mCurrentState.displays.add(token, info);
mInterceptor->saveDisplayCreation(info);
return token;
@@ -425,11 +427,11 @@
}
const DisplayDeviceState& info(mCurrentState.displays.valueAt(idx));
- if (!info.isVirtualDisplay()) {
+ if (!info.isVirtual()) {
ALOGE("destroyDisplay called for non-virtual display");
return;
}
- mInterceptor->saveDisplayDeletion(info.displayId);
+ mInterceptor->saveDisplayDeletion(info.sequenceId);
mCurrentState.displays.removeItemsAt(idx);
setTransactionFlags(eDisplayTransactionNeeded);
}
@@ -2224,10 +2226,11 @@
if (!mBuiltinDisplays[displayType].get()) {
ALOGV("Creating built in display %d", displayType);
mBuiltinDisplays[displayType] = new BBinder();
- // All non-virtual displays are currently considered secure.
- DisplayDeviceState info(displayType, true);
+ DisplayDeviceState info;
+ info.type = displayType;
info.displayName = displayType == DisplayDevice::DISPLAY_PRIMARY ?
"Built-in Screen" : "External Screen";
+ info.isSecure = true; // All physical displays are currently considered secure.
mCurrentState.displays.add(mBuiltinDisplays[displayType], info);
mInterceptor->saveDisplayCreation(info);
}
@@ -2237,7 +2240,7 @@
ssize_t idx = mCurrentState.displays.indexOfKey(mBuiltinDisplays[displayType]);
if (idx >= 0) {
const DisplayDeviceState& info(mCurrentState.displays.valueAt(idx));
- mInterceptor->saveDisplayDeletion(info.displayId);
+ mInterceptor->saveDisplayDeletion(info.sequenceId);
mCurrentState.displays.removeItemsAt(idx);
}
mBuiltinDisplays[displayType].clear();
@@ -2415,7 +2418,7 @@
mCreateBufferQueue(&bqProducer, &bqConsumer, false);
int32_t hwcId = -1;
- if (state.isVirtualDisplay()) {
+ if (state.isVirtual()) {
// Virtual displays without a surface are dormant:
// they have external state (layer stack, projection,
// etc.) but no internal state (i.e. a DisplayDevice).
@@ -2462,7 +2465,7 @@
mDisplays.add(display,
setupNewDisplayDeviceInternal(display, hwcId, state, dispSurface,
producer));
- if (!state.isVirtualDisplay()) {
+ if (!state.isVirtual()) {
mEventThread->onHotplugReceived(state.type, true);
}
}
@@ -3701,7 +3704,7 @@
ALOGW("Surface Interceptor SavePowerMode: invalid display token");
return;
}
- mInterceptor->savePowerModeUpdate(mCurrentState.displays.valueAt(idx).displayId, mode);
+ mInterceptor->savePowerModeUpdate(mCurrentState.displays.valueAt(idx).sequenceId, mode);
}
if (currentMode == HWC_POWER_MODE_OFF) {
diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp
index 4596a21..e70506d 100644
--- a/services/surfaceflinger/SurfaceInterceptor.cpp
+++ b/services/surfaceflinger/SurfaceInterceptor.cpp
@@ -122,10 +122,10 @@
transaction->set_synchronous(false);
transaction->set_animation(false);
- addDisplaySurfaceLocked(transaction, display.displayId, display.surface);
- addDisplayLayerStackLocked(transaction, display.displayId, display.layerStack);
- addDisplaySizeLocked(transaction, display.displayId, display.width, display.height);
- addDisplayProjectionLocked(transaction, display.displayId, display.orientation,
+ addDisplaySurfaceLocked(transaction, display.sequenceId, display.surface);
+ addDisplayLayerStackLocked(transaction, display.sequenceId, display.layerStack);
+ addDisplaySizeLocked(transaction, display.sequenceId, display.width, display.height);
+ addDisplayProjectionLocked(transaction, display.sequenceId, display.orientation,
display.viewport, display.frame);
}
@@ -177,10 +177,10 @@
}
DisplayChange* SurfaceInterceptor::createDisplayChangeLocked(Transaction* transaction,
- int32_t displayId)
+ int32_t sequenceId)
{
DisplayChange* dispChange(transaction->add_display_change());
- dispChange->set_id(displayId);
+ dispChange->set_id(sequenceId);
return dispChange;
}
@@ -379,19 +379,19 @@
}
void SurfaceInterceptor::addDisplayChangesLocked(Transaction* transaction,
- const DisplayState& state, int32_t displayId)
+ const DisplayState& state, int32_t sequenceId)
{
if (state.what & DisplayState::eSurfaceChanged) {
- addDisplaySurfaceLocked(transaction, displayId, state.surface);
+ addDisplaySurfaceLocked(transaction, sequenceId, state.surface);
}
if (state.what & DisplayState::eLayerStackChanged) {
- addDisplayLayerStackLocked(transaction, displayId, state.layerStack);
+ addDisplayLayerStackLocked(transaction, sequenceId, state.layerStack);
}
if (state.what & DisplayState::eDisplaySizeChanged) {
- addDisplaySizeLocked(transaction, displayId, state.width, state.height);
+ addDisplaySizeLocked(transaction, sequenceId, state.width, state.height);
}
if (state.what & DisplayState::eDisplayProjectionChanged) {
- addDisplayProjectionLocked(transaction, displayId, state.orientation, state.viewport,
+ addDisplayProjectionLocked(transaction, sequenceId, state.orientation, state.viewport,
state.frame);
}
}
@@ -411,7 +411,7 @@
ssize_t dpyIdx = displays.indexOfKey(disp.token);
if (dpyIdx >= 0) {
const DisplayDeviceState& dispState(displays.valueAt(dpyIdx));
- addDisplayChangesLocked(transaction, disp, dispState.displayId);
+ addDisplayChangesLocked(transaction, disp, dispState.sequenceId);
}
}
}
@@ -448,7 +448,7 @@
event->set_when(timestamp);
}
-void SurfaceInterceptor::addDisplaySurfaceLocked(Transaction* transaction, int32_t displayId,
+void SurfaceInterceptor::addDisplaySurfaceLocked(Transaction* transaction, int32_t sequenceId,
const sp<const IGraphicBufferProducer>& surface)
{
if (surface == nullptr) {
@@ -457,7 +457,7 @@
uint64_t bufferQueueId = 0;
status_t err(surface->getUniqueId(&bufferQueueId));
if (err == NO_ERROR) {
- DisplayChange* dispChange(createDisplayChangeLocked(transaction, displayId));
+ DisplayChange* dispChange(createDisplayChangeLocked(transaction, sequenceId));
DispSurfaceChange* surfaceChange(dispChange->mutable_surface());
surfaceChange->set_buffer_queue_id(bufferQueueId);
surfaceChange->set_buffer_queue_name(surface->getConsumerName().string());
@@ -469,26 +469,26 @@
}
void SurfaceInterceptor::addDisplayLayerStackLocked(Transaction* transaction,
- int32_t displayId, uint32_t layerStack)
+ int32_t sequenceId, uint32_t layerStack)
{
- DisplayChange* dispChange(createDisplayChangeLocked(transaction, displayId));
+ DisplayChange* dispChange(createDisplayChangeLocked(transaction, sequenceId));
LayerStackChange* layerStackChange(dispChange->mutable_layer_stack());
layerStackChange->set_layer_stack(layerStack);
}
-void SurfaceInterceptor::addDisplaySizeLocked(Transaction* transaction, int32_t displayId,
+void SurfaceInterceptor::addDisplaySizeLocked(Transaction* transaction, int32_t sequenceId,
uint32_t w, uint32_t h)
{
- DisplayChange* dispChange(createDisplayChangeLocked(transaction, displayId));
+ DisplayChange* dispChange(createDisplayChangeLocked(transaction, sequenceId));
SizeChange* sizeChange(dispChange->mutable_size());
sizeChange->set_w(w);
sizeChange->set_h(h);
}
void SurfaceInterceptor::addDisplayProjectionLocked(Transaction* transaction,
- int32_t displayId, int32_t orientation, const Rect& viewport, const Rect& frame)
+ int32_t sequenceId, int32_t orientation, const Rect& viewport, const Rect& frame)
{
- DisplayChange* dispChange(createDisplayChangeLocked(transaction, displayId));
+ DisplayChange* dispChange(createDisplayChangeLocked(transaction, sequenceId));
ProjectionChange* projectionChange(dispChange->mutable_projection());
projectionChange->set_orientation(orientation);
Rectangle* viewportRect(projectionChange->mutable_viewport());
@@ -501,22 +501,22 @@
const DisplayDeviceState& info)
{
DisplayCreation* creation(increment->mutable_display_creation());
- creation->set_id(info.displayId);
+ creation->set_id(info.sequenceId);
creation->set_name(info.displayName);
creation->set_type(info.type);
creation->set_is_secure(info.isSecure);
}
-void SurfaceInterceptor::addDisplayDeletionLocked(Increment* increment, int32_t displayId) {
+void SurfaceInterceptor::addDisplayDeletionLocked(Increment* increment, int32_t sequenceId) {
DisplayDeletion* deletion(increment->mutable_display_deletion());
- deletion->set_id(displayId);
+ deletion->set_id(sequenceId);
}
-void SurfaceInterceptor::addPowerModeUpdateLocked(Increment* increment, int32_t displayId,
+void SurfaceInterceptor::addPowerModeUpdateLocked(Increment* increment, int32_t sequenceId,
int32_t mode)
{
PowerModeUpdate* powerModeUpdate(increment->mutable_power_mode_update());
- powerModeUpdate->set_id(displayId);
+ powerModeUpdate->set_id(sequenceId);
powerModeUpdate->set_mode(mode);
}
@@ -579,22 +579,22 @@
addDisplayCreationLocked(createTraceIncrementLocked(), info);
}
-void SurfaceInterceptor::saveDisplayDeletion(int32_t displayId) {
+void SurfaceInterceptor::saveDisplayDeletion(int32_t sequenceId) {
if (!mEnabled) {
return;
}
ATRACE_CALL();
std::lock_guard<std::mutex> protoGuard(mTraceMutex);
- addDisplayDeletionLocked(createTraceIncrementLocked(), displayId);
+ addDisplayDeletionLocked(createTraceIncrementLocked(), sequenceId);
}
-void SurfaceInterceptor::savePowerModeUpdate(int32_t displayId, int32_t mode) {
+void SurfaceInterceptor::savePowerModeUpdate(int32_t sequenceId, int32_t mode) {
if (!mEnabled) {
return;
}
ATRACE_CALL();
std::lock_guard<std::mutex> protoGuard(mTraceMutex);
- addPowerModeUpdateLocked(createTraceIncrementLocked(), displayId, mode);
+ addPowerModeUpdateLocked(createTraceIncrementLocked(), sequenceId, mode);
}
} // namespace impl
diff --git a/services/surfaceflinger/SurfaceInterceptor.h b/services/surfaceflinger/SurfaceInterceptor.h
index 96defcc..218a1d2 100644
--- a/services/surfaceflinger/SurfaceInterceptor.h
+++ b/services/surfaceflinger/SurfaceInterceptor.h
@@ -66,8 +66,8 @@
// Intercept display data
virtual void saveDisplayCreation(const DisplayDeviceState& info) = 0;
- virtual void saveDisplayDeletion(int32_t displayId) = 0;
- virtual void savePowerModeUpdate(int32_t displayId, int32_t mode) = 0;
+ virtual void saveDisplayDeletion(int32_t sequenceId) = 0;
+ virtual void savePowerModeUpdate(int32_t sequenceId, int32_t mode) = 0;
virtual void saveVSyncEvent(nsecs_t timestamp) = 0;
};
@@ -101,8 +101,8 @@
// Intercept display data
void saveDisplayCreation(const DisplayDeviceState& info) override;
- void saveDisplayDeletion(int32_t displayId) override;
- void savePowerModeUpdate(int32_t displayId, int32_t mode) override;
+ void saveDisplayDeletion(int32_t sequenceId) override;
+ void savePowerModeUpdate(int32_t sequenceId, int32_t mode) override;
void saveVSyncEvent(nsecs_t timestamp) override;
private:
@@ -127,8 +127,8 @@
uint32_t height, uint64_t frameNumber);
void addVSyncUpdateLocked(Increment* increment, nsecs_t timestamp);
void addDisplayCreationLocked(Increment* increment, const DisplayDeviceState& info);
- void addDisplayDeletionLocked(Increment* increment, int32_t displayId);
- void addPowerModeUpdateLocked(Increment* increment, int32_t displayId, int32_t mode);
+ void addDisplayDeletionLocked(Increment* increment, int32_t sequenceId);
+ void addPowerModeUpdateLocked(Increment* increment, int32_t sequenceId, int32_t mode);
// Add surface transactions to the trace
SurfaceChange* createSurfaceChangeLocked(Transaction* transaction, int32_t layerId);
@@ -155,17 +155,17 @@
const Vector<DisplayState>& changedDisplays, uint32_t transactionFlags);
// Add display transactions to the trace
- DisplayChange* createDisplayChangeLocked(Transaction* transaction, int32_t displayId);
- void addDisplaySurfaceLocked(Transaction* transaction, int32_t displayId,
+ DisplayChange* createDisplayChangeLocked(Transaction* transaction, int32_t sequenceId);
+ void addDisplaySurfaceLocked(Transaction* transaction, int32_t sequenceId,
const sp<const IGraphicBufferProducer>& surface);
- void addDisplayLayerStackLocked(Transaction* transaction, int32_t displayId,
+ void addDisplayLayerStackLocked(Transaction* transaction, int32_t sequenceId,
uint32_t layerStack);
- void addDisplaySizeLocked(Transaction* transaction, int32_t displayId, uint32_t w,
+ void addDisplaySizeLocked(Transaction* transaction, int32_t sequenceId, uint32_t w,
uint32_t h);
- void addDisplayProjectionLocked(Transaction* transaction, int32_t displayId,
+ void addDisplayProjectionLocked(Transaction* transaction, int32_t sequenceId,
int32_t orientation, const Rect& viewport, const Rect& frame);
void addDisplayChangesLocked(Transaction* transaction,
- const DisplayState& state, int32_t displayId);
+ const DisplayState& state, int32_t sequenceId);
bool mEnabled {false};
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
index f39ca00..48424fb 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
@@ -230,7 +230,9 @@
.add(displayToken,
FakeDisplayDeviceFactory(mFlinger, displayToken, type, type).build());
- DisplayDeviceState state(type, true);
+ DisplayDeviceState state;
+ state.type = type;
+ state.isSecure = true;
mFlinger.mutableCurrentState().displays.add(displayToken, state);
mFlinger.mutableDrawingState().displays.add(displayToken, state);