Revert "Use renderengine::drawLayers api everywhere."
This reverts commit 0f7148365cfa405e8ed802b33c0e2a7b790fcf42.
Reason for revert: b/123878751, preparing while i investigate
Bug: 123878751
Test: bug no longer repros
Change-Id: I6c77427f0c113f96b68aec479ccec52b0bbb2c6b
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
index 84b2423..a7ab472 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
@@ -97,9 +97,10 @@
// TODO(lpique): Make this protected once it is only internally called.
virtual OutputCompositionState& editState() = 0;
- // Gets the dirty region in layer stack space.
- // If repaintEverything is true, this will be the full display bounds.
- virtual Region getDirtyRegion(bool repaintEverything) const = 0;
+ // Gets the physical space dirty region. If repaintEverything is true, this
+ // will be the full display bounds. Internally the dirty region is stored in
+ // logical (aka layer stack) space.
+ virtual Region getPhysicalSpaceDirtyRegion(bool repaintEverything) const = 0;
// Tests whether a given layerStackId belongs in this output.
// A layer belongs to the output if its layerStackId matches the of the output layerStackId,
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/RenderSurface.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/RenderSurface.h
index ddeb730..dd01b05 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/RenderSurface.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/RenderSurface.h
@@ -75,13 +75,16 @@
// Allocates a buffer as scratch space for GPU composition
virtual sp<GraphicBuffer> dequeueBuffer() = 0;
- // Queues the drawn buffer for consumption by HWC. readyFence is the fence
- // which will fire when the buffer is ready for consumption.
- virtual void queueBuffer(base::unique_fd&& readyFence) = 0;
+ // Queues the drawn buffer for consumption by HWC
+ virtual void queueBuffer() = 0;
// Called after the HWC calls are made to present the display
virtual void onPresentDisplayCompleted() = 0;
+ // Marks the current buffer has finished, so that it can be presented and
+ // swapped out
+ virtual void finishBuffer() = 0;
+
// Called to set the viewport and projection state for rendering into this
// surface
virtual void setViewportAndProjection() = 0;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
index 3fd057c..d876c03 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
@@ -61,7 +61,7 @@
const OutputCompositionState& getState() const override;
OutputCompositionState& editState() override;
- Region getDirtyRegion(bool repaintEverything) const override;
+ Region getPhysicalSpaceDirtyRegion(bool repaintEverything) const override;
bool belongsInOutput(uint32_t, bool) const override;
compositionengine::OutputLayer* getOutputLayerForLayer(
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/RenderSurface.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/RenderSurface.h
index 2f0fceb..0489310 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/RenderSurface.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/RenderSurface.h
@@ -53,8 +53,9 @@
status_t beginFrame(bool mustRecompose) override;
status_t prepareFrame(std::vector<CompositionInfo>& compositionData) override;
sp<GraphicBuffer> dequeueBuffer() override;
- void queueBuffer(base::unique_fd&& readyFence) override;
+ void queueBuffer() override;
void onPresentDisplayCompleted() override;
+ void finishBuffer() override;
void setViewportAndProjection() override;
void flip() override;
@@ -76,6 +77,9 @@
const sp<ANativeWindow> mNativeWindow;
// Current buffer being rendered into
sp<GraphicBuffer> mGraphicBuffer;
+ // File descriptor indicating that mGraphicBuffer is ready for display, i.e.
+ // that drawing to the buffer is now complete.
+ base::unique_fd mBufferReady;
const sp<DisplaySurface> mDisplaySurface;
ui::Size mSize;
std::uint32_t mPageFlipCount{0};
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
index 2972ad7..e79a44c 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
@@ -56,7 +56,7 @@
MOCK_CONST_METHOD0(getState, const OutputCompositionState&());
MOCK_METHOD0(editState, OutputCompositionState&());
- MOCK_CONST_METHOD1(getDirtyRegion, Region(bool));
+ MOCK_CONST_METHOD1(getPhysicalSpaceDirtyRegion, Region(bool));
MOCK_CONST_METHOD2(belongsInOutput, bool(uint32_t, bool));
MOCK_CONST_METHOD1(getOutputLayerForLayer,
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/RenderSurface.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/RenderSurface.h
index e9ff330..2269e57 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/RenderSurface.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/RenderSurface.h
@@ -39,8 +39,9 @@
MOCK_METHOD1(beginFrame, status_t(bool mustRecompose));
MOCK_METHOD1(prepareFrame, status_t(std::vector<CompositionInfo>& compositionData));
MOCK_METHOD0(dequeueBuffer, sp<GraphicBuffer>());
- MOCK_METHOD1(queueBuffer, void(base::unique_fd&&));
+ MOCK_METHOD0(queueBuffer, void());
MOCK_METHOD0(onPresentDisplayCompleted, void());
+ MOCK_METHOD0(finishBuffer, void());
MOCK_METHOD0(setViewportAndProjection, void());
MOCK_METHOD0(flip, void());
MOCK_CONST_METHOD1(dump, void(std::string& result));
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index f6c1a33..6d5147d 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -170,10 +170,13 @@
return mState;
}
-Region Output::getDirtyRegion(bool repaintEverything) const {
- Region dirty(mState.viewport);
- if (!repaintEverything) {
- dirty.andSelf(mState.dirtyRegion);
+Region Output::getPhysicalSpaceDirtyRegion(bool repaintEverything) const {
+ Region dirty;
+ if (repaintEverything) {
+ dirty.set(mState.bounds);
+ } else {
+ dirty = mState.transform.transform(mState.dirtyRegion);
+ dirty.andSelf(mState.bounds);
}
return dirty;
}
diff --git a/services/surfaceflinger/CompositionEngine/src/RenderSurface.cpp b/services/surfaceflinger/CompositionEngine/src/RenderSurface.cpp
index 3f841d2..d546fc8 100644
--- a/services/surfaceflinger/CompositionEngine/src/RenderSurface.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/RenderSurface.cpp
@@ -156,7 +156,7 @@
return mGraphicBuffer;
}
-void RenderSurface::queueBuffer(base::unique_fd&& readyFence) {
+void RenderSurface::queueBuffer() {
auto& hwc = mCompositionEngine.getHwComposer();
const auto id = mDisplay.getId();
@@ -178,9 +178,9 @@
if (mGraphicBuffer == nullptr) {
ALOGE("No buffer is ready for display [%s]", mDisplay.getName().c_str());
} else {
- status_t result =
- mNativeWindow->queueBuffer(mNativeWindow.get(),
- mGraphicBuffer->getNativeBuffer(), dup(readyFence));
+ status_t result = mNativeWindow->queueBuffer(mNativeWindow.get(),
+ mGraphicBuffer->getNativeBuffer(),
+ dup(mBufferReady));
if (result != NO_ERROR) {
ALOGE("Error when queueing buffer for display [%s]: %d", mDisplay.getName().c_str(),
result);
@@ -190,10 +190,12 @@
LOG_ALWAYS_FATAL("ANativeWindow::queueBuffer failed with error: %d", result);
} else {
mNativeWindow->cancelBuffer(mNativeWindow.get(),
- mGraphicBuffer->getNativeBuffer(), dup(readyFence));
+ mGraphicBuffer->getNativeBuffer(),
+ dup(mBufferReady));
}
}
+ mBufferReady.reset();
mGraphicBuffer = nullptr;
}
}
@@ -215,6 +217,14 @@
ui::Transform::ROT_0);
}
+void RenderSurface::finishBuffer() {
+ auto& renderEngine = mCompositionEngine.getRenderEngine();
+ mBufferReady = renderEngine.flush();
+ if (mBufferReady.get() < 0) {
+ renderEngine.finish();
+ }
+}
+
void RenderSurface::flip() {
mPageFlipCount++;
}
@@ -253,5 +263,9 @@
return mGraphicBuffer;
}
+base::unique_fd& RenderSurface::mutableBufferReadyForTest() {
+ return mBufferReady;
+}
+
} // namespace impl
} // namespace android::compositionengine
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
index 314d98b..cc1211b 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
@@ -215,32 +215,52 @@
}
/* ------------------------------------------------------------------------
- * Output::getDirtyRegion()
+ * Output::getPhysicalSpaceDirtyRegion()
*/
-TEST_F(OutputTest, getDirtyRegionWithRepaintEverythingTrue) {
- const Rect viewport{100, 200};
- mOutput.editState().viewport = viewport;
+TEST_F(OutputTest, getPhysicalSpaceDirtyRegionWithRepaintEverythingTrue) {
+ const Rect displaySize{100, 200};
+ mOutput.editState().bounds = displaySize;
mOutput.editState().dirtyRegion.set(50, 300);
{
- Region result = mOutput.getDirtyRegion(true);
+ Region result = mOutput.getPhysicalSpaceDirtyRegion(true);
- EXPECT_THAT(result, RegionEq(Region(viewport)));
+ EXPECT_THAT(result, RegionEq(Region(displaySize)));
+ }
+
+ // For repaint everything == true, the returned value does not depend on the display
+ // rotation.
+ mOutput.editState().transform.set(ui::Transform::ROT_90, 0, 0);
+
+ {
+ Region result = mOutput.getPhysicalSpaceDirtyRegion(true);
+
+ EXPECT_THAT(result, RegionEq(Region(displaySize)));
}
}
-TEST_F(OutputTest, getDirtyRegionWithRepaintEverythingFalse) {
- const Rect viewport{100, 200};
- mOutput.editState().viewport = viewport;
+TEST_F(OutputTest, getPhysicalSpaceDirtyRegionWithRepaintEverythingFalse) {
+ const Rect displaySize{100, 200};
+ mOutput.editState().bounds = displaySize;
mOutput.editState().dirtyRegion.set(50, 300);
{
- Region result = mOutput.getDirtyRegion(false);
+ Region result = mOutput.getPhysicalSpaceDirtyRegion(false);
// The dirtyRegion should be clipped to the display bounds.
EXPECT_THAT(result, RegionEq(Region(Rect(50, 200))));
}
+
+ mOutput.editState().transform.set(ui::Transform::ROT_90, displaySize.getWidth(),
+ displaySize.getHeight());
+
+ {
+ Region result = mOutput.getPhysicalSpaceDirtyRegion(false);
+
+ // The dirtyRegion should be rotated and clipped to the display bounds.
+ EXPECT_THAT(result, RegionEq(Region(Rect(100, 50))));
+ }
}
/* ------------------------------------------------------------------------
diff --git a/services/surfaceflinger/CompositionEngine/tests/RenderSurfaceTest.cpp b/services/surfaceflinger/CompositionEngine/tests/RenderSurfaceTest.cpp
index c56d92a..13cc663 100644
--- a/services/surfaceflinger/CompositionEngine/tests/RenderSurfaceTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/RenderSurfaceTest.cpp
@@ -373,7 +373,7 @@
.WillOnce(Return(false));
EXPECT_CALL(*mDisplaySurface, advanceFrame()).Times(1);
- mSurface.queueBuffer(base::unique_fd());
+ mSurface.queueBuffer();
EXPECT_EQ(buffer.get(), mSurface.mutableGraphicBufferForTest().get());
}
@@ -387,7 +387,7 @@
.WillOnce(Return(NO_ERROR));
EXPECT_CALL(*mDisplaySurface, advanceFrame()).Times(1);
- mSurface.queueBuffer(base::unique_fd());
+ mSurface.queueBuffer();
EXPECT_EQ(nullptr, mSurface.mutableGraphicBufferForTest().get());
}
@@ -402,7 +402,7 @@
.WillOnce(Return(NO_ERROR));
EXPECT_CALL(*mDisplaySurface, advanceFrame()).Times(1);
- mSurface.queueBuffer(base::unique_fd());
+ mSurface.queueBuffer();
EXPECT_EQ(nullptr, mSurface.mutableGraphicBufferForTest().get());
}
@@ -419,7 +419,7 @@
.WillOnce(Return(NO_ERROR));
EXPECT_CALL(*mDisplaySurface, advanceFrame()).Times(1);
- mSurface.queueBuffer(base::unique_fd());
+ mSurface.queueBuffer();
EXPECT_EQ(nullptr, mSurface.mutableGraphicBufferForTest().get());
}
@@ -436,7 +436,7 @@
.WillOnce(Return(NO_ERROR));
EXPECT_CALL(*mDisplaySurface, advanceFrame()).Times(1);
- mSurface.queueBuffer(base::unique_fd());
+ mSurface.queueBuffer();
EXPECT_EQ(nullptr, mSurface.mutableGraphicBufferForTest().get());
}
@@ -452,6 +452,29 @@
}
/* ------------------------------------------------------------------------
+ * RenderSurface::finishBuffer()
+ */
+
+TEST_F(RenderSurfaceTest, finishBufferJustFlushesRenderEngine) {
+ int fd = dup(1);
+
+ EXPECT_CALL(mRenderEngine, flush()).WillOnce(Return(ByMove(base::unique_fd(fd))));
+
+ mSurface.finishBuffer();
+
+ EXPECT_EQ(fd, mSurface.mutableBufferReadyForTest().release());
+}
+
+TEST_F(RenderSurfaceTest, finishBufferFlushesAndFinishesRenderEngine) {
+ EXPECT_CALL(mRenderEngine, flush()).WillOnce(Return(ByMove(base::unique_fd(-2))));
+ EXPECT_CALL(mRenderEngine, finish()).Times(1);
+
+ mSurface.finishBuffer();
+
+ EXPECT_EQ(-2, mSurface.mutableBufferReadyForTest().release());
+}
+
+/* ------------------------------------------------------------------------
* RenderSurface::setViewportAndProjection()
*/