Merge "Address Pointer Capture crash during rapid toggles" into tm-dev
diff --git a/include/android/input.h b/include/android/input.h
index fb5e204..38b27bc 100644
--- a/include/android/input.h
+++ b/include/android/input.h
@@ -810,7 +810,7 @@
/**
* Constants that identify different gesture classification types.
*/
-enum {
+enum AMotionClassification : uint32_t {
/**
* Classification constant: None.
*
@@ -820,7 +820,8 @@
/**
* Classification constant: Ambiguous gesture.
*
- * The user's intent with respect to the current event stream is not yet determined.
+ * The user's intent with respect to the current event stream is not yet determined. Events
+ * starting in AMBIGUOUS_GESTURE will eventually resolve into either DEEP_PRESS or NONE.
* Gestural actions, such as scrolling, should be inhibited until the classification resolves
* to another value or the event stream ends.
*/
@@ -1357,8 +1358,17 @@
* Get the action button for the motion event. Returns a valid action button when the
* event is associated with a button press or button release action. For other actions
* the return value is undefined.
+ *
+ * @see #AMOTION_EVENT_BUTTON_PRIMARY
+ * @see #AMOTION_EVENT_BUTTON_SECONDARY
+ * @see #AMOTION_EVENT_BUTTON_TERTIARY
+ * @see #AMOTION_EVENT_BUTTON_BACK
+ * @see #AMOTION_EVENT_BUTTON_FORWARD
+ * @see #AMOTION_EVENT_BUTTON_STYLUS_PRIMARY
+ * @see #AMOTION_EVENT_BUTTON_STYLUS_SECONDARY
*/
-int32_t AMotionEvent_getActionButton(const AInputEvent* motion_event);
+int32_t AMotionEvent_getActionButton(const AInputEvent* motion_event)
+ __INTRODUCED_IN(__ANDROID_API_T__);
/**
* Returns the classification for the current gesture.
@@ -1368,7 +1378,8 @@
* @see #AMOTION_EVENT_CLASSIFICATION_AMBIGUOUS_GESTURE
* @see #AMOTION_EVENT_CLASSIFICATION_DEEP_PRESS
*/
-int32_t AMotionEvent_getClassification(const AInputEvent* motion_event);
+int32_t AMotionEvent_getClassification(const AInputEvent* motion_event)
+ __INTRODUCED_IN(__ANDROID_API_T__);
/**
* Creates a native AInputEvent* object that is a copy of the specified Java
diff --git a/libs/battery/MultiStateCounter.h b/libs/battery/MultiStateCounter.h
index 0caf005..ce9cd1c 100644
--- a/libs/battery/MultiStateCounter.h
+++ b/libs/battery/MultiStateCounter.h
@@ -139,22 +139,35 @@
return;
}
- if (!enabled) {
+ if (isEnabled) {
// Confirm the current state for the side-effect of updating the time-in-state
// counter for the current state.
setState(currentState, timestamp);
- }
+ isEnabled = false;
+ } else {
+ // If the counter is being enabled with an out-of-order timestamp, just push back
+ // the timestamp to avoid having the situation where
+ // timeInStateSinceUpdate > timeSinceUpdate
+ if (timestamp < lastUpdateTimestamp) {
+ timestamp = lastUpdateTimestamp;
+ }
- isEnabled = enabled;
-
- if (lastStateChangeTimestamp >= 0) {
- lastStateChangeTimestamp = timestamp;
+ if (lastStateChangeTimestamp >= 0) {
+ lastStateChangeTimestamp = timestamp;
+ }
+ isEnabled = true;
}
}
template <class T>
void MultiStateCounter<T>::setState(state_t state, time_t timestamp) {
- if (isEnabled && lastStateChangeTimestamp >= 0) {
+ if (isEnabled && lastStateChangeTimestamp >= 0 && lastUpdateTimestamp >= 0) {
+ // If the update arrived out-of-order, just push back the timestamp to
+ // avoid having the situation where timeInStateSinceUpdate > timeSinceUpdate
+ if (timestamp < lastUpdateTimestamp) {
+ timestamp = lastUpdateTimestamp;
+ }
+
if (timestamp >= lastStateChangeTimestamp) {
states[currentState].timeInStateSinceUpdate += timestamp - lastStateChangeTimestamp;
} else {
@@ -185,6 +198,12 @@
// If the counter is disabled, we ignore the update, except when the counter got disabled after
// the previous update, in which case we still need to pick up the residual delta.
if (isEnabled || lastUpdateTimestamp < lastStateChangeTimestamp) {
+ // If the update arrived out of order, just push back the timestamp to
+ // avoid having the situation where timeInStateSinceUpdate > timeSinceUpdate
+ if (timestamp < lastStateChangeTimestamp) {
+ timestamp = lastStateChangeTimestamp;
+ }
+
// Confirm the current state for the side-effect of updating the time-in-state
// counter for the current state.
setState(currentState, timestamp);
diff --git a/libs/gui/ScreenCaptureResults.cpp b/libs/gui/ScreenCaptureResults.cpp
index e91f74f..fe38706 100644
--- a/libs/gui/ScreenCaptureResults.cpp
+++ b/libs/gui/ScreenCaptureResults.cpp
@@ -36,6 +36,7 @@
}
SAFE_PARCEL(parcel->writeBool, capturedSecureLayers);
+ SAFE_PARCEL(parcel->writeBool, capturedHdrLayers);
SAFE_PARCEL(parcel->writeUint32, static_cast<uint32_t>(capturedDataspace));
SAFE_PARCEL(parcel->writeInt32, result);
return NO_ERROR;
@@ -57,6 +58,7 @@
}
SAFE_PARCEL(parcel->readBool, &capturedSecureLayers);
+ SAFE_PARCEL(parcel->readBool, &capturedHdrLayers);
uint32_t dataspace = 0;
SAFE_PARCEL(parcel->readUint32, &dataspace);
capturedDataspace = static_cast<ui::Dataspace>(dataspace);
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 7a63af0..6642ec6 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -1495,6 +1495,18 @@
s->bufferData = std::move(bufferData);
registerSurfaceControlForCallback(sc);
+ // With the current infrastructure, a release callback will not be invoked if there's no
+ // transaction callback in the case when a buffer is latched and not released early. This is
+ // because the legacy implementation didn't have a release callback and sent releases in the
+ // transaction callback. Because of this, we need to make sure to have a transaction callback
+ // set up when a buffer is sent in a transaction to ensure the caller gets the release
+ // callback, regardless if they set up a transaction callback.
+ //
+ // TODO (b/230380821): Remove when release callbacks are separated from transaction callbacks
+ addTransactionCompletedCallback([](void*, nsecs_t, const sp<Fence>&,
+ const std::vector<SurfaceControlStats>&) {},
+ nullptr);
+
mContainsBuffer = true;
return *this;
}
diff --git a/libs/gui/include/gui/ScreenCaptureResults.h b/libs/gui/include/gui/ScreenCaptureResults.h
index 99c35c1..724c11c 100644
--- a/libs/gui/include/gui/ScreenCaptureResults.h
+++ b/libs/gui/include/gui/ScreenCaptureResults.h
@@ -33,6 +33,7 @@
sp<GraphicBuffer> buffer;
sp<Fence> fence = Fence::NO_FENCE;
bool capturedSecureLayers{false};
+ bool capturedHdrLayers{false};
ui::Dataspace capturedDataspace{ui::Dataspace::V0_SRGB};
status_t result = OK;
};
diff --git a/libs/renderengine/include/renderengine/DisplaySettings.h b/libs/renderengine/include/renderengine/DisplaySettings.h
index a5e0879..59ef991 100644
--- a/libs/renderengine/include/renderengine/DisplaySettings.h
+++ b/libs/renderengine/include/renderengine/DisplaySettings.h
@@ -21,6 +21,7 @@
#include <iosfwd>
#include <math/mat4.h>
+#include <renderengine/PrintMatrix.h>
#include <ui/GraphicTypes.h>
#include <ui/Rect.h>
#include <ui/Region.h>
@@ -82,11 +83,38 @@
static inline bool operator==(const DisplaySettings& lhs, const DisplaySettings& rhs) {
return lhs.physicalDisplay == rhs.physicalDisplay && lhs.clip == rhs.clip &&
- lhs.maxLuminance == rhs.maxLuminance && lhs.outputDataspace == rhs.outputDataspace &&
- lhs.colorTransform == rhs.colorTransform && lhs.orientation == rhs.orientation;
+ lhs.maxLuminance == rhs.maxLuminance &&
+ lhs.currentLuminanceNits == rhs.currentLuminanceNits &&
+ lhs.outputDataspace == rhs.outputDataspace &&
+ lhs.colorTransform == rhs.colorTransform &&
+ lhs.deviceHandlesColorTransform == rhs.deviceHandlesColorTransform &&
+ lhs.orientation == rhs.orientation &&
+ lhs.targetLuminanceNits == rhs.targetLuminanceNits &&
+ lhs.dimmingStage == rhs.dimmingStage && lhs.renderIntent == rhs.renderIntent;
}
-// Defining PrintTo helps with Google Tests.
+static const char* orientation_to_string(uint32_t orientation) {
+ switch (orientation) {
+ case ui::Transform::ROT_0:
+ return "ROT_0";
+ case ui::Transform::FLIP_H:
+ return "FLIP_H";
+ case ui::Transform::FLIP_V:
+ return "FLIP_V";
+ case ui::Transform::ROT_90:
+ return "ROT_90";
+ case ui::Transform::ROT_180:
+ return "ROT_180";
+ case ui::Transform::ROT_270:
+ return "ROT_270";
+ case ui::Transform::ROT_INVALID:
+ return "ROT_INVALID";
+ default:
+ ALOGE("invalid orientation!");
+ return "invalid orientation";
+ }
+}
+
static inline void PrintTo(const DisplaySettings& settings, ::std::ostream* os) {
*os << "DisplaySettings {";
*os << "\n .physicalDisplay = ";
@@ -94,10 +122,19 @@
*os << "\n .clip = ";
PrintTo(settings.clip, os);
*os << "\n .maxLuminance = " << settings.maxLuminance;
+ *os << "\n .currentLuminanceNits = " << settings.currentLuminanceNits;
*os << "\n .outputDataspace = ";
PrintTo(settings.outputDataspace, os);
- *os << "\n .colorTransform = " << settings.colorTransform;
- *os << "\n .clearRegion = ";
+ *os << "\n .colorTransform = ";
+ PrintMatrix(settings.colorTransform, os);
+ *os << "\n .deviceHandlesColorTransform = " << settings.deviceHandlesColorTransform;
+ *os << "\n .orientation = " << orientation_to_string(settings.orientation);
+ *os << "\n .targetLuminanceNits = " << settings.targetLuminanceNits;
+ *os << "\n .dimmingStage = "
+ << aidl::android::hardware::graphics::composer3::toString(settings.dimmingStage).c_str();
+ *os << "\n .renderIntent = "
+ << aidl::android::hardware::graphics::composer3::toString(settings.renderIntent).c_str();
+ *os << "\n}";
}
} // namespace renderengine
diff --git a/libs/renderengine/include/renderengine/LayerSettings.h b/libs/renderengine/include/renderengine/LayerSettings.h
index 171cbaa..154e526 100644
--- a/libs/renderengine/include/renderengine/LayerSettings.h
+++ b/libs/renderengine/include/renderengine/LayerSettings.h
@@ -19,7 +19,9 @@
#include <math/mat4.h>
#include <math/vec3.h>
#include <renderengine/ExternalTexture.h>
+#include <renderengine/PrintMatrix.h>
#include <ui/BlurRegion.h>
+#include <ui/DebugUtils.h>
#include <ui/Fence.h>
#include <ui/FloatRect.h>
#include <ui/GraphicBuffer.h>
@@ -208,6 +210,10 @@
lhs.casterIsTranslucent == rhs.casterIsTranslucent;
}
+static inline bool operator!=(const ShadowSettings& lhs, const ShadowSettings& rhs) {
+ return !(operator==(lhs, rhs));
+}
+
static inline bool operator==(const LayerSettings& lhs, const LayerSettings& rhs) {
if (lhs.blurRegions.size() != rhs.blurRegions.size()) {
return false;
@@ -226,18 +232,19 @@
lhs.skipContentDraw == rhs.skipContentDraw && lhs.shadow == rhs.shadow &&
lhs.backgroundBlurRadius == rhs.backgroundBlurRadius &&
lhs.blurRegionTransform == rhs.blurRegionTransform &&
- lhs.stretchEffect == rhs.stretchEffect;
+ lhs.stretchEffect == rhs.stretchEffect && lhs.whitePointNits == rhs.whitePointNits;
}
-// Defining PrintTo helps with Google Tests.
-
static inline void PrintTo(const Buffer& settings, ::std::ostream* os) {
*os << "Buffer {";
- *os << "\n .buffer = " << settings.buffer.get();
+ *os << "\n .buffer = " << settings.buffer.get() << " "
+ << (settings.buffer.get() ? decodePixelFormat(settings.buffer->getPixelFormat()).c_str()
+ : "");
*os << "\n .fence = " << settings.fence.get();
*os << "\n .textureName = " << settings.textureName;
*os << "\n .useTextureFiltering = " << settings.useTextureFiltering;
- *os << "\n .textureTransform = " << settings.textureTransform;
+ *os << "\n .textureTransform = ";
+ PrintMatrix(settings.textureTransform, os);
*os << "\n .usePremultipliedAlpha = " << settings.usePremultipliedAlpha;
*os << "\n .isOpaque = " << settings.isOpaque;
*os << "\n .isY410BT2020 = " << settings.isY410BT2020;
@@ -249,7 +256,8 @@
*os << "Geometry {";
*os << "\n .boundaries = ";
PrintTo(settings.boundaries, os);
- *os << "\n .positionTransform = " << settings.positionTransform;
+ *os << "\n .positionTransform = ";
+ PrintMatrix(settings.positionTransform, os);
*os << "\n .roundedCornersRadius = " << settings.roundedCornersRadius;
*os << "\n .roundedCornersCrop = ";
PrintTo(settings.roundedCornersCrop, os);
@@ -258,10 +266,14 @@
static inline void PrintTo(const PixelSource& settings, ::std::ostream* os) {
*os << "PixelSource {";
- *os << "\n .buffer = ";
- PrintTo(settings.buffer, os);
- *os << "\n .solidColor = " << settings.solidColor;
- *os << "\n}";
+ if (settings.buffer.buffer) {
+ *os << "\n .buffer = ";
+ PrintTo(settings.buffer, os);
+ *os << "\n}";
+ } else {
+ *os << "\n .solidColor = " << settings.solidColor;
+ *os << "\n}";
+ }
}
static inline void PrintTo(const ShadowSettings& settings, ::std::ostream* os) {
@@ -301,18 +313,28 @@
*os << "\n .alpha = " << settings.alpha;
*os << "\n .sourceDataspace = ";
PrintTo(settings.sourceDataspace, os);
- *os << "\n .colorTransform = " << settings.colorTransform;
+ *os << "\n .colorTransform = ";
+ PrintMatrix(settings.colorTransform, os);
*os << "\n .disableBlending = " << settings.disableBlending;
*os << "\n .skipContentDraw = " << settings.skipContentDraw;
- *os << "\n .backgroundBlurRadius = " << settings.backgroundBlurRadius;
- for (auto blurRegion : settings.blurRegions) {
- *os << "\n";
- PrintTo(blurRegion, os);
+ if (settings.shadow != ShadowSettings()) {
+ *os << "\n .shadow = ";
+ PrintTo(settings.shadow, os);
}
- *os << "\n .shadow = ";
- PrintTo(settings.shadow, os);
- *os << "\n .stretchEffect = ";
- PrintTo(settings.stretchEffect, os);
+ *os << "\n .backgroundBlurRadius = " << settings.backgroundBlurRadius;
+ if (settings.blurRegions.size()) {
+ *os << "\n .blurRegions =";
+ for (auto blurRegion : settings.blurRegions) {
+ *os << "\n";
+ PrintTo(blurRegion, os);
+ }
+ }
+ *os << "\n .blurRegionTransform = ";
+ PrintMatrix(settings.blurRegionTransform, os);
+ if (settings.stretchEffect != StretchEffect()) {
+ *os << "\n .stretchEffect = ";
+ PrintTo(settings.stretchEffect, os);
+ }
*os << "\n .whitePointNits = " << settings.whitePointNits;
*os << "\n}";
}
diff --git a/libs/renderengine/include/renderengine/PrintMatrix.h b/libs/renderengine/include/renderengine/PrintMatrix.h
new file mode 100644
index 0000000..11a5c21
--- /dev/null
+++ b/libs/renderengine/include/renderengine/PrintMatrix.h
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2022 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.
+ */
+
+#pragma once
+
+#include <math/mat4.h>
+#include <iosfwd>
+
+namespace android::renderengine {
+
+// This method simplifies printing the identity matrix, so it can be easily
+// skipped over visually.
+inline void PrintMatrix(const mat4& matrix, ::std::ostream* os) {
+ if (matrix == mat4()) {
+ *os << "I";
+ } else {
+ // Print the matrix starting on a new line. This ensures that all lines
+ // are aligned.
+ *os << "\n" << matrix;
+ }
+}
+
+} // namespace android::renderengine
diff --git a/libs/renderengine/skia/Cache.cpp b/libs/renderengine/skia/Cache.cpp
index 5c137a4..f3064f3 100644
--- a/libs/renderengine/skia/Cache.cpp
+++ b/libs/renderengine/skia/Cache.cpp
@@ -439,7 +439,7 @@
const nsecs_t timeAfter = systemTime();
const float compileTimeMs = static_cast<float>(timeAfter - timeBefore) / 1.0E6;
- const int shadersCompiled = renderengine->reportShadersCompiled();
+ const int shadersCompiled = renderengine->reportShadersCompiled() - previousCount;
ALOGD("Shader cache generated %d shaders in %f ms\n", shadersCompiled, compileTimeMs);
}
}
diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.cpp b/libs/renderengine/skia/SkiaGLRenderEngine.cpp
index f440f4a..97271cb 100644
--- a/libs/renderengine/skia/SkiaGLRenderEngine.cpp
+++ b/libs/renderengine/skia/SkiaGLRenderEngine.cpp
@@ -67,7 +67,7 @@
namespace {
// Debugging settings
static const bool kPrintLayerSettings = false;
-static const bool kFlushAfterEveryLayer = false;
+static const bool kFlushAfterEveryLayer = kPrintLayerSettings;
} // namespace
bool checkGlError(const char* op, int lineNumber);
@@ -296,14 +296,8 @@
ATRACE_FORMAT("SF cache: %i shaders", mTotalShadersCompiled);
}
-void SkiaGLRenderEngine::assertShadersCompiled(int numShaders) {
- const int cached = mSkSLCacheMonitor.shadersCachedSinceLastCall();
- LOG_ALWAYS_FATAL_IF(cached != numShaders, "Attempted to cache %i shaders; cached %i",
- numShaders, cached);
-}
-
int SkiaGLRenderEngine::reportShadersCompiled() {
- return mSkSLCacheMonitor.shadersCachedSinceLastCall();
+ return mSkSLCacheMonitor.totalShadersCompiled();
}
SkiaGLRenderEngine::SkiaGLRenderEngine(const RenderEngineCreationArgs& args, EGLDisplay display,
@@ -746,6 +740,23 @@
return std::abs(expected - value) < margin;
}
+namespace {
+template <typename T>
+void logSettings(const T& t) {
+ std::stringstream stream;
+ PrintTo(t, &stream);
+ auto string = stream.str();
+ size_t pos = 0;
+ // Perfetto ignores \n, so split up manually into separate ALOGD statements.
+ const size_t size = string.size();
+ while (pos < size) {
+ const size_t end = std::min(string.find("\n", pos), size);
+ ALOGD("%s", string.substr(pos, end - pos).c_str());
+ pos = end + 1;
+ }
+}
+} // namespace
+
void SkiaGLRenderEngine::drawLayersInternal(
const std::shared_ptr<std::promise<RenderEngineResult>>&& resultPromise,
const DisplaySettings& display, const std::vector<LayerSettings>& layers,
@@ -855,18 +866,14 @@
canvas->clear(SK_ColorTRANSPARENT);
initCanvas(canvas, display);
+ if (kPrintLayerSettings) {
+ logSettings(display);
+ }
for (const auto& layer : layers) {
ATRACE_FORMAT("DrawLayer: %s", layer.name.c_str());
if (kPrintLayerSettings) {
- std::stringstream ls;
- PrintTo(layer, &ls);
- auto debugs = ls.str();
- int pos = 0;
- while (pos < debugs.size()) {
- ALOGD("cache_debug %s", debugs.substr(pos, 1000).c_str());
- pos += 1000;
- }
+ logSettings(layer);
}
sk_sp<SkImage> blurInput;
diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.h b/libs/renderengine/skia/SkiaGLRenderEngine.h
index 56815fe..5ef9944 100644
--- a/libs/renderengine/skia/SkiaGLRenderEngine.h
+++ b/libs/renderengine/skia/SkiaGLRenderEngine.h
@@ -61,7 +61,6 @@
bool supportsProtectedContent() const override;
void useProtectedContext(bool useProtectedContext) override;
bool supportsBackgroundBlur() override { return mBlurFilter != nullptr; }
- void assertShadersCompiled(int numShaders) override;
void onActiveDisplaySizeChanged(ui::Size size) override;
int reportShadersCompiled() override;
@@ -178,6 +177,8 @@
return shadersCachedSinceLastCall;
}
+ int totalShadersCompiled() const { return mTotalShadersCompiled; }
+
private:
int mShadersCachedSinceLastCall = 0;
int mTotalShadersCompiled = 0;
diff --git a/libs/renderengine/skia/SkiaRenderEngine.h b/libs/renderengine/skia/SkiaRenderEngine.h
index 5d10b6f..160a186 100644
--- a/libs/renderengine/skia/SkiaRenderEngine.h
+++ b/libs/renderengine/skia/SkiaRenderEngine.h
@@ -45,7 +45,6 @@
virtual bool isProtected() const override { return false; } // mInProtectedContext; }
virtual bool supportsProtectedContent() const override { return false; };
virtual int getContextPriority() override { return 0; }
- virtual void assertShadersCompiled(int numShaders) {}
virtual int reportShadersCompiled() { return 0; }
virtual void setEnableTracing(bool tracingEnabled) override;
diff --git a/libs/renderengine/tests/Android.bp b/libs/renderengine/tests/Android.bp
index e66fee1..bbab792 100644
--- a/libs/renderengine/tests/Android.bp
+++ b/libs/renderengine/tests/Android.bp
@@ -29,6 +29,8 @@
],
test_suites: ["device-tests"],
srcs: [
+ "DisplaySettingsTest.cpp",
+ "LayerSettingsTest.cpp",
"RenderEngineTest.cpp",
"RenderEngineThreadedTest.cpp",
],
diff --git a/libs/renderengine/tests/DisplaySettingsTest.cpp b/libs/renderengine/tests/DisplaySettingsTest.cpp
new file mode 100644
index 0000000..0e93c88
--- /dev/null
+++ b/libs/renderengine/tests/DisplaySettingsTest.cpp
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2022 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 "DisplaySettingsTest"
+
+#include <gtest/gtest.h>
+#include <renderengine/DisplaySettings.h>
+
+namespace android::renderengine {
+
+TEST(DisplaySettingsTest, currentLuminanceNits) {
+ DisplaySettings a, b;
+ ASSERT_EQ(a, b);
+
+ a.currentLuminanceNits = 45.f;
+
+ ASSERT_FALSE(a == b);
+}
+
+TEST(DisplaySettingsTest, targetLuminanceNits) {
+ DisplaySettings a, b;
+ ASSERT_EQ(a, b);
+
+ a.targetLuminanceNits = 45.f;
+
+ ASSERT_FALSE(a == b);
+}
+
+TEST(DisplaySettingsTest, deviceHandlesColorTransform) {
+ DisplaySettings a, b;
+ ASSERT_EQ(a, b);
+
+ a.deviceHandlesColorTransform = true;
+
+ ASSERT_FALSE(a == b);
+}
+
+TEST(DisplaySettingsTest, dimmingStage) {
+ DisplaySettings a, b;
+ ASSERT_EQ(a, b);
+
+ a.dimmingStage = aidl::android::hardware::graphics::composer3::DimmingStage::GAMMA_OETF;
+
+ ASSERT_FALSE(a == b);
+}
+
+TEST(DisplaySettingsTest, renderIntent) {
+ DisplaySettings a, b;
+ ASSERT_EQ(a, b);
+
+ a.renderIntent = aidl::android::hardware::graphics::composer3::RenderIntent::TONE_MAP_ENHANCE;
+
+ ASSERT_FALSE(a == b);
+}
+} // namespace android::renderengine
diff --git a/libs/renderengine/tests/LayerSettingsTest.cpp b/libs/renderengine/tests/LayerSettingsTest.cpp
new file mode 100644
index 0000000..4737335
--- /dev/null
+++ b/libs/renderengine/tests/LayerSettingsTest.cpp
@@ -0,0 +1,33 @@
+/*
+ * Copyright 2022 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 "LayerSettingsTest"
+
+#include <gtest/gtest.h>
+#include <renderengine/LayerSettings.h>
+
+namespace android::renderengine {
+
+TEST(LayerSettingsTest, whitePointNits) {
+ LayerSettings a, b;
+ ASSERT_EQ(a, b);
+
+ a.whitePointNits = 45.f;
+
+ ASSERT_FALSE(a == b);
+}
+} // namespace android::renderengine
diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp
index 31598e3..7c70a74 100644
--- a/libs/renderengine/tests/RenderEngineTest.cpp
+++ b/libs/renderengine/tests/RenderEngineTest.cpp
@@ -3034,6 +3034,23 @@
expectBufferColor(Rect(1, 0, 2, 1), 255, 0, 0, 255); // Still red.
expectBufferColor(Rect(0, 0, 1, 1), 0, 70, 0, 255);
}
+
+TEST_P(RenderEngineTest, primeShaderCache) {
+ if (GetParam()->type() == renderengine::RenderEngine::RenderEngineType::GLES) {
+ GTEST_SKIP();
+ }
+
+ initializeRenderEngine();
+
+ auto fut = mRE->primeCache();
+ if (fut.valid()) {
+ fut.wait();
+ }
+
+ const int minimumExpectedShadersCompiled = GetParam()->useColorManagement() ? 60 : 30;
+ ASSERT_GT(static_cast<skia::SkiaGLRenderEngine*>(mRE.get())->reportShadersCompiled(),
+ minimumExpectedShadersCompiled);
+}
} // namespace renderengine
} // namespace android
diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp
index 31d331b..9c5a129 100644
--- a/services/inputflinger/reader/InputReader.cpp
+++ b/services/inputflinger/reader/InputReader.cpp
@@ -47,8 +47,8 @@
mEventHub(eventHub),
mPolicy(policy),
mQueuedListener(listener),
- mGlobalMetaState(0),
- mLedMetaState(AMETA_NUM_LOCK_ON),
+ mGlobalMetaState(AMETA_NONE),
+ mLedMetaState(AMETA_NONE),
mGeneration(1),
mNextInputDeviceId(END_RESERVED_ID),
mDisableVirtualKeysTimeout(LLONG_MIN),
diff --git a/services/inputflinger/reader/mapper/CursorInputMapper.cpp b/services/inputflinger/reader/mapper/CursorInputMapper.cpp
index dc5fcec..a9a4c71 100644
--- a/services/inputflinger/reader/mapper/CursorInputMapper.cpp
+++ b/services/inputflinger/reader/mapper/CursorInputMapper.cpp
@@ -27,6 +27,9 @@
namespace android {
+// The default velocity control parameters that has no effect.
+static const VelocityControlParameters FLAT_VELOCITY_CONTROL_PARAMS{};
+
// --- CursorMotionAccumulator ---
CursorMotionAccumulator::CursorMotionAccumulator() {
@@ -154,8 +157,9 @@
mHWheelScale = 1.0f;
}
- if ((!changes && config->pointerCaptureRequest.enable) ||
- (changes & InputReaderConfiguration::CHANGE_POINTER_CAPTURE)) {
+ const bool configurePointerCapture = (!changes && config->pointerCaptureRequest.enable) ||
+ (changes & InputReaderConfiguration::CHANGE_POINTER_CAPTURE);
+ if (configurePointerCapture) {
if (config->pointerCaptureRequest.enable) {
if (mParameters.mode == Parameters::MODE_POINTER) {
mParameters.mode = Parameters::MODE_POINTER_RELATIVE;
@@ -180,10 +184,18 @@
}
}
- if (!changes || (changes & InputReaderConfiguration::CHANGE_POINTER_SPEED)) {
- mPointerVelocityControl.setParameters(config->pointerVelocityControlParameters);
- mWheelXVelocityControl.setParameters(config->wheelVelocityControlParameters);
- mWheelYVelocityControl.setParameters(config->wheelVelocityControlParameters);
+ if (!changes || (changes & InputReaderConfiguration::CHANGE_POINTER_SPEED) ||
+ configurePointerCapture) {
+ if (config->pointerCaptureRequest.enable) {
+ // Disable any acceleration or scaling when Pointer Capture is enabled.
+ mPointerVelocityControl.setParameters(FLAT_VELOCITY_CONTROL_PARAMS);
+ mWheelXVelocityControl.setParameters(FLAT_VELOCITY_CONTROL_PARAMS);
+ mWheelYVelocityControl.setParameters(FLAT_VELOCITY_CONTROL_PARAMS);
+ } else {
+ mPointerVelocityControl.setParameters(config->pointerVelocityControlParameters);
+ mWheelXVelocityControl.setParameters(config->wheelVelocityControlParameters);
+ mWheelYVelocityControl.setParameters(config->wheelVelocityControlParameters);
+ }
}
if (!changes || (changes & InputReaderConfiguration::CHANGE_DISPLAY_INFO)) {
diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.cpp b/services/inputflinger/reader/mapper/TouchInputMapper.cpp
index c1934ff..637b1cb 100644
--- a/services/inputflinger/reader/mapper/TouchInputMapper.cpp
+++ b/services/inputflinger/reader/mapper/TouchInputMapper.cpp
@@ -1657,9 +1657,8 @@
dispatchPointerUsage(when, readTime, policyFlags, pointerUsage);
} else {
- updateTouchSpots();
-
if (!mCurrentMotionAborted) {
+ updateTouchSpots();
dispatchButtonRelease(when, readTime, policyFlags);
dispatchHoverExit(when, readTime, policyFlags);
dispatchTouches(when, readTime, policyFlags);
diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp
index bda7755..a26a0bc 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -375,6 +375,11 @@
float getPointerGestureMovementSpeedRatio() { return mConfig.pointerGestureMovementSpeedRatio; }
+ void setVelocityControlParams(const VelocityControlParameters& params) {
+ mConfig.pointerVelocityControlParameters = params;
+ mConfig.wheelVelocityControlParameters = params;
+ }
+
private:
uint32_t mNextPointerCaptureSequenceNumber = 0;
@@ -2949,7 +2954,10 @@
}
void configureDevice(uint32_t changes) {
- if (!changes || (changes & InputReaderConfiguration::CHANGE_DISPLAY_INFO)) {
+ if (!changes ||
+ (changes &
+ (InputReaderConfiguration::CHANGE_DISPLAY_INFO |
+ InputReaderConfiguration::CHANGE_POINTER_CAPTURE))) {
mReader->requestRefreshConfiguration(changes);
mReader->loopOnce();
}
@@ -3364,9 +3372,8 @@
KeyboardInputMapper& mapper =
addMapperAndConfigure<KeyboardInputMapper>(AINPUT_SOURCE_KEYBOARD,
AINPUT_KEYBOARD_TYPE_ALPHABETIC);
- // Initial metastate to AMETA_NONE.
- ASSERT_EQ(AMETA_NUM_LOCK_ON, mapper.getMetaState());
- mapper.updateMetaState(AKEYCODE_NUM_LOCK);
+ // Initial metastate is AMETA_NONE.
+ ASSERT_EQ(AMETA_NONE, mapper.getMetaState());
// Key down by scan code.
process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_HOME, 1);
@@ -3491,9 +3498,8 @@
addMapperAndConfigure<KeyboardInputMapper>(AINPUT_SOURCE_KEYBOARD,
AINPUT_KEYBOARD_TYPE_ALPHABETIC);
- // Initial metastate to AMETA_NONE.
- ASSERT_EQ(AMETA_NUM_LOCK_ON, mapper.getMetaState());
- mapper.updateMetaState(AKEYCODE_NUM_LOCK);
+ // Initial metastate is AMETA_NONE.
+ ASSERT_EQ(AMETA_NONE, mapper.getMetaState());
// Metakey down.
process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_LEFTSHIFT, 1);
@@ -3738,9 +3744,8 @@
KeyboardInputMapper& mapper =
addMapperAndConfigure<KeyboardInputMapper>(AINPUT_SOURCE_KEYBOARD,
AINPUT_KEYBOARD_TYPE_ALPHABETIC);
- // Initialize metastate to AMETA_NUM_LOCK_ON.
- ASSERT_EQ(AMETA_NUM_LOCK_ON, mapper.getMetaState());
- mapper.updateMetaState(AKEYCODE_NUM_LOCK);
+ // Initial metastate is AMETA_NONE.
+ ASSERT_EQ(AMETA_NONE, mapper.getMetaState());
// Initialization should have turned all of the lights off.
ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_CAPSL));
@@ -3806,8 +3811,6 @@
addMapperAndConfigure<KeyboardInputMapper>(AINPUT_SOURCE_KEYBOARD,
AINPUT_KEYBOARD_TYPE_NON_ALPHABETIC);
- // Initial metastate should be AMETA_NONE as no meta keys added.
- ASSERT_EQ(AMETA_NONE, mapper.getMetaState());
// Meta state should be AMETA_NONE after reset
mapper.reset(ARBITRARY_TIME);
ASSERT_EQ(AMETA_NONE, mapper.getMetaState());
@@ -3922,9 +3925,8 @@
KeyboardInputMapper& mapper =
addMapperAndConfigure<KeyboardInputMapper>(AINPUT_SOURCE_KEYBOARD,
AINPUT_KEYBOARD_TYPE_ALPHABETIC);
- // Initial metastate to AMETA_NONE.
- ASSERT_EQ(AMETA_NUM_LOCK_ON, mapper.getMetaState());
- mapper.updateMetaState(AKEYCODE_NUM_LOCK);
+ // Initial metastate is AMETA_NONE.
+ ASSERT_EQ(AMETA_NONE, mapper.getMetaState());
// Initialization should have turned all of the lights off.
ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_CAPSL));
@@ -3991,9 +3993,8 @@
KeyboardInputMapper& mapper =
addMapperAndConfigure<KeyboardInputMapper>(AINPUT_SOURCE_KEYBOARD,
AINPUT_KEYBOARD_TYPE_ALPHABETIC);
- // Initialize metastate to AMETA_NUM_LOCK_ON.
- ASSERT_EQ(AMETA_NUM_LOCK_ON, mapper.getMetaState());
- mapper.updateMetaState(AKEYCODE_NUM_LOCK);
+ // Initial metastate is AMETA_NONE.
+ ASSERT_EQ(AMETA_NONE, mapper.getMetaState());
mReader->toggleCapsLockState(DEVICE_ID);
ASSERT_EQ(AMETA_CAPS_LOCK_ON, mapper.getMetaState());
@@ -4033,7 +4034,13 @@
device2->configure(ARBITRARY_TIME, mFakePolicy->getReaderConfiguration(), 0 /*changes*/);
device2->reset(ARBITRARY_TIME);
- // Initial metastate is AMETA_NUM_LOCK_ON, turn it off.
+ // Initial metastate is AMETA_NONE.
+ ASSERT_EQ(AMETA_NONE, mapper1.getMetaState());
+ ASSERT_EQ(AMETA_NONE, mapper2.getMetaState());
+
+ // Toggle num lock on and off.
+ process(mapper1, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_NUMLOCK, 1);
+ process(mapper1, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_NUMLOCK, 0);
ASSERT_TRUE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_NUML));
ASSERT_EQ(AMETA_NUM_LOCK_ON, mapper1.getMetaState());
ASSERT_EQ(AMETA_NUM_LOCK_ON, mapper2.getMetaState());
@@ -4914,6 +4921,54 @@
ASSERT_NO_FATAL_FAILURE(assertPosition(*mFakePointerController, 110.0f, 220.0f));
}
+/**
+ * When Pointer Capture is enabled, we expect to report unprocessed relative movements, so any
+ * pointer acceleration or speed processing should not be applied.
+ */
+TEST_F(CursorInputMapperTest, PointerCaptureDisablesVelocityProcessing) {
+ addConfigurationProperty("cursor.mode", "pointer");
+ const VelocityControlParameters testParams(5.f /*scale*/, 0.f /*low threshold*/,
+ 100.f /*high threshold*/, 10.f /*acceleration*/);
+ mFakePolicy->setVelocityControlParams(testParams);
+ CursorInputMapper& mapper = addMapperAndConfigure<CursorInputMapper>();
+
+ NotifyDeviceResetArgs resetArgs;
+ ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyDeviceResetWasCalled(&resetArgs));
+ ASSERT_EQ(ARBITRARY_TIME, resetArgs.eventTime);
+ ASSERT_EQ(DEVICE_ID, resetArgs.deviceId);
+
+ NotifyMotionArgs args;
+
+ // Move and verify scale is applied.
+ process(mapper, ARBITRARY_TIME, READ_TIME, EV_REL, REL_X, 10);
+ process(mapper, ARBITRARY_TIME, READ_TIME, EV_REL, REL_Y, 20);
+ process(mapper, ARBITRARY_TIME, READ_TIME, EV_SYN, SYN_REPORT, 0);
+ ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasCalled(&args));
+ ASSERT_EQ(AINPUT_SOURCE_MOUSE, args.source);
+ ASSERT_EQ(AMOTION_EVENT_ACTION_HOVER_MOVE, args.action);
+ const float relX = args.pointerCoords[0].getAxisValue(AMOTION_EVENT_AXIS_RELATIVE_X);
+ const float relY = args.pointerCoords[0].getAxisValue(AMOTION_EVENT_AXIS_RELATIVE_Y);
+ ASSERT_GT(relX, 10);
+ ASSERT_GT(relY, 20);
+
+ // Enable Pointer Capture
+ mFakePolicy->setPointerCapture(true);
+ configureDevice(InputReaderConfiguration::CHANGE_POINTER_CAPTURE);
+ NotifyPointerCaptureChangedArgs captureArgs;
+ ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyCaptureWasCalled(&captureArgs));
+ ASSERT_TRUE(captureArgs.request.enable);
+
+ // Move and verify scale is not applied.
+ process(mapper, ARBITRARY_TIME, READ_TIME, EV_REL, REL_X, 10);
+ process(mapper, ARBITRARY_TIME, READ_TIME, EV_REL, REL_Y, 20);
+ process(mapper, ARBITRARY_TIME, READ_TIME, EV_SYN, SYN_REPORT, 0);
+ ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasCalled(&args));
+ ASSERT_EQ(AINPUT_SOURCE_MOUSE_RELATIVE, args.source);
+ ASSERT_EQ(AMOTION_EVENT_ACTION_MOVE, args.action);
+ ASSERT_EQ(10, args.pointerCoords[0].getX());
+ ASSERT_EQ(20, args.pointerCoords[0].getY());
+}
+
TEST_F(CursorInputMapperTest, Process_ShouldHandleDisplayId) {
CursorInputMapper& mapper = addMapperAndConfigure<CursorInputMapper>();
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
index 5846e67..db2fd1b 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
@@ -267,6 +267,9 @@
// Enables predicting composition strategy to run client composition earlier
virtual void setPredictCompositionStrategy(bool) = 0;
+ // Enables overriding the 170M trasnfer function as sRGB
+ virtual void setTreat170mAsSrgb(bool) = 0;
+
protected:
virtual void setDisplayColorProfile(std::unique_ptr<DisplayColorProfile>) = 0;
virtual void setRenderSurface(std::unique_ptr<RenderSurface>) = 0;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
index 0feb9f7..31c51e6 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
@@ -108,6 +108,7 @@
void cacheClientCompositionRequests(uint32_t) override;
bool canPredictCompositionStrategy(const CompositionRefreshArgs&) override;
void setPredictCompositionStrategy(bool) override;
+ void setTreat170mAsSrgb(bool) override;
// Testing
const ReleasedLayers& getReleasedLayersForTest() const;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
index 61be983..c65d467 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
@@ -162,6 +162,8 @@
CompositionStrategyPredictionState strategyPrediction =
CompositionStrategyPredictionState::DISABLED;
+ bool treat170mAsSrgb = false;
+
// Debugging
void dump(std::string& result) const;
};
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
index fa86076..cb9fbad 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
@@ -132,6 +132,7 @@
MOCK_METHOD1(cacheClientCompositionRequests, void(uint32_t));
MOCK_METHOD1(canPredictCompositionStrategy, bool(const CompositionRefreshArgs&));
MOCK_METHOD1(setPredictCompositionStrategy, void(bool));
+ MOCK_METHOD1(setTreat170mAsSrgb, void(bool));
};
} // namespace android::compositionengine::mock
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index ec86731..4c30f99 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -1488,6 +1488,10 @@
}
}
+void Output::setTreat170mAsSrgb(bool enable) {
+ editState().treat170mAsSrgb = enable;
+}
+
bool Output::canPredictCompositionStrategy(const CompositionRefreshArgs& refreshArgs) {
if (!getState().isEnabled || !mHwComposerAsyncWorker) {
ALOGV("canPredictCompositionStrategy disabled");
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp
index 3b85e3b..948c0c9 100644
--- a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp
@@ -66,6 +66,9 @@
out.append("\n ");
dumpVal(out, "compositionStrategyPredictionState", ftl::enum_string(strategyPrediction));
+ out.append("\n ");
+ dumpVal(out, "treate170mAsSrgb", treat170mAsSrgb);
+
out += '\n';
}
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
index 744561c..5ffbb7f 100644
--- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
@@ -320,6 +320,17 @@
? outputState.targetDataspace
: layerFEState->dataspace;
+ // Override the dataspace transfer from 170M to sRGB if the device configuration requests this.
+ // We do this here instead of in buffer info so that dumpsys can still report layers that are
+ // using the 170M transfer. Also we only do this if the colorspace is not agnostic for the
+ // layer, in case the color profile uses a 170M transfer function.
+ if (outputState.treat170mAsSrgb && !layerFEState->isColorspaceAgnostic &&
+ (state.dataspace & HAL_DATASPACE_TRANSFER_MASK) == HAL_DATASPACE_TRANSFER_SMPTE_170M) {
+ state.dataspace = static_cast<ui::Dataspace>(
+ (state.dataspace & HAL_DATASPACE_STANDARD_MASK) |
+ (state.dataspace & HAL_DATASPACE_RANGE_MASK) | HAL_DATASPACE_TRANSFER_SRGB);
+ }
+
// For hdr content, treat the white point as the display brightness - HDR content should not be
// boosted or dimmed.
// If the layer explicitly requests to disable dimming, then don't dim either.
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
index f96570a..7038e8c 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
@@ -658,6 +658,23 @@
EXPECT_EQ(ui::Dataspace::V0_SCRGB, mOutputLayer.getState().dataspace);
}
+TEST_F(OutputLayerUpdateCompositionStateTest, setsOutputLayerColorspaceWith170mReplacement) {
+ mLayerFEState.dataspace = ui::Dataspace::TRANSFER_SMPTE_170M;
+ mOutputState.targetDataspace = ui::Dataspace::V0_SCRGB;
+ mOutputState.treat170mAsSrgb = false;
+ mLayerFEState.isColorspaceAgnostic = false;
+
+ mOutputLayer.updateCompositionState(false, false, ui::Transform::RotationFlags::ROT_0);
+
+ EXPECT_EQ(ui::Dataspace::TRANSFER_SMPTE_170M, mOutputLayer.getState().dataspace);
+
+ // Rewrite SMPTE 170M as sRGB
+ mOutputState.treat170mAsSrgb = true;
+ mOutputLayer.updateCompositionState(false, false, ui::Transform::RotationFlags::ROT_0);
+
+ EXPECT_EQ(ui::Dataspace::TRANSFER_SRGB, mOutputLayer.getState().dataspace);
+}
+
TEST_F(OutputLayerUpdateCompositionStateTest, setsWhitePointNitsAndDimmingRatioCorrectly) {
mOutputState.sdrWhitePointNits = 200.f;
mOutputState.displayBrightnessNits = 800.f;
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
index 42c8b37..3a3c91e 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
@@ -248,6 +248,20 @@
}
/*
+ * Output::setTreat170mAsSrgb()
+ */
+
+TEST_F(OutputTest, setTreat170mAsSrgb) {
+ EXPECT_FALSE(mOutput->getState().treat170mAsSrgb);
+
+ mOutput->setTreat170mAsSrgb(true);
+ EXPECT_TRUE(mOutput->getState().treat170mAsSrgb);
+
+ mOutput->setTreat170mAsSrgb(false);
+ EXPECT_FALSE(mOutput->getState().treat170mAsSrgb);
+}
+
+/*
* Output::setLayerCachingEnabled()
*/
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 52529d6..a915b61 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -89,6 +89,7 @@
}
mCompositionDisplay->setPredictCompositionStrategy(mFlinger->mPredictCompositionStrategy);
+ mCompositionDisplay->setTreat170mAsSrgb(mFlinger->mTreat170mAsSrgb);
mCompositionDisplay->createDisplayColorProfile(
compositionengine::DisplayColorProfileCreationArgsBuilder()
.setHasWideColorGamut(args.hasWideColorGamut)
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 3a92ca4..e1eec8b 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -47,6 +47,7 @@
#include <stdint.h>
#include <stdlib.h>
#include <sys/types.h>
+#include <system/graphics-base-v1.0.h>
#include <ui/DataspaceUtils.h>
#include <ui/DebugUtils.h>
#include <ui/GraphicBuffer.h>
@@ -600,6 +601,18 @@
layerSettings.alpha = alpha;
layerSettings.sourceDataspace = getDataSpace();
+ // Override the dataspace transfer from 170M to sRGB if the device configuration requests this.
+ // We do this here instead of in buffer info so that dumpsys can still report layers that are
+ // using the 170M transfer.
+ if (mFlinger->mTreat170mAsSrgb &&
+ (layerSettings.sourceDataspace & HAL_DATASPACE_TRANSFER_MASK) ==
+ HAL_DATASPACE_TRANSFER_SMPTE_170M) {
+ layerSettings.sourceDataspace = static_cast<ui::Dataspace>(
+ (layerSettings.sourceDataspace & HAL_DATASPACE_STANDARD_MASK) |
+ (layerSettings.sourceDataspace & HAL_DATASPACE_RANGE_MASK) |
+ HAL_DATASPACE_TRANSFER_SRGB);
+ }
+
layerSettings.whitePointNits = targetSettings.whitePointNits;
switch (targetSettings.blurSetting) {
case LayerFE::ClientCompositionTargetSettings::BlurSetting::Enabled:
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index d39176b..e72e21c 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -419,6 +419,9 @@
property_get("debug.sf.predict_hwc_composition_strategy", value, "1");
mPredictCompositionStrategy = atoi(value);
+ property_get("debug.sf.treat_170m_as_sRGB", value, "0");
+ mTreat170mAsSrgb = atoi(value);
+
// We should be reading 'persist.sys.sf.color_saturation' here
// but since /data may be encrypted, we need to wait until after vold
// comes online to attempt to read the property. The property is
@@ -3472,6 +3475,15 @@
l->latchAndReleaseBuffer();
}
+ // If a layer has a parent, we allow it to out-live it's handle
+ // with the idea that the parent holds a reference and will eventually
+ // be cleaned up. However no one cleans up the top-level so we do so
+ // here.
+ if (l->isAtRoot()) {
+ l->setIsAtRoot(false);
+ mCurrentState.layersSortedByZ.remove(l);
+ }
+
// If the layer has been removed and has no parent, then it will not be reachable
// when traversing layers on screen. Add the layer to the offscreenLayers set to
// ensure we can copy its current to drawing state.
@@ -4762,14 +4774,6 @@
void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp<Layer>& layer) {
Mutex::Autolock lock(mStateLock);
- // If a layer has a parent, we allow it to out-live it's handle
- // with the idea that the parent holds a reference and will eventually
- // be cleaned up. However no one cleans up the top-level so we do so
- // here.
- if (layer->isAtRoot()) {
- layer->setIsAtRoot(false);
- mCurrentState.layersSortedByZ.remove(layer);
- }
markLayerPendingRemovalLocked(layer);
mBufferCountTracker.remove(handle);
layer.clear();
@@ -6709,6 +6713,9 @@
auto dataspace = renderArea.getReqDataSpace();
auto parent = renderArea.getParentLayer();
auto renderIntent = RenderIntent::TONE_MAP_COLORIMETRIC;
+ auto sdrWhitePointNits = DisplayDevice::sDefaultMaxLumiance;
+ auto displayBrightnessNits = DisplayDevice::sDefaultMaxLumiance;
+
if ((dataspace == ui::Dataspace::UNKNOWN) && (parent != nullptr)) {
Mutex::Autolock lock(mStateLock);
auto display = findDisplay([layerStack = parent->getLayerStack()](const auto& display) {
@@ -6722,6 +6729,8 @@
const ui::ColorMode colorMode = display->getCompositionDisplay()->getState().colorMode;
dataspace = pickDataspaceFromColorMode(colorMode);
renderIntent = display->getCompositionDisplay()->getState().renderIntent;
+ sdrWhitePointNits = display->getCompositionDisplay()->getState().sdrWhitePointNits;
+ displayBrightnessNits = display->getCompositionDisplay()->getState().displayBrightnessNits;
}
captureResults.capturedDataspace = dataspace;
@@ -6780,7 +6789,7 @@
BlurSetting::Disabled
: compositionengine::LayerFE::ClientCompositionTargetSettings::
BlurSetting::Enabled,
- DisplayDevice::sDefaultMaxLumiance,
+ isHdrDataspace(dataspace) ? displayBrightnessNits : sdrWhitePointNits,
};
std::vector<compositionengine::LayerFE::LayerSettings> results =
@@ -6795,6 +6804,7 @@
if (regionSampling) {
settings.backgroundBlurRadius = 0;
}
+ captureResults.capturedHdrLayers |= isHdrDataspace(settings.sourceDataspace);
}
clientCompositionLayers.insert(clientCompositionLayers.end(),
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 74e0407..c70e174 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -349,6 +349,12 @@
// run parallel to the hwc validateDisplay call and re-run if the predition is incorrect.
bool mPredictCompositionStrategy = false;
+ // If true, then any layer with a SMPTE 170M transfer function is decoded using the sRGB
+ // transfer instead. This is mainly to preserve legacy behavior, where implementations treated
+ // SMPTE 170M as sRGB prior to color management being implemented, and now implementations rely
+ // on this behavior to increase contrast for some media sources.
+ bool mTreat170mAsSrgb = false;
+
protected:
// We're reference counted, never destroy SurfaceFlinger directly
virtual ~SurfaceFlinger();
diff --git a/services/surfaceflinger/tests/ScreenCapture_test.cpp b/services/surfaceflinger/tests/ScreenCapture_test.cpp
index f9b3185..6a7d8b8 100644
--- a/services/surfaceflinger/tests/ScreenCapture_test.cpp
+++ b/services/surfaceflinger/tests/ScreenCapture_test.cpp
@@ -112,7 +112,7 @@
args.captureSecureLayers = true;
ASSERT_EQ(NO_ERROR, ScreenCapture::captureDisplay(args, mCaptureResults));
ASSERT_TRUE(mCaptureResults.capturedSecureLayers);
- ScreenCapture sc(mCaptureResults.buffer);
+ ScreenCapture sc(mCaptureResults.buffer, mCaptureResults.capturedHdrLayers);
sc.expectColor(Rect(0, 0, 32, 32), Color::RED);
}
@@ -147,7 +147,7 @@
args.captureSecureLayers = true;
ASSERT_EQ(NO_ERROR, ScreenCapture::captureDisplay(args, mCaptureResults));
ASSERT_TRUE(mCaptureResults.capturedSecureLayers);
- ScreenCapture sc(mCaptureResults.buffer);
+ ScreenCapture sc(mCaptureResults.buffer, mCaptureResults.capturedHdrLayers);
sc.expectColor(Rect(0, 0, 10, 10), Color::BLUE);
}
@@ -374,7 +374,7 @@
ASSERT_NO_FATAL_FAILURE(fillBufferStateLayerColor(child, Color::RED, 32, 32));
SurfaceComposerClient::Transaction().apply(true);
ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(args, captureResults));
- ScreenCapture sc(captureResults.buffer);
+ ScreenCapture sc(captureResults.buffer, captureResults.capturedHdrLayers);
sc.expectColor(Rect(0, 0, 9, 9), Color::RED);
}
@@ -860,6 +860,46 @@
mCapture->expectColor(Rect(0, 0, 32, 32), Color::RED);
}
+TEST_F(ScreenCaptureTest, CaptureNonHdrLayer) {
+ sp<SurfaceControl> layer;
+ ASSERT_NO_FATAL_FAILURE(layer = createLayer("test layer", 32, 32,
+ ISurfaceComposerClient::eFXSurfaceBufferState,
+ mBGSurfaceControl.get()));
+ ASSERT_NO_FATAL_FAILURE(fillBufferStateLayerColor(layer, Color::BLACK, 32, 32));
+ Transaction()
+ .show(layer)
+ .setLayer(layer, INT32_MAX)
+ .setDataspace(layer, ui::Dataspace::V0_SRGB)
+ .apply();
+
+ LayerCaptureArgs captureArgs;
+ captureArgs.layerHandle = layer->getHandle();
+
+ ScreenCapture::captureLayers(&mCapture, captureArgs);
+ mCapture->expectColor(Rect(0, 0, 32, 32), Color::BLACK);
+ ASSERT_FALSE(mCapture->capturedHdrLayers());
+}
+
+TEST_F(ScreenCaptureTest, CaptureHdrLayer) {
+ sp<SurfaceControl> layer;
+ ASSERT_NO_FATAL_FAILURE(layer = createLayer("test layer", 32, 32,
+ ISurfaceComposerClient::eFXSurfaceBufferState,
+ mBGSurfaceControl.get()));
+ ASSERT_NO_FATAL_FAILURE(fillBufferStateLayerColor(layer, Color::BLACK, 32, 32));
+ Transaction()
+ .show(layer)
+ .setLayer(layer, INT32_MAX)
+ .setDataspace(layer, ui::Dataspace::BT2020_ITU_PQ)
+ .apply();
+
+ LayerCaptureArgs captureArgs;
+ captureArgs.layerHandle = layer->getHandle();
+
+ ScreenCapture::captureLayers(&mCapture, captureArgs);
+ mCapture->expectColor(Rect(0, 0, 32, 32), Color::BLACK);
+ ASSERT_TRUE(mCapture->capturedHdrLayers());
+}
+
// In the following tests we verify successful skipping of a parent layer,
// so we use the same verification logic and only change how we mutate
// the parent layer to verify that various properties are ignored.
diff --git a/services/surfaceflinger/tests/TransactionTestHarnesses.h b/services/surfaceflinger/tests/TransactionTestHarnesses.h
index 60cffb1..8ce63bc 100644
--- a/services/surfaceflinger/tests/TransactionTestHarnesses.h
+++ b/services/surfaceflinger/tests/TransactionTestHarnesses.h
@@ -79,7 +79,8 @@
BufferItem item;
itemConsumer->acquireBuffer(&item, 0, true);
- auto sc = std::make_unique<ScreenCapture>(item.mGraphicBuffer);
+ constexpr bool kContainsHdr = false;
+ auto sc = std::make_unique<ScreenCapture>(item.mGraphicBuffer, kContainsHdr);
itemConsumer->releaseBuffer(item);
SurfaceComposerClient::destroyDisplay(vDisplay);
return sc;
diff --git a/services/surfaceflinger/tests/utils/ScreenshotUtils.h b/services/surfaceflinger/tests/utils/ScreenshotUtils.h
index ee7e92c..f879430 100644
--- a/services/surfaceflinger/tests/utils/ScreenshotUtils.h
+++ b/services/surfaceflinger/tests/utils/ScreenshotUtils.h
@@ -60,7 +60,8 @@
DisplayCaptureArgs& captureArgs) {
ScreenCaptureResults captureResults;
ASSERT_EQ(NO_ERROR, captureDisplay(captureArgs, captureResults));
- *sc = std::make_unique<ScreenCapture>(captureResults.buffer);
+ *sc = std::make_unique<ScreenCapture>(captureResults.buffer,
+ captureResults.capturedHdrLayers);
}
static status_t captureLayers(LayerCaptureArgs& captureArgs,
@@ -81,9 +82,12 @@
static void captureLayers(std::unique_ptr<ScreenCapture>* sc, LayerCaptureArgs& captureArgs) {
ScreenCaptureResults captureResults;
ASSERT_EQ(NO_ERROR, captureLayers(captureArgs, captureResults));
- *sc = std::make_unique<ScreenCapture>(captureResults.buffer);
+ *sc = std::make_unique<ScreenCapture>(captureResults.buffer,
+ captureResults.capturedHdrLayers);
}
+ bool capturedHdrLayers() const { return mContainsHdr; }
+
void expectColor(const Rect& rect, const Color& color, uint8_t tolerance = 0) {
ASSERT_NE(nullptr, mOutBuffer);
ASSERT_NE(nullptr, mPixels);
@@ -181,7 +185,8 @@
EXPECT_EQ(height, mOutBuffer->getHeight());
}
- explicit ScreenCapture(const sp<GraphicBuffer>& outBuffer) : mOutBuffer(outBuffer) {
+ explicit ScreenCapture(const sp<GraphicBuffer>& outBuffer, bool containsHdr)
+ : mOutBuffer(outBuffer), mContainsHdr(containsHdr) {
if (mOutBuffer) {
mOutBuffer->lock(GRALLOC_USAGE_SW_READ_OFTEN, reinterpret_cast<void**>(&mPixels));
}
@@ -193,6 +198,7 @@
private:
sp<GraphicBuffer> mOutBuffer;
+ bool mContainsHdr = mContainsHdr;
uint8_t* mPixels = nullptr;
};
} // namespace