SF: adapt region sampling to display orientation
Pass in the orientation flags of SF to RenderEngine when conducting the
sampling composition. This resulted mis-sampled areas, especially when
the region was outside of the clip of the 0-degree rotated display.
Bug: 132394665
Test: manual verification with 90, 270, 0 rotations
Test: new tests in libsurfaceflinger_unittest#RegionSamplingTest.*
Change-Id: I2869ef191572dbcc9170df8d3ed17414ab053ca4
diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp
index 3684260..7fa33f5 100644
--- a/services/surfaceflinger/RegionSamplingThread.cpp
+++ b/services/surfaceflinger/RegionSamplingThread.cpp
@@ -26,6 +26,8 @@
#include <utils/Trace.h>
#include <string>
+#include <compositionengine/Display.h>
+#include <compositionengine/impl/OutputCompositionState.h>
#include "DisplayDevice.h"
#include "Layer.h"
#include "SurfaceFlinger.h"
@@ -264,7 +266,22 @@
}
} // anonymous namespace
-float sampleArea(const uint32_t* data, int32_t stride, const Rect& area) {
+float sampleArea(const uint32_t* data, int32_t width, int32_t height, int32_t stride,
+ uint32_t orientation, const Rect& sample_area) {
+ if (!sample_area.isValid() || (sample_area.getWidth() > width) ||
+ (sample_area.getHeight() > height)) {
+ ALOGE("invalid sampling region requested");
+ return 0.0f;
+ }
+
+ // (b/133849373) ROT_90 screencap images produced upside down
+ auto area = sample_area;
+ if (orientation & ui::Transform::ROT_90) {
+ area.top = height - area.top;
+ area.bottom = height - area.bottom;
+ std::swap(area.top, area.bottom);
+ }
+
std::array<int32_t, 256> brightnessBuckets = {};
const int32_t majoritySampleNum = area.getWidth() * area.getHeight() / 2;
@@ -293,18 +310,21 @@
std::vector<float> RegionSamplingThread::sampleBuffer(
const sp<GraphicBuffer>& buffer, const Point& leftTop,
- const std::vector<RegionSamplingThread::Descriptor>& descriptors) {
+ const std::vector<RegionSamplingThread::Descriptor>& descriptors, uint32_t orientation) {
void* data_raw = nullptr;
buffer->lock(GRALLOC_USAGE_SW_READ_OFTEN, &data_raw);
std::shared_ptr<uint32_t> data(reinterpret_cast<uint32_t*>(data_raw),
[&buffer](auto) { buffer->unlock(); });
if (!data) return {};
+ const int32_t width = buffer->getWidth();
+ const int32_t height = buffer->getHeight();
const int32_t stride = buffer->getStride();
std::vector<float> lumas(descriptors.size());
std::transform(descriptors.begin(), descriptors.end(), lumas.begin(),
[&](auto const& descriptor) {
- return sampleArea(data.get(), stride, descriptor.area - leftTop);
+ return sampleArea(data.get(), width, height, stride, orientation,
+ descriptor.area - leftTop);
});
return lumas;
}
@@ -317,6 +337,11 @@
return;
}
+ const auto device = mFlinger.getDefaultDisplayDevice();
+ const auto display = device->getCompositionDisplay();
+ const auto state = display->getState();
+ const auto orientation = static_cast<ui::Transform::orientation_flags>(state.orientation);
+
std::vector<RegionSamplingThread::Descriptor> descriptors;
Region sampleRegion;
for (const auto& [listener, descriptor] : mDescriptors) {
@@ -326,10 +351,28 @@
const Rect sampledArea = sampleRegion.bounds();
- sp<const DisplayDevice> device = mFlinger.getDefaultDisplayDevice();
- DisplayRenderArea renderArea(device, sampledArea, sampledArea.getWidth(),
- sampledArea.getHeight(), ui::Dataspace::V0_SRGB,
- ui::Transform::ROT_0);
+ auto dx = 0;
+ auto dy = 0;
+ switch (orientation) {
+ case ui::Transform::ROT_90:
+ dx = device->getWidth();
+ break;
+ case ui::Transform::ROT_180:
+ dx = device->getWidth();
+ dy = device->getHeight();
+ break;
+ case ui::Transform::ROT_270:
+ dy = device->getHeight();
+ break;
+ default:
+ break;
+ }
+
+ ui::Transform t(orientation);
+ auto screencapRegion = t.transform(sampleRegion);
+ screencapRegion = screencapRegion.translate(dx, dy);
+ DisplayRenderArea renderArea(device, screencapRegion.bounds(), sampledArea.getWidth(),
+ sampledArea.getHeight(), ui::Dataspace::V0_SRGB, orientation);
std::unordered_set<sp<IRegionSamplingListener>, SpHash<IRegionSamplingListener>> listeners;
@@ -395,8 +438,8 @@
}
ALOGV("Sampling %zu descriptors", activeDescriptors.size());
- std::vector<float> lumas = sampleBuffer(buffer, sampledArea.leftTop(), activeDescriptors);
-
+ std::vector<float> lumas =
+ sampleBuffer(buffer, sampledArea.leftTop(), activeDescriptors, orientation);
if (lumas.size() != activeDescriptors.size()) {
ALOGW("collected %zu median luma values for %zu descriptors", lumas.size(),
activeDescriptors.size());
diff --git a/services/surfaceflinger/RegionSamplingThread.h b/services/surfaceflinger/RegionSamplingThread.h
index 08134e6..3c6fcf3 100644
--- a/services/surfaceflinger/RegionSamplingThread.h
+++ b/services/surfaceflinger/RegionSamplingThread.h
@@ -37,7 +37,8 @@
class SurfaceFlinger;
struct SamplingOffsetCallback;
-float sampleArea(const uint32_t* data, int32_t stride, const Rect& area);
+float sampleArea(const uint32_t* data, int32_t width, int32_t height, int32_t stride,
+ uint32_t orientation, const Rect& area);
class RegionSamplingThread : public IBinder::DeathRecipient {
public:
@@ -94,7 +95,7 @@
};
std::vector<float> sampleBuffer(
const sp<GraphicBuffer>& buffer, const Point& leftTop,
- const std::vector<RegionSamplingThread::Descriptor>& descriptors);
+ const std::vector<RegionSamplingThread::Descriptor>& descriptors, uint32_t orientation);
void doSample();
void binderDied(const wp<IBinder>& who) override;
diff --git a/services/surfaceflinger/tests/unittests/RegionSamplingTest.cpp b/services/surfaceflinger/tests/unittests/RegionSamplingTest.cpp
index 51d6d7e..160f041 100644
--- a/services/surfaceflinger/tests/unittests/RegionSamplingTest.cpp
+++ b/services/surfaceflinger/tests/unittests/RegionSamplingTest.cpp
@@ -17,6 +17,8 @@
#undef LOG_TAG
#define LOG_TAG "RegionSamplingTest"
+#include <ui/Transform.h>
+
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <array>
@@ -33,18 +35,21 @@
static int constexpr kWidth = 98;
static int constexpr kStride = 100;
static int constexpr kHeight = 29;
+ static int constexpr kOrientation = ui::Transform::ROT_0;
std::array<uint32_t, kHeight * kStride> buffer;
Rect const whole_area{0, 0, kWidth, kHeight};
};
TEST_F(RegionSamplingTest, calculate_mean_white) {
std::fill(buffer.begin(), buffer.end(), kWhite);
- EXPECT_THAT(sampleArea(buffer.data(), kStride, whole_area), testing::FloatEq(1.0f));
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, whole_area),
+ testing::FloatEq(1.0f));
}
TEST_F(RegionSamplingTest, calculate_mean_black) {
std::fill(buffer.begin(), buffer.end(), kBlack);
- EXPECT_THAT(sampleArea(buffer.data(), kStride, whole_area), testing::FloatEq(0.0f));
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, whole_area),
+ testing::FloatEq(0.0f));
}
TEST_F(RegionSamplingTest, calculate_mean_partial_region) {
@@ -54,7 +59,8 @@
whole_area.top + halfway_down};
std::fill(buffer.begin(), buffer.begin() + half, 0);
std::fill(buffer.begin() + half, buffer.end(), kWhite);
- EXPECT_THAT(sampleArea(buffer.data(), kStride, partial_region), testing::FloatEq(0.0f));
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, partial_region),
+ testing::FloatEq(0.0f));
}
TEST_F(RegionSamplingTest, calculate_mean_mixed_values) {
@@ -63,15 +69,71 @@
n++;
return pixel;
});
- EXPECT_THAT(sampleArea(buffer.data(), kStride, whole_area), testing::FloatNear(0.083f, 0.01f));
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, whole_area),
+ testing::FloatNear(0.083f, 0.01f));
}
TEST_F(RegionSamplingTest, bimodal_tiebreaker) {
std::generate(buffer.begin(), buffer.end(),
[n = 0]() mutable { return (n++ % 2) ? kBlack : kWhite; });
// presently there's no tiebreaking strategy in place, accept either of the means
- EXPECT_THAT(sampleArea(buffer.data(), kStride, whole_area),
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, whole_area),
testing::AnyOf(testing::FloatEq(1.0), testing::FloatEq(0.0f)));
}
+TEST_F(RegionSamplingTest, bounds_checking) {
+ std::generate(buffer.begin(), buffer.end(),
+ [n = 0]() mutable { return (n++ > (kStride * kHeight >> 1)) ? kBlack : kWhite; });
+
+ Rect invalid_region{0, 0, 4, kHeight + 1};
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, invalid_region),
+ testing::Eq(0.0));
+
+ invalid_region = Rect{0, 0, -4, kHeight};
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, invalid_region),
+ testing::Eq(0.0));
+
+ invalid_region = Rect{3, 0, 2, 0};
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, invalid_region),
+ testing::Eq(0.0));
+
+ invalid_region = Rect{0, 3, 0, 2};
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, invalid_region),
+ testing::Eq(0.0));
+}
+
+// workaround for b/133849373
+TEST_F(RegionSamplingTest, orientation_90) {
+ std::generate(buffer.begin(), buffer.end(),
+ [n = 0]() mutable { return (n++ > (kStride * kHeight >> 1)) ? kBlack : kWhite; });
+
+ Rect tl_region{0, 0, 4, 4};
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_0,
+ tl_region),
+ testing::Eq(1.0));
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_180,
+ tl_region),
+ testing::Eq(1.0));
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_90,
+ tl_region),
+ testing::Eq(0.0));
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_270,
+ tl_region),
+ testing::Eq(0.0));
+
+ Rect br_region{kWidth - 4, kHeight - 4, kWidth, kHeight};
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_0,
+ br_region),
+ testing::Eq(0.0));
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_180,
+ br_region),
+ testing::Eq(0.0));
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_90,
+ br_region),
+ testing::Eq(1.0));
+ EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_270,
+ br_region),
+ testing::Eq(1.0));
+}
+
} // namespace android