FramebufferSurface: fix for limitFramebufferSize
This CL fixes limitFramebufferSize and adds a test for the
broken use case. Also limitFramebufferSize is renamed to
limitSize because "Framebuffer" is implied by the class
name. The implementation is moved to a static
limitSizeInternal which can be tested more easily.
Bug: 180908183
Test: atest FramebufferSurfaceTest
Change-Id: Ic094b36fef6f9a17c87ec95a6c3f5f0a09c99d2f
diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
index f7fc162..8d685cf 100644
--- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
+++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
@@ -76,14 +76,14 @@
mConsumer->setConsumerUsageBits(GRALLOC_USAGE_HW_FB |
GRALLOC_USAGE_HW_RENDER |
GRALLOC_USAGE_HW_COMPOSER);
- const auto limitedSize = limitFramebufferSize(size);
+ const auto limitedSize = limitSize(size);
mConsumer->setDefaultBufferSize(limitedSize.width, limitedSize.height);
mConsumer->setMaxAcquiredBufferCount(
SurfaceFlinger::maxFrameBufferAcquiredBuffers - 1);
}
void FramebufferSurface::resizeBuffers(const ui::Size& newSize) {
- const auto limitedSize = limitFramebufferSize(newSize);
+ const auto limitedSize = limitSize(newSize);
mConsumer->setDefaultBufferSize(limitedSize.width, limitedSize.height);
}
@@ -179,19 +179,23 @@
}
}
-ui::Size FramebufferSurface::limitFramebufferSize(const ui::Size& size) {
+ui::Size FramebufferSurface::limitSize(const ui::Size& size) {
+ return limitSizeInternal(size, mMaxSize);
+}
+
+ui::Size FramebufferSurface::limitSizeInternal(const ui::Size& size, const ui::Size& maxSize) {
ui::Size limitedSize = size;
bool wasLimited = false;
- if (size.width > mMaxSize.width && mMaxSize.width != 0) {
+ if (size.width > maxSize.width && maxSize.width != 0) {
const float aspectRatio = static_cast<float>(size.width) / size.height;
- limitedSize.height = mMaxSize.width / aspectRatio;
- limitedSize.width = mMaxSize.width;
+ limitedSize.height = maxSize.width / aspectRatio;
+ limitedSize.width = maxSize.width;
wasLimited = true;
}
- if (size.height > mMaxSize.height && mMaxSize.height != 0) {
+ if (limitedSize.height > maxSize.height && maxSize.height != 0) {
const float aspectRatio = static_cast<float>(size.width) / size.height;
- limitedSize.height = mMaxSize.height;
- limitedSize.width = mMaxSize.height * aspectRatio;
+ limitedSize.height = maxSize.height;
+ limitedSize.width = maxSize.height * aspectRatio;
wasLimited = true;
}
ALOGI_IF(wasLimited, "framebuffer size has been limited to [%dx%d] from [%dx%d]",
diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
index 5d1e131..3123351 100644
--- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
+++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
@@ -55,15 +55,20 @@
virtual const sp<Fence>& getClientTargetAcquireFence() const override;
private:
+ friend class FramebufferSurfaceTest;
+
+ // Limits the width and height by the maximum width specified.
+ ui::Size limitSize(const ui::Size&);
+
+ // Used for testing purposes.
+ static ui::Size limitSizeInternal(const ui::Size&, const ui::Size& maxSize);
+
virtual ~FramebufferSurface() { }; // this class cannot be overloaded
virtual void freeBufferLocked(int slotIndex);
virtual void dumpLocked(String8& result, const char* prefix) const;
- // Limits the width and height by the maximum width specified in the constructor.
- ui::Size limitFramebufferSize(const ui::Size&);
-
// nextBuffer waits for and then latches the next buffer from the
// BufferQueue and releases the previously latched buffer to the
// BufferQueue. The new buffer is returned in the 'buffer' argument.
diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp
index 3c1b9d8..9f94c73 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -55,6 +55,7 @@
"EventThreadTest.cpp",
"FpsReporterTest.cpp",
"FpsTest.cpp",
+ "FramebufferSurfaceTest.cpp",
"FrameTimelineTest.cpp",
"HWComposerTest.cpp",
"OneShotTimerTest.cpp",
diff --git a/services/surfaceflinger/tests/unittests/FramebufferSurfaceTest.cpp b/services/surfaceflinger/tests/unittests/FramebufferSurfaceTest.cpp
new file mode 100644
index 0000000..b8df640
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/FramebufferSurfaceTest.cpp
@@ -0,0 +1,40 @@
+/*
+ * Copyright 2021 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.
+ */
+
+#include "DisplayHardware/FramebufferSurface.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+namespace android {
+
+class FramebufferSurfaceTest : public testing::Test {
+public:
+ ui::Size limitSize(const ui::Size& size, const ui::Size maxSize) {
+ return FramebufferSurface::limitSizeInternal(size, maxSize);
+ }
+};
+
+TEST_F(FramebufferSurfaceTest, limitSize) {
+ const ui::Size kMaxSize(1920, 1080);
+ EXPECT_EQ(ui::Size(1920, 1080), limitSize({3840, 2160}, kMaxSize));
+ EXPECT_EQ(ui::Size(1920, 1080), limitSize({1920, 1080}, kMaxSize));
+ EXPECT_EQ(ui::Size(1920, 1012), limitSize({4096, 2160}, kMaxSize));
+ EXPECT_EQ(ui::Size(1080, 1080), limitSize({3840, 3840}, kMaxSize));
+ EXPECT_EQ(ui::Size(1280, 720), limitSize({1280, 720}, kMaxSize));
+}
+
+} // namespace android