summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Leon Scroggins III <scroggo@google.com> 2023-05-09 13:58:18 -0400
committer Leon Scroggins III <scroggo@google.com> 2023-05-11 10:07:46 -0400
commit85d4b228a98982c8beb9eb8b1787f38fdf06a02b (patch)
tree1b05e5973a2a6f12b3da4dea9f9db86f84376695
parentca7b3dc793cfb1a9f251e411b1a23b503c5a6c29 (diff)
Move rotation flags to SF
The rotation flags are typically only used for a camera preview, which wants to avoid changing its orientation and flicker during rotation. Prior to this CL, the rotation flags were tied to the primary display, meaning that if the camera preview was on another display, the rotation flags may not be up to date. For example, if the primary display is off, its flags will not be updated on rotation. Ideally, the flags should be based on the display where the preview will be shown, but this is a much larger architectural change, tracked in b/259407931. As a temporary workaround, associate the flags with the active display. Store the flags in SurfaceFlinger, which knows when the active display changes. Update when the active display switches to a different display or when the active display rotates, matching the behavior of mActiveDisplayTransformHint, which seems similar but is different. Store the flags as a static variable so that LayerFE can access it. LayerFE does not have a way to access the actual SurfaceFlinger object, and it should not. Access to the new flags is safe because it is only read or written from the main thread. Bug: 269685949 Bug: 259407931 Test: ActiveDisplayRotationFlagsTest Change-Id: I5532e140a603be222cb3ea1ae563638317c1d745
-rw-r--r--services/surfaceflinger/DisplayDevice.cpp10
-rw-r--r--services/surfaceflinger/DisplayDevice.h4
-rw-r--r--services/surfaceflinger/FrontEnd/DisplayInfo.h2
-rw-r--r--services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp1
-rw-r--r--services/surfaceflinger/Layer.cpp4
-rw-r--r--services/surfaceflinger/LayerFE.cpp4
-rw-r--r--services/surfaceflinger/SurfaceFlinger.cpp7
-rw-r--r--services/surfaceflinger/SurfaceFlinger.h12
-rw-r--r--services/surfaceflinger/tests/unittests/ActiveDisplayRotationFlagsTest.cpp148
-rw-r--r--services/surfaceflinger/tests/unittests/Android.bp1
-rw-r--r--services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h4
11 files changed, 177 insertions, 20 deletions
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 20f4de1d67..f6ca9e2856 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -50,8 +50,6 @@ namespace android {
namespace hal = hardware::graphics::composer::hal;
-ui::Transform::RotationFlags DisplayDevice::sPrimaryDisplayRotationFlags = ui::Transform::ROT_0;
-
DisplayDeviceCreationArgs::DisplayDeviceCreationArgs(
const sp<SurfaceFlinger>& flinger, HWComposer& hwComposer, const wp<IBinder>& displayToken,
std::shared_ptr<compositionengine::Display> compositionDisplay)
@@ -282,10 +280,6 @@ void DisplayDevice::setProjection(ui::Rotation orientation, Rect layerStackSpace
Rect orientedDisplaySpaceRect) {
mOrientation = orientation;
- if (isPrimary()) {
- sPrimaryDisplayRotationFlags = ui::Transform::toRotationFlags(orientation);
- }
-
// We need to take care of display rotation for globalTransform for case if the panel is not
// installed aligned with device orientation.
const auto transformOrientation = orientation + mPhysicalOrientation;
@@ -326,10 +320,6 @@ std::optional<float> DisplayDevice::getStagedBrightness() const {
return mStagedBrightness;
}
-ui::Transform::RotationFlags DisplayDevice::getPrimaryDisplayRotationFlags() {
- return sPrimaryDisplayRotationFlags;
-}
-
void DisplayDevice::dump(utils::Dumper& dumper) const {
using namespace std::string_view_literals;
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index 51876e79a7..dc5f8a85af 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -109,8 +109,6 @@ public:
ui::Rotation getPhysicalOrientation() const { return mPhysicalOrientation; }
ui::Rotation getOrientation() const { return mOrientation; }
- static ui::Transform::RotationFlags getPrimaryDisplayRotationFlags();
-
std::optional<float> getStagedBrightness() const REQUIRES(kMainThreadContext);
ui::Transform::RotationFlags getTransformHint() const;
const ui::Transform& getTransform() const;
@@ -274,8 +272,6 @@ private:
const ui::Rotation mPhysicalOrientation;
ui::Rotation mOrientation = ui::ROTATION_0;
- static ui::Transform::RotationFlags sPrimaryDisplayRotationFlags;
-
// Allow nullopt as initial power mode.
using TracedPowerMode = TracedOrdinal<hardware::graphics::composer::hal::PowerMode>;
std::optional<TracedPowerMode> mPowerMode;
diff --git a/services/surfaceflinger/FrontEnd/DisplayInfo.h b/services/surfaceflinger/FrontEnd/DisplayInfo.h
index 527f6189fe..6502f36f70 100644
--- a/services/surfaceflinger/FrontEnd/DisplayInfo.h
+++ b/services/surfaceflinger/FrontEnd/DisplayInfo.h
@@ -31,7 +31,7 @@ struct DisplayInfo {
ui::Transform transform;
bool receivesInput;
bool isSecure;
- // TODO(b/238781169) can eliminate once sPrimaryDisplayRotationFlags is removed.
+ // TODO(b/259407931): can eliminate once SurfaceFlinger::sActiveDisplayRotationFlags is removed.
bool isPrimary;
bool isVirtual;
ui::Transform::RotationFlags rotationFlags;
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
index 7df36457df..96ff70cc34 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
@@ -665,6 +665,7 @@ void LayerSnapshotBuilder::resetRelativeState(LayerSnapshot& snapshot) {
snapshot.relativeLayerMetadata.mMap.clear();
}
+// TODO (b/259407931): Remove.
uint32_t getPrimaryDisplayRotationFlags(const DisplayInfos& displays) {
for (auto& [_, display] : displays) {
if (display.isPrimary) {
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index a8214662c6..bf9b13b91a 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -2977,7 +2977,7 @@ bool Layer::updateGeometry() {
if (mDrawingState.bufferTransform & ui::Transform::ROT_90) {
std::swap(bufferWidth, bufferHeight);
}
- uint32_t invTransform = DisplayDevice::getPrimaryDisplayRotationFlags();
+ uint32_t invTransform = SurfaceFlinger::getActiveDisplayRotationFlags();
if (mDrawingState.transformToDisplayInverse) {
if (invTransform & ui::Transform::ROT_90) {
std::swap(bufferWidth, bufferHeight);
@@ -3304,7 +3304,7 @@ Rect Layer::getBufferSize(const State& /*s*/) const {
}
if (getTransformToDisplayInverse()) {
- uint32_t invTransform = DisplayDevice::getPrimaryDisplayRotationFlags();
+ uint32_t invTransform = SurfaceFlinger::getActiveDisplayRotationFlags();
if (invTransform & ui::Transform::ROT_90) {
std::swap(bufWidth, bufHeight);
}
diff --git a/services/surfaceflinger/LayerFE.cpp b/services/surfaceflinger/LayerFE.cpp
index e713263433..f855f278c3 100644
--- a/services/surfaceflinger/LayerFE.cpp
+++ b/services/surfaceflinger/LayerFE.cpp
@@ -25,8 +25,8 @@
#include <system/window.h>
#include <utils/Log.h>
-#include "DisplayDevice.h"
#include "LayerFE.h"
+#include "SurfaceFlinger.h"
namespace android {
@@ -260,7 +260,7 @@ void LayerFE::prepareBufferStateClientComposition(
* the code below applies the primary display's inverse transform to
* the texture transform
*/
- uint32_t transform = DisplayDevice::getPrimaryDisplayRotationFlags();
+ uint32_t transform = SurfaceFlinger::getActiveDisplayRotationFlags();
mat4 tr = inverseOrientation(transform);
/**
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index de0280cf96..0dff2579ce 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -356,6 +356,8 @@ bool callingThreadHasPermission(const String16& permission) {
PermissionCache::checkPermission(permission, pid, uid);
}
+ui::Transform::RotationFlags SurfaceFlinger::sActiveDisplayRotationFlags = ui::Transform::ROT_0;
+
SurfaceFlinger::SurfaceFlinger(Factory& factory, SkipInitializationTag)
: mFactory(factory),
mPid(getpid()),
@@ -2531,7 +2533,7 @@ CompositeResult SurfaceFlinger::composite(scheduler::FrameTargeter& pacesetterFr
refreshArgs.updatingOutputGeometryThisFrame = mVisibleRegionsDirty;
refreshArgs.updatingGeometryThisFrame = mGeometryDirty.exchange(false) || mVisibleRegionsDirty;
- refreshArgs.internalDisplayRotationFlags = DisplayDevice::getPrimaryDisplayRotationFlags();
+ refreshArgs.internalDisplayRotationFlags = getActiveDisplayRotationFlags();
if (CC_UNLIKELY(mDrawingState.colorMatrixChanged)) {
refreshArgs.colorTransformMatrix = mDrawingState.colorMatrix;
@@ -3475,6 +3477,8 @@ void SurfaceFlinger::processDisplayChanged(const wp<IBinder>& displayToken,
currentState.orientedDisplaySpaceRect);
if (display->getId() == mActiveDisplayId) {
mActiveDisplayTransformHint = display->getTransformHint();
+ sActiveDisplayRotationFlags =
+ ui::Transform::toRotationFlags(display->getOrientation());
}
}
if (currentState.width != drawingState.width ||
@@ -7875,6 +7879,7 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const DisplayDevice* inactiveD
onActiveDisplaySizeChanged(activeDisplay);
mActiveDisplayTransformHint = activeDisplay.getTransformHint();
+ sActiveDisplayRotationFlags = ui::Transform::toRotationFlags(activeDisplay.getOrientation());
// The policy of the new active/pacesetter display may have changed while it was inactive. In
// that case, its preferred mode has not been propagated to HWC (via setDesiredActiveMode). In
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 1db2ab6dbb..57a0d502fe 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -332,6 +332,14 @@ public:
const DisplayDevice* getDisplayFromLayerStack(ui::LayerStack)
REQUIRES(mStateLock, kMainThreadContext);
+ // TODO (b/259407931): Remove.
+ // TODO (b/281857977): This should be annotated with REQUIRES(kMainThreadContext), but this
+ // would require thread safety annotations throughout the frontend (in particular Layer and
+ // LayerFE).
+ static ui::Transform::RotationFlags getActiveDisplayRotationFlags() {
+ return sActiveDisplayRotationFlags;
+ }
+
protected:
// We're reference counted, never destroy SurfaceFlinger directly
virtual ~SurfaceFlinger();
@@ -1364,6 +1372,10 @@ private:
std::atomic<ui::Transform::RotationFlags> mActiveDisplayTransformHint;
+ // Must only be accessed on the main thread.
+ // TODO (b/259407931): Remove.
+ static ui::Transform::RotationFlags sActiveDisplayRotationFlags;
+
bool isRefreshRateOverlayEnabled() const REQUIRES(mStateLock) {
return hasDisplay(
[](const auto& display) { return display.isRefreshRateOverlayEnabled(); });
diff --git a/services/surfaceflinger/tests/unittests/ActiveDisplayRotationFlagsTest.cpp b/services/surfaceflinger/tests/unittests/ActiveDisplayRotationFlagsTest.cpp
new file mode 100644
index 0000000000..7077523fc3
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/ActiveDisplayRotationFlagsTest.cpp
@@ -0,0 +1,148 @@
+/*
+ * Copyright 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#undef LOG_TAG
+#define LOG_TAG "LibSurfaceFlingerUnittests"
+
+#include "DisplayTransactionTestHelpers.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+namespace android {
+namespace {
+
+struct ActiveDisplayRotationFlagsTest : DisplayTransactionTest {
+ static constexpr bool kWithMockScheduler = false;
+ ActiveDisplayRotationFlagsTest() : DisplayTransactionTest(kWithMockScheduler) {}
+
+ void SetUp() override {
+ injectMockScheduler(kInnerDisplayId);
+
+ // Inject inner and outer displays with uninitialized power modes.
+ constexpr bool kInitPowerMode = false;
+ {
+ InnerDisplayVariant::injectHwcDisplay<kInitPowerMode>(this);
+ auto injector = InnerDisplayVariant::makeFakeExistingDisplayInjector(this);
+ injector.setPowerMode(std::nullopt);
+ injector.setRefreshRateSelector(mFlinger.scheduler()->refreshRateSelector());
+ mInnerDisplay = injector.inject();
+ }
+ {
+ OuterDisplayVariant::injectHwcDisplay<kInitPowerMode>(this);
+ auto injector = OuterDisplayVariant::makeFakeExistingDisplayInjector(this);
+ injector.setPowerMode(std::nullopt);
+ mOuterDisplay = injector.inject();
+ }
+
+ mFlinger.setPowerModeInternal(mInnerDisplay, PowerMode::ON);
+ mFlinger.setPowerModeInternal(mOuterDisplay, PowerMode::ON);
+
+ // The flags are a static variable, so by modifying them in the test, we
+ // are modifying the real ones used by SurfaceFlinger. Save the original
+ // flags so we can restore them on teardown. This isn't perfect - the
+ // phone may have been rotated during the test, so we're restoring the
+ // wrong flags. But if the phone is rotated, this may also fail the test.
+ mOldRotationFlags = mFlinger.mutableActiveDisplayRotationFlags();
+
+ // Reset to the expected default state.
+ mFlinger.mutableActiveDisplayRotationFlags() = ui::Transform::ROT_0;
+ }
+
+ void TearDown() override { mFlinger.mutableActiveDisplayRotationFlags() = mOldRotationFlags; }
+
+ static inline PhysicalDisplayId kInnerDisplayId = InnerDisplayVariant::DISPLAY_ID::get();
+ static inline PhysicalDisplayId kOuterDisplayId = OuterDisplayVariant::DISPLAY_ID::get();
+
+ sp<DisplayDevice> mInnerDisplay, mOuterDisplay;
+ ui::Transform::RotationFlags mOldRotationFlags;
+};
+
+TEST_F(ActiveDisplayRotationFlagsTest, defaultRotation) {
+ ASSERT_EQ(ui::Transform::ROT_0, SurfaceFlinger::getActiveDisplayRotationFlags());
+}
+
+TEST_F(ActiveDisplayRotationFlagsTest, rotate90) {
+ auto displayToken = mInnerDisplay->getDisplayToken().promote();
+ mFlinger.mutableDrawingState().displays.editValueFor(displayToken).orientation = ui::ROTATION_0;
+ mFlinger.mutableCurrentState().displays.editValueFor(displayToken).orientation =
+ ui::ROTATION_90;
+
+ mFlinger.commitTransactionsLocked(eDisplayTransactionNeeded);
+ ASSERT_EQ(ui::Transform::ROT_90, SurfaceFlinger::getActiveDisplayRotationFlags());
+}
+
+TEST_F(ActiveDisplayRotationFlagsTest, rotate90_inactive) {
+ auto displayToken = mOuterDisplay->getDisplayToken().promote();
+ mFlinger.mutableDrawingState().displays.editValueFor(displayToken).orientation = ui::ROTATION_0;
+ mFlinger.mutableCurrentState().displays.editValueFor(displayToken).orientation =
+ ui::ROTATION_90;
+
+ mFlinger.commitTransactionsLocked(eDisplayTransactionNeeded);
+ ASSERT_EQ(ui::Transform::ROT_0, SurfaceFlinger::getActiveDisplayRotationFlags());
+}
+
+TEST_F(ActiveDisplayRotationFlagsTest, rotateBoth_innerActive) {
+ auto displayToken = mInnerDisplay->getDisplayToken().promote();
+ mFlinger.mutableDrawingState().displays.editValueFor(displayToken).orientation = ui::ROTATION_0;
+ mFlinger.mutableCurrentState().displays.editValueFor(displayToken).orientation =
+ ui::ROTATION_180;
+
+ displayToken = mOuterDisplay->getDisplayToken().promote();
+ mFlinger.mutableDrawingState().displays.editValueFor(displayToken).orientation = ui::ROTATION_0;
+ mFlinger.mutableCurrentState().displays.editValueFor(displayToken).orientation =
+ ui::ROTATION_270;
+
+ mFlinger.commitTransactionsLocked(eDisplayTransactionNeeded);
+ ASSERT_EQ(ui::Transform::ROT_180, SurfaceFlinger::getActiveDisplayRotationFlags());
+}
+
+TEST_F(ActiveDisplayRotationFlagsTest, rotateBoth_outerActive) {
+ mFlinger.mutableActiveDisplayId() = kOuterDisplayId;
+ auto displayToken = mInnerDisplay->getDisplayToken().promote();
+ mFlinger.mutableDrawingState().displays.editValueFor(displayToken).orientation = ui::ROTATION_0;
+ mFlinger.mutableCurrentState().displays.editValueFor(displayToken).orientation =
+ ui::ROTATION_180;
+
+ displayToken = mOuterDisplay->getDisplayToken().promote();
+ mFlinger.mutableDrawingState().displays.editValueFor(displayToken).orientation = ui::ROTATION_0;
+ mFlinger.mutableCurrentState().displays.editValueFor(displayToken).orientation =
+ ui::ROTATION_270;
+
+ mFlinger.commitTransactionsLocked(eDisplayTransactionNeeded);
+ ASSERT_EQ(ui::Transform::ROT_270, SurfaceFlinger::getActiveDisplayRotationFlags());
+}
+
+TEST_F(ActiveDisplayRotationFlagsTest, onActiveDisplayChanged) {
+ auto displayToken = mInnerDisplay->getDisplayToken().promote();
+ mFlinger.mutableDrawingState().displays.editValueFor(displayToken).orientation = ui::ROTATION_0;
+ mFlinger.mutableCurrentState().displays.editValueFor(displayToken).orientation =
+ ui::ROTATION_180;
+
+ displayToken = mOuterDisplay->getDisplayToken().promote();
+ mFlinger.mutableDrawingState().displays.editValueFor(displayToken).orientation = ui::ROTATION_0;
+ mFlinger.mutableCurrentState().displays.editValueFor(displayToken).orientation =
+ ui::ROTATION_270;
+
+ mFlinger.commitTransactionsLocked(eDisplayTransactionNeeded);
+ ASSERT_EQ(ui::Transform::ROT_180, SurfaceFlinger::getActiveDisplayRotationFlags());
+
+ mFlinger.onActiveDisplayChanged(mInnerDisplay.get(), *mOuterDisplay);
+ ASSERT_EQ(ui::Transform::ROT_270, SurfaceFlinger::getActiveDisplayRotationFlags());
+}
+
+} // namespace
+} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp
index 55705cab76..70f8a8321e 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -70,6 +70,7 @@ cc_test {
":libsurfaceflinger_mock_sources",
":libsurfaceflinger_sources",
"libsurfaceflinger_unittest_main.cpp",
+ "ActiveDisplayRotationFlagsTest.cpp",
"CompositionTest.cpp",
"DisplayIdGeneratorTest.cpp",
"DisplayTransactionTest.cpp",
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 780bbc4c07..3a8e2ff4b7 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -617,6 +617,10 @@ public:
auto& mutablePrimaryHwcDisplayId() { return getHwComposer().mPrimaryHwcDisplayId; }
auto& mutableActiveDisplayId() { return mFlinger->mActiveDisplayId; }
+ auto& mutableActiveDisplayRotationFlags() {
+ return SurfaceFlinger::sActiveDisplayRotationFlags;
+ }
+
auto fromHandle(const sp<IBinder>& handle) { return LayerHandle::getLayer(handle); }
~TestableSurfaceFlinger() {