CE: Use distinct source and destination clip rectangles
The existing logic used a single clip rectangle, which was passed to
RenderEngine as both DisplaySettings.physicalDisplay and
DisplaySettings.clip. In at least the GLES backend, the first is used to
set the GL viewport, and the later is used to set the clip rectangle.
However if an Output is set up to render an arbitrary subrectangle of
a larger physical display space, distinct values need to be used --
typically the viewport (destination clip) is set to (0, 0, w, h), while
the source clip is set to some (x0, y0, x0 + w, y0 + w).
Note that in particular we still need to both rectangles to the same (0,
y0, w, y0 + h) rectangle when the goal is to hide a notch on a physical
device, so the rectangle settings must be obtained from code outside of
libcompositionengine.
Test: go/wm-smoke on crosshatch, checking notch-hiding
Test: atest libcompositionengine_test
Test: CtsColorModeTestCases
Test: CtsDisplayTestCases
Test: CtsGraphicsTestCases
Test: CtsUiRenderingTestCases
Test: CtsViewTestCases
Test: android.media.cts.EncodeVirtualDisplayWithCompositionTest
Bug: 144116549
Change-Id: Ic7218da2457f2645af180a1b4eed3dd6896db7d1
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
index a5da0b1..4ab7082 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
@@ -163,7 +163,8 @@
// Sets the projection state to use
virtual void setProjection(const ui::Transform&, uint32_t orientation, const Rect& frame,
- const Rect& viewport, const Rect& scissor, bool needsFiltering) = 0;
+ const Rect& viewport, const Rect& sourceClip,
+ const Rect& destinationClip, bool needsFiltering) = 0;
// Sets the bounds to use
virtual void setBounds(const ui::Size&) = 0;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
index f469e62..8dc440c 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
@@ -39,7 +39,8 @@
std::optional<DisplayId> getDisplayId() const override;
void setCompositionEnabled(bool) override;
void setProjection(const ui::Transform&, uint32_t orientation, const Rect& frame,
- const Rect& viewport, const Rect& scissor, bool needsFiltering) override;
+ const Rect& viewport, const Rect& sourceClip, const Rect& destinationClip,
+ bool needsFiltering) override;
void setBounds(const ui::Size&) override;
void setLayerStackFilter(uint32_t layerStackId, bool isInternal) override;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
index e700b76..66ed2b6 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
@@ -79,8 +79,11 @@
// The logical space user viewport rectangle
Rect viewport;
- // The physical space scissor rectangle
- Rect scissor;
+ // The physical space source clip rectangle
+ Rect sourceClip;
+
+ // The physical space destination clip rectangle
+ Rect destinationClip;
// If true, RenderEngine filtering should be enabled
bool needsFiltering{false};
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
index c41302d..59906b9 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
@@ -37,8 +37,9 @@
MOCK_CONST_METHOD0(getDisplayId, std::optional<DisplayId>());
MOCK_METHOD1(setCompositionEnabled, void(bool));
- MOCK_METHOD6(setProjection,
- void(const ui::Transform&, uint32_t, const Rect&, const Rect&, const Rect&, bool));
+ MOCK_METHOD7(setProjection,
+ void(const ui::Transform&, uint32_t, const Rect&, const Rect&, const Rect&,
+ const Rect&, bool));
MOCK_METHOD1(setBounds, void(const ui::Size&));
MOCK_METHOD2(setLayerStackFilter, void(uint32_t, bool));
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index f12d1c7..ce4b84a 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -107,11 +107,13 @@
}
void Output::setProjection(const ui::Transform& transform, uint32_t orientation, const Rect& frame,
- const Rect& viewport, const Rect& scissor, bool needsFiltering) {
+ const Rect& viewport, const Rect& sourceClip,
+ const Rect& destinationClip, bool needsFiltering) {
auto& outputState = editState();
outputState.transform = transform;
outputState.orientation = orientation;
- outputState.scissor = scissor;
+ outputState.sourceClip = sourceClip;
+ outputState.destinationClip = destinationClip;
outputState.frame = frame;
outputState.viewport = viewport;
outputState.needsFiltering = needsFiltering;
@@ -834,8 +836,8 @@
const bool supportsProtectedContent = renderEngine.supportsProtectedContent();
renderengine::DisplaySettings clientCompositionDisplay;
- clientCompositionDisplay.physicalDisplay = outputState.scissor;
- clientCompositionDisplay.clip = outputState.scissor;
+ clientCompositionDisplay.physicalDisplay = outputState.destinationClip;
+ clientCompositionDisplay.clip = outputState.sourceClip;
clientCompositionDisplay.globalTransform = outputState.transform.asMatrix4();
clientCompositionDisplay.orientation = outputState.orientation;
clientCompositionDisplay.outputDataspace = mDisplayColorProfile->hasWideColorGamut()
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp
index 84d79f7..ca5be48 100644
--- a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp
@@ -40,7 +40,8 @@
dumpVal(out, "frame", frame);
dumpVal(out, "viewport", viewport);
- dumpVal(out, "scissor", scissor);
+ dumpVal(out, "sourceClip", sourceClip);
+ dumpVal(out, "destinationClip", destinationClip);
dumpVal(out, "needsFiltering", needsFiltering);
out.append("\n ");
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
index 6e8d3df..463d095 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
@@ -249,16 +249,19 @@
const int32_t orientation = 123;
const Rect frame{1, 2, 3, 4};
const Rect viewport{5, 6, 7, 8};
- const Rect scissor{9, 10, 11, 12};
+ const Rect sourceClip{9, 10, 11, 12};
+ const Rect destinationClip{13, 14, 15, 16};
const bool needsFiltering = true;
- mOutput->setProjection(transform, orientation, frame, viewport, scissor, needsFiltering);
+ mOutput->setProjection(transform, orientation, frame, viewport, sourceClip, destinationClip,
+ needsFiltering);
EXPECT_THAT(mOutput->getState().transform, transform);
EXPECT_EQ(orientation, mOutput->getState().orientation);
EXPECT_EQ(frame, mOutput->getState().frame);
EXPECT_EQ(viewport, mOutput->getState().viewport);
- EXPECT_EQ(scissor, mOutput->getState().scissor);
+ EXPECT_EQ(sourceClip, mOutput->getState().sourceClip);
+ EXPECT_EQ(destinationClip, mOutput->getState().destinationClip);
EXPECT_EQ(needsFiltering, mOutput->getState().needsFiltering);
}
@@ -2778,7 +2781,8 @@
mOutput.mState.frame = kDefaultOutputFrame;
mOutput.mState.viewport = kDefaultOutputViewport;
- mOutput.mState.scissor = kDefaultOutputScissor;
+ mOutput.mState.sourceClip = kDefaultOutputSourceClip;
+ mOutput.mState.destinationClip = kDefaultOutputDestinationClip;
mOutput.mState.transform = ui::Transform{kDefaultOutputOrientation};
mOutput.mState.orientation = kDefaultOutputOrientation;
mOutput.mState.dataspace = kDefaultOutputDataspace;
@@ -2822,7 +2826,8 @@
static const Rect kDefaultOutputFrame;
static const Rect kDefaultOutputViewport;
- static const Rect kDefaultOutputScissor;
+ static const Rect kDefaultOutputSourceClip;
+ static const Rect kDefaultOutputDestinationClip;
static const mat4 kDefaultColorTransformMat;
static const Region kDebugRegion;
@@ -2842,7 +2847,8 @@
const Rect OutputComposeSurfacesTest::kDefaultOutputFrame{1001, 1002, 1003, 1004};
const Rect OutputComposeSurfacesTest::kDefaultOutputViewport{1005, 1006, 1007, 1008};
-const Rect OutputComposeSurfacesTest::kDefaultOutputScissor{1009, 1010, 1011, 1012};
+const Rect OutputComposeSurfacesTest::kDefaultOutputSourceClip{1009, 1010, 1011, 1012};
+const Rect OutputComposeSurfacesTest::kDefaultOutputDestinationClip{1013, 1014, 1015, 1016};
const mat4 OutputComposeSurfacesTest::kDefaultColorTransformMat{mat4() * 0.5f};
const Region OutputComposeSurfacesTest::kDebugRegion{Rect{100, 101, 102, 103}};
const HdrCapabilities OutputComposeSurfacesTest::
@@ -3085,9 +3091,10 @@
verify().ifMixedCompositionIs(true)
.andIfUsesHdr(true)
.andIfSkipColorTransform(false)
- .thenExpectDisplaySettingsUsed({kDefaultOutputScissor, kDefaultOutputScissor, mat4(),
- kDefaultMaxLuminance, kDefaultOutputDataspace, mat4(),
- Region::INVALID_REGION, kDefaultOutputOrientation})
+ .thenExpectDisplaySettingsUsed({kDefaultOutputDestinationClip, kDefaultOutputSourceClip,
+ mat4(), kDefaultMaxLuminance, kDefaultOutputDataspace,
+ mat4(), Region::INVALID_REGION,
+ kDefaultOutputOrientation})
.execute()
.expectAFenceWasReturned();
}
@@ -3096,9 +3103,10 @@
verify().ifMixedCompositionIs(true)
.andIfUsesHdr(false)
.andIfSkipColorTransform(false)
- .thenExpectDisplaySettingsUsed({kDefaultOutputScissor, kDefaultOutputScissor, mat4(),
- kDefaultMaxLuminance, kDefaultOutputDataspace, mat4(),
- Region::INVALID_REGION, kDefaultOutputOrientation})
+ .thenExpectDisplaySettingsUsed({kDefaultOutputDestinationClip, kDefaultOutputSourceClip,
+ mat4(), kDefaultMaxLuminance, kDefaultOutputDataspace,
+ mat4(), Region::INVALID_REGION,
+ kDefaultOutputOrientation})
.execute()
.expectAFenceWasReturned();
}
@@ -3107,8 +3115,8 @@
verify().ifMixedCompositionIs(false)
.andIfUsesHdr(true)
.andIfSkipColorTransform(false)
- .thenExpectDisplaySettingsUsed({kDefaultOutputScissor, kDefaultOutputScissor, mat4(),
- kDefaultMaxLuminance, kDefaultOutputDataspace,
+ .thenExpectDisplaySettingsUsed({kDefaultOutputDestinationClip, kDefaultOutputSourceClip,
+ mat4(), kDefaultMaxLuminance, kDefaultOutputDataspace,
kDefaultColorTransformMat, Region::INVALID_REGION,
kDefaultOutputOrientation})
.execute()
@@ -3119,8 +3127,8 @@
verify().ifMixedCompositionIs(false)
.andIfUsesHdr(false)
.andIfSkipColorTransform(false)
- .thenExpectDisplaySettingsUsed({kDefaultOutputScissor, kDefaultOutputScissor, mat4(),
- kDefaultMaxLuminance, kDefaultOutputDataspace,
+ .thenExpectDisplaySettingsUsed({kDefaultOutputDestinationClip, kDefaultOutputSourceClip,
+ mat4(), kDefaultMaxLuminance, kDefaultOutputDataspace,
kDefaultColorTransformMat, Region::INVALID_REGION,
kDefaultOutputOrientation})
.execute()
@@ -3132,9 +3140,10 @@
verify().ifMixedCompositionIs(false)
.andIfUsesHdr(true)
.andIfSkipColorTransform(true)
- .thenExpectDisplaySettingsUsed({kDefaultOutputScissor, kDefaultOutputScissor, mat4(),
- kDefaultMaxLuminance, kDefaultOutputDataspace, mat4(),
- Region::INVALID_REGION, kDefaultOutputOrientation})
+ .thenExpectDisplaySettingsUsed({kDefaultOutputDestinationClip, kDefaultOutputSourceClip,
+ mat4(), kDefaultMaxLuminance, kDefaultOutputDataspace,
+ mat4(), Region::INVALID_REGION,
+ kDefaultOutputOrientation})
.execute()
.expectAFenceWasReturned();
}
@@ -3345,7 +3354,8 @@
GenerateClientCompositionRequestsTest_ThreeLayers() {
mOutput.mState.frame = kDisplayFrame;
mOutput.mState.viewport = kDisplayViewport;
- mOutput.mState.scissor = kDisplayScissor;
+ mOutput.mState.sourceClip = kDisplaySourceClip;
+ mOutput.mState.destinationClip = kDisplayDestinationClip;
mOutput.mState.transform = ui::Transform{kDisplayOrientation};
mOutput.mState.orientation = kDisplayOrientation;
mOutput.mState.needsFiltering = false;
@@ -3376,14 +3386,17 @@
static const Rect kDisplayFrame;
static const Rect kDisplayViewport;
- static const Rect kDisplayScissor;
+ static const Rect kDisplaySourceClip;
+ static const Rect kDisplayDestinationClip;
std::array<Layer, 3> mLayers;
};
const Rect GenerateClientCompositionRequestsTest_ThreeLayers::kDisplayFrame(0, 0, 100, 200);
const Rect GenerateClientCompositionRequestsTest_ThreeLayers::kDisplayViewport(0, 0, 101, 201);
-const Rect GenerateClientCompositionRequestsTest_ThreeLayers::kDisplayScissor(0, 0, 102, 202);
+const Rect GenerateClientCompositionRequestsTest_ThreeLayers::kDisplaySourceClip(0, 0, 102, 202);
+const Rect GenerateClientCompositionRequestsTest_ThreeLayers::kDisplayDestinationClip(0, 0, 103,
+ 203);
TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, handlesNoClientCompostionLayers) {
EXPECT_CALL(mLayers[0].mOutputLayer, requiresClientComposition()).WillOnce(Return(false));
@@ -3789,13 +3802,15 @@
const Rect kPortraitFrame(0, 0, 1000, 2000);
const Rect kPortraitViewport(0, 0, 2000, 1000);
- const Rect kPortraitScissor(0, 0, 1000, 2000);
+ const Rect kPortraitSourceClip(0, 0, 1000, 2000);
+ const Rect kPortraitDestinationClip(0, 0, 1000, 2000);
const uint32_t kPortraitOrientation = TR_ROT_90;
constexpr ui::Dataspace kOutputDataspace = ui::Dataspace::DISPLAY_P3;
mOutput.mState.frame = kPortraitFrame;
mOutput.mState.viewport = kPortraitViewport;
- mOutput.mState.scissor = kPortraitScissor;
+ mOutput.mState.sourceClip = kPortraitSourceClip;
+ mOutput.mState.destinationClip = kPortraitDestinationClip;
mOutput.mState.transform = ui::Transform{kPortraitOrientation};
mOutput.mState.orientation = kPortraitOrientation;
mOutput.mState.needsFiltering = false;
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 4ae6dad..f3fe159 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -222,10 +222,13 @@
const bool needsFiltering =
(!globalTransform.preserveRects() || (type >= ui::Transform::SCALE));
- Rect scissor = globalTransform.transform(viewport);
- if (scissor.isEmpty()) {
- scissor = displayBounds;
+ Rect sourceClip = globalTransform.transform(viewport);
+ if (sourceClip.isEmpty()) {
+ sourceClip = displayBounds;
}
+ // For normal display use we always set the source and destination clip
+ // rectangles to the same values.
+ const Rect& destinationClip = sourceClip;
uint32_t transformOrientation;
@@ -236,8 +239,8 @@
transformOrientation = ui::Transform::toRotationFlags(orientation);
}
- getCompositionDisplay()->setProjection(globalTransform, transformOrientation,
- frame, viewport, scissor, needsFiltering);
+ getCompositionDisplay()->setProjection(globalTransform, transformOrientation, frame, viewport,
+ sourceClip, destinationClip, needsFiltering);
}
ui::Transform::RotationFlags DisplayDevice::getPrimaryDisplayRotationFlags() {
@@ -302,8 +305,8 @@
return mCompositionDisplay->getState().frame;
}
-const Rect& DisplayDevice::getScissor() const {
- return mCompositionDisplay->getState().scissor;
+const Rect& DisplayDevice::getSourceClip() const {
+ return mCompositionDisplay->getState().sourceClip;
}
bool DisplayDevice::hasWideColorGamut() const {
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index ff48ecd..75c709e 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -96,7 +96,7 @@
const ui::Transform& getTransform() const;
const Rect& getViewport() const;
const Rect& getFrame() const;
- const Rect& getScissor() const;
+ const Rect& getSourceClip() const;
bool needsFiltering() const;
uint32_t getLayerStack() const;
@@ -271,7 +271,7 @@
Rect getSourceCrop() const override {
// use the projected display viewport by default.
if (mSourceCrop.isEmpty()) {
- return mDisplay->getScissor();
+ return mDisplay->getSourceClip();
}
// Recompute the device transformation for the source crop.
@@ -280,14 +280,14 @@
ui::Transform translateLogical;
ui::Transform scale;
const Rect& viewport = mDisplay->getViewport();
- const Rect& scissor = mDisplay->getScissor();
+ const Rect& sourceClip = mDisplay->getSourceClip();
const Rect& frame = mDisplay->getFrame();
const auto flags = ui::Transform::toRotationFlags(mDisplay->getPhysicalOrientation());
rotation.set(flags, getWidth(), getHeight());
translateLogical.set(-viewport.left, -viewport.top);
- translatePhysical.set(scissor.left, scissor.top);
+ translatePhysical.set(sourceClip.left, sourceClip.top);
scale.set(frame.getWidth() / float(viewport.getWidth()), 0, 0,
frame.getHeight() / float(viewport.getHeight()));
const ui::Transform finalTransform =
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
index 9680a17..dddad92 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
@@ -1357,7 +1357,8 @@
mHardwareDisplaySize.height),
compositionState.transform);
EXPECT_EQ(TRANSFORM_FLAGS_ROT_0, compositionState.orientation);
- EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.scissor);
+ EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.sourceClip);
+ EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.destinationClip);
EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.frame);
EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.viewport);
EXPECT_EQ(false, compositionState.needsFiltering);
@@ -1369,7 +1370,8 @@
mHardwareDisplaySize.height),
compositionState.transform);
EXPECT_EQ(TRANSFORM_FLAGS_ROT_90, compositionState.orientation);
- EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.scissor);
+ EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.sourceClip);
+ EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.destinationClip);
// For 90, the frame and viewport have the hardware display size width and height swapped
EXPECT_EQ(Rect(SwapWH(mHardwareDisplaySize)), compositionState.frame);
EXPECT_EQ(Rect(SwapWH(mHardwareDisplaySize)), compositionState.viewport);
@@ -1382,7 +1384,8 @@
mHardwareDisplaySize.height),
compositionState.transform);
EXPECT_EQ(TRANSFORM_FLAGS_ROT_180, compositionState.orientation);
- EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.scissor);
+ EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.sourceClip);
+ EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.destinationClip);
EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.frame);
EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.viewport);
EXPECT_EQ(false, compositionState.needsFiltering);
@@ -1394,7 +1397,8 @@
mHardwareDisplaySize.height),
compositionState.transform);
EXPECT_EQ(TRANSFORM_FLAGS_ROT_270, compositionState.orientation);
- EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.scissor);
+ EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.sourceClip);
+ EXPECT_EQ(Rect(mHardwareDisplaySize), compositionState.destinationClip);
// For 270, the frame and viewport have the hardware display size width and height swapped
EXPECT_EQ(Rect(SwapWH(mHardwareDisplaySize)), compositionState.frame);
EXPECT_EQ(Rect(SwapWH(mHardwareDisplaySize)), compositionState.viewport);