Skip to content

Commit f8dcf14

Browse files
Dominik LaskowskiAndroid (Google) Code Review
authored andcommitted
Merge "SF: Remove StrongTyping in favor of FTL mixins" into main
2 parents 8b789ff + 43baf90 commit f8dcf14

14 files changed

Lines changed: 89 additions & 273 deletions

services/surfaceflinger/DisplayHardware/DisplayMode.h

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,17 @@
2121

2222
#include <android-base/stringprintf.h>
2323
#include <android/configuration.h>
24+
#include <ftl/mixins.h>
2425
#include <ftl/small_map.h>
2526
#include <ui/DisplayId.h>
2627
#include <ui/DisplayMode.h>
2728
#include <ui/Size.h>
2829
#include <utils/Timers.h>
2930

31+
#include <common/FlagManager.h>
3032
#include <scheduler/Fps.h>
3133

32-
#include <common/FlagManager.h>
3334
#include "DisplayHardware/Hal.h"
34-
#include "Scheduler/StrongTyping.h"
3535

3636
namespace android {
3737

@@ -46,7 +46,12 @@ bool operator>(const DisplayModePtr&, const DisplayModePtr&) = delete;
4646
bool operator<=(const DisplayModePtr&, const DisplayModePtr&) = delete;
4747
bool operator>=(const DisplayModePtr&, const DisplayModePtr&) = delete;
4848

49-
using DisplayModeId = StrongTyping<ui::DisplayModeId, struct DisplayModeIdTag, Compare>;
49+
struct DisplayModeId : ftl::DefaultConstructible<DisplayModeId, ui::DisplayModeId>,
50+
ftl::Incrementable<DisplayModeId>,
51+
ftl::Equatable<DisplayModeId>,
52+
ftl::Orderable<DisplayModeId> {
53+
using DefaultConstructible::DefaultConstructible;
54+
};
5055

5156
using DisplayModes = ftl::SmallMap<DisplayModeId, DisplayModePtr, 3>;
5257
using DisplayModeIterator = DisplayModes::const_iterator;
@@ -185,7 +190,7 @@ inline bool equalsExceptDisplayModeId(const DisplayMode& lhs, const DisplayMode&
185190
inline std::string to_string(const DisplayMode& mode) {
186191
return base::StringPrintf("{id=%d, hwcId=%d, resolution=%dx%d, vsyncRate=%s, "
187192
"dpi=%.2fx%.2f, group=%d, vrrConfig=%s}",
188-
mode.getId().value(), mode.getHwcId(), mode.getWidth(),
193+
ftl::to_underlying(mode.getId()), mode.getHwcId(), mode.getWidth(),
189194
mode.getHeight(), to_string(mode.getVsyncRate()).c_str(),
190195
mode.getDpi().x, mode.getDpi().y, mode.getGroup(),
191196
to_string(mode.getVrrConfig()).c_str());

services/surfaceflinger/Scheduler/EventThread.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ DisplayEventReceiver::Event makeModeChanged(const scheduler::FrameRateMode& mode
148148
DisplayEventReceiver::Event event;
149149
event.header = {DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE,
150150
mode.modePtr->getPhysicalDisplayId(), systemTime()};
151-
event.modeChange.modeId = mode.modePtr->getId().value();
151+
event.modeChange.modeId = ftl::to_underlying(mode.modePtr->getId());
152152
event.modeChange.vsyncPeriod = mode.fps.getPeriodNsecs();
153153
return event;
154154
}

services/surfaceflinger/Scheduler/RefreshRateSelector.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,8 @@ struct RefreshRateSelector::RefreshRateScoreComparator {
286286
std::string RefreshRateSelector::Policy::toString() const {
287287
return base::StringPrintf("{defaultModeId=%d, allowGroupSwitching=%s"
288288
", primaryRanges=%s, appRequestRanges=%s}",
289-
defaultMode.value(), allowGroupSwitching ? "true" : "false",
289+
ftl::to_underlying(defaultMode),
290+
allowGroupSwitching ? "true" : "false",
290291
to_string(primaryRanges).c_str(),
291292
to_string(appRequestRanges).c_str());
292293
}

services/surfaceflinger/Scheduler/RefreshRateSelector.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131

3232
#include "DisplayHardware/DisplayMode.h"
3333
#include "Scheduler/OneShotTimer.h"
34-
#include "Scheduler/StrongTyping.h"
3534
#include "ThreadContext.h"
3635
#include "Utils/Dumper.h"
3736

services/surfaceflinger/Scheduler/StrongTyping.h

Lines changed: 0 additions & 89 deletions
This file was deleted.

services/surfaceflinger/Scheduler/VSyncDispatch.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,9 @@
2020
#include <optional>
2121
#include <string>
2222

23+
#include <ftl/mixins.h>
2324
#include <utils/Timers.h>
2425

25-
#include "StrongTyping.h"
26-
2726
namespace android::scheduler {
2827

2928
using ScheduleResult = std::optional<nsecs_t>;
@@ -35,7 +34,11 @@ enum class CancelResult { Cancelled, TooLate, Error };
3534
*/
3635
class VSyncDispatch {
3736
public:
38-
using CallbackToken = StrongTyping<size_t, class CallbackTokenTag, Compare>;
37+
struct CallbackToken : ftl::DefaultConstructible<CallbackToken, size_t>,
38+
ftl::Equatable<CallbackToken>,
39+
ftl::Incrementable<CallbackToken> {
40+
using DefaultConstructible::DefaultConstructible;
41+
};
3942

4043
virtual ~VSyncDispatch();
4144

services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ VSyncDispatchTimerQueue::CallbackToken VSyncDispatchTimerQueue::registerCallback
352352
Callback callback, std::string callbackName) {
353353
std::lock_guard lock(mMutex);
354354
return mCallbacks
355-
.try_emplace(CallbackToken{++mCallbackToken},
355+
.try_emplace(++mCallbackToken,
356356
std::make_shared<VSyncDispatchTimerQueueEntry>(std::move(callbackName),
357357
std::move(callback),
358358
mMinVsyncDistance))

services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ class VSyncDispatchTimerQueue : public VSyncDispatch {
158158
nsecs_t const mTimerSlack;
159159
nsecs_t const mMinVsyncDistance;
160160

161-
size_t mCallbackToken GUARDED_BY(mMutex) = 0;
161+
CallbackToken mCallbackToken GUARDED_BY(mMutex);
162162

163163
CallbackMap mCallbacks GUARDED_BY(mMutex);
164164
nsecs_t mIntendedWakeupTime GUARDED_BY(mMutex) = kInvalidTime;

services/surfaceflinger/SurfaceFlinger.cpp

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,7 +1083,7 @@ void SurfaceFlinger::getDynamicDisplayInfoInternal(ui::DynamicDisplayInfo*& info
10831083

10841084
for (const auto& [id, mode] : displayModes) {
10851085
ui::DisplayMode outMode;
1086-
outMode.id = static_cast<int32_t>(id.value());
1086+
outMode.id = ftl::to_underlying(id);
10871087

10881088
auto [width, height] = mode->getResolution();
10891089
auto [xDpi, yDpi] = mode->getDpi();
@@ -1132,7 +1132,7 @@ void SurfaceFlinger::getDynamicDisplayInfoInternal(ui::DynamicDisplayInfo*& info
11321132
const PhysicalDisplayId displayId = snapshot.displayId();
11331133

11341134
const auto mode = display->refreshRateSelector().getActiveMode();
1135-
info->activeDisplayModeId = mode.modePtr->getId().value();
1135+
info->activeDisplayModeId = ftl::to_underlying(mode.modePtr->getId());
11361136
info->renderFrameRate = mode.fps.getValue();
11371137
info->activeColorMode = display->getCompositionDisplay()->getState().colorMode;
11381138
info->hdrCapabilities = filterOut4k30(display->getHdrCapabilities());
@@ -1148,7 +1148,7 @@ void SurfaceFlinger::getDynamicDisplayInfoInternal(ui::DynamicDisplayInfo*& info
11481148
if (getHwComposer().hasCapability(Capability::BOOT_DISPLAY_CONFIG)) {
11491149
if (const auto hwcId = getHwComposer().getPreferredBootDisplayMode(displayId)) {
11501150
if (const auto modeId = snapshot.translateModeId(*hwcId)) {
1151-
info->preferredBootDisplayMode = modeId->value();
1151+
info->preferredBootDisplayMode = ftl::to_underlying(*modeId);
11521152
}
11531153
}
11541154
}
@@ -1312,7 +1312,7 @@ status_t SurfaceFlinger::setActiveModeFromBackdoor(const sp<display::DisplayToke
13121312
[](const DisplayModePtr& mode) { return mode->getPeakFps(); });
13131313

13141314
if (!fpsOpt) {
1315-
ALOGE("%s: Invalid mode %d for display %s", whence, modeId.value(),
1315+
ALOGE("%s: Invalid mode %d for display %s", whence, ftl::to_underlying(modeId),
13161316
to_string(snapshot.displayId()).c_str());
13171317
return BAD_VALUE;
13181318
}
@@ -1421,12 +1421,12 @@ void SurfaceFlinger::initiateDisplayModeChanges() {
14211421

14221422
if (!displayModePtrOpt) {
14231423
ALOGW("Desired display mode is no longer supported. Mode ID = %d",
1424-
desiredModeId.value());
1425-
dropModeRequest(display);
1424+
ftl::to_underlying(desiredModeId));
14261425
continue;
14271426
}
14281427

1429-
ALOGV("%s changing active mode to %d(%s) for display %s", __func__, desiredModeId.value(),
1428+
ALOGV("%s changing active mode to %d(%s) for display %s", __func__,
1429+
ftl::to_underlying(desiredModeId),
14301430
to_string(displayModePtrOpt->get()->getVsyncRate()).c_str(),
14311431
to_string(display->getId()).c_str());
14321432

@@ -1619,7 +1619,7 @@ status_t SurfaceFlinger::setBootDisplayMode(const sp<display::DisplayToken>& dis
16191619
[](const DisplayModePtr& mode) { return mode->getHwcId(); });
16201620

16211621
if (!hwcIdOpt) {
1622-
ALOGE("%s: Invalid mode %d for display %s", whence, modeId.value(),
1622+
ALOGE("%s: Invalid mode %d for display %s", whence, ftl::to_underlying(modeId),
16231623
to_string(snapshot.displayId()).c_str());
16241624
return BAD_VALUE;
16251625
}
@@ -3393,15 +3393,15 @@ std::pair<DisplayModes, DisplayModePtr> SurfaceFlinger::loadDisplayModes(
33933393
})
33943394
.value_or(DisplayModes{});
33953395

3396-
ui::DisplayModeId nextModeId = 1 +
3397-
std::accumulate(oldModes.begin(), oldModes.end(), static_cast<ui::DisplayModeId>(-1),
3398-
[](ui::DisplayModeId max, const auto& pair) {
3399-
return std::max(max, pair.first.value());
3400-
});
3396+
DisplayModeId nextModeId = std::accumulate(oldModes.begin(), oldModes.end(), DisplayModeId(-1),
3397+
[](DisplayModeId max, const auto& pair) {
3398+
return std::max(max, pair.first);
3399+
});
3400+
++nextModeId;
34013401

34023402
DisplayModes newModes;
34033403
for (const auto& hwcMode : hwcModes) {
3404-
const DisplayModeId id{nextModeId++};
3404+
const auto id = nextModeId++;
34053405
newModes.try_emplace(id,
34063406
DisplayMode::Builder(hwcMode.hwcId)
34073407
.setId(id)
@@ -4182,8 +4182,8 @@ void SurfaceFlinger::requestDisplayModes(std::vector<display::DisplayModeRequest
41824182
if (display->refreshRateSelector().isModeAllowed(request.mode)) {
41834183
setDesiredMode(std::move(request));
41844184
} else {
4185-
ALOGV("%s: Mode %d is disallowed for display %s", __func__, modePtr->getId().value(),
4186-
to_string(displayId).c_str());
4185+
ALOGV("%s: Mode %d is disallowed for display %s", __func__,
4186+
ftl::to_underlying(modePtr->getId()), to_string(displayId).c_str());
41874187
}
41884188
}
41894189
}
@@ -8477,11 +8477,11 @@ status_t SurfaceFlinger::applyRefreshRateSelectorPolicy(
84778477
const auto preferredModeId = preferredMode.modePtr->getId();
84788478

84798479
const Fps preferredFps = preferredMode.fps;
8480-
ALOGV("Switching to Scheduler preferred mode %d (%s)", preferredModeId.value(),
8480+
ALOGV("Switching to Scheduler preferred mode %d (%s)", ftl::to_underlying(preferredModeId),
84818481
to_string(preferredFps).c_str());
84828482

84838483
if (!selector.isModeAllowed(preferredMode)) {
8484-
ALOGE("%s: Preferred mode %d is disallowed", __func__, preferredModeId.value());
8484+
ALOGE("%s: Preferred mode %d is disallowed", __func__, ftl::to_underlying(preferredModeId));
84858485
return INVALID_OPERATION;
84868486
}
84878487

@@ -8569,7 +8569,7 @@ status_t SurfaceFlinger::getDesiredDisplayModeSpecs(const sp<IBinder>& displayTo
85698569

85708570
scheduler::RefreshRateSelector::Policy policy =
85718571
display->refreshRateSelector().getDisplayManagerPolicy();
8572-
outSpecs->defaultMode = policy.defaultMode.value();
8572+
outSpecs->defaultMode = ftl::to_underlying(policy.defaultMode);
85738573
outSpecs->allowGroupSwitching = policy.allowGroupSwitching;
85748574
outSpecs->primaryRanges = translate(policy.primaryRanges);
85758575
outSpecs->appRequestRanges = translate(policy.appRequestRanges);

services/surfaceflinger/tests/unittests/Android.bp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,6 @@ cc_test {
129129
"TransactionTraceWriterTest.cpp",
130130
"TransactionTracingTest.cpp",
131131
"TunnelModeEnabledReporterTest.cpp",
132-
"StrongTypingTest.cpp",
133132
"VSyncCallbackRegistrationTest.cpp",
134133
"VSyncDispatchTimerQueueTest.cpp",
135134
"VSyncDispatchRealtimeTest.cpp",

0 commit comments

Comments
 (0)