diff options
36 files changed, 565 insertions, 98 deletions
diff --git a/include/android/input.h b/include/android/input.h index fb5e204450..38b27bc587 100644 --- a/include/android/input.h +++ b/include/android/input.h @@ -810,7 +810,7 @@ enum { /** * Constants that identify different gesture classification types. */ -enum { +enum AMotionClassification : uint32_t { /** * Classification constant: None. * @@ -820,7 +820,8 @@ enum { /** * 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 @@ float AMotionEvent_getHistoricalAxisValue(const AInputEvent* motion_event, * 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 @@ int32_t AMotionEvent_getActionButton(const AInputEvent* motion_event); * @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 0caf005a9a..ce9cd1cebe 100644 --- a/libs/battery/MultiStateCounter.h +++ b/libs/battery/MultiStateCounter.h @@ -139,22 +139,35 @@ void MultiStateCounter<T>::setEnabled(bool enabled, time_t timestamp) { 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 = enabled; + 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; + } - 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 @@ const T& MultiStateCounter<T>::updateValue(const T& value, time_t timestamp) { // 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 e91f74f3d3..fe387064bc 100644 --- a/libs/gui/ScreenCaptureResults.cpp +++ b/libs/gui/ScreenCaptureResults.cpp @@ -36,6 +36,7 @@ status_t ScreenCaptureResults::writeToParcel(android::Parcel* parcel) const { } 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 @@ status_t ScreenCaptureResults::readFromParcel(const android::Parcel* parcel) { } 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 7a63af0ed3..6642ec6337 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -1495,6 +1495,18 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffe 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 99c35c1a3d..724c11c881 100644 --- a/libs/gui/include/gui/ScreenCaptureResults.h +++ b/libs/gui/include/gui/ScreenCaptureResults.h @@ -33,6 +33,7 @@ public: 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 a5e0879ce5..59ef991eec 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 @@ struct DisplaySettings { 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; +} + +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"; + } } -// Defining PrintTo helps with Google Tests. static inline void PrintTo(const DisplaySettings& settings, ::std::ostream* os) { *os << "DisplaySettings {"; *os << "\n .physicalDisplay = "; @@ -94,10 +122,19 @@ static inline void PrintTo(const DisplaySettings& settings, ::std::ostream* os) *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 171cbaa2ef..154e5269f0 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 @@ static inline bool operator==(const ShadowSettings& lhs, const ShadowSettings& r 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 @@ static inline bool operator==(const LayerSettings& lhs, const LayerSettings& rhs 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 @@ static inline void PrintTo(const Geometry& settings, ::std::ostream* os) { *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 Geometry& settings, ::std::ostream* os) { 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 @@ static inline void PrintTo(const LayerSettings& settings, ::std::ostream* os) { *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; + if (settings.shadow != ShadowSettings()) { + *os << "\n .shadow = "; + PrintTo(settings.shadow, os); + } *os << "\n .backgroundBlurRadius = " << settings.backgroundBlurRadius; - for (auto blurRegion : settings.blurRegions) { - *os << "\n"; - PrintTo(blurRegion, os); + 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 .shadow = "; - PrintTo(settings.shadow, os); - *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 0000000000..11a5c211bc --- /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 5c137a4b5f..f3064f3c69 100644 --- a/libs/renderengine/skia/Cache.cpp +++ b/libs/renderengine/skia/Cache.cpp @@ -439,7 +439,7 @@ void Cache::primeShaderCache(SkiaRenderEngine* renderengine) { 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 f440f4a058..97271cb32d 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 @@ void SkiaGLRenderEngine::SkSLCacheMonitor::store(const SkData& key, const SkData 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 @@ static bool equalsWithinMargin(float expected, float value, float margin = kDefa 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 @@ void SkiaGLRenderEngine::drawLayersInternal( 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 56815fee7b..5ef9944b14 100644 --- a/libs/renderengine/skia/SkiaGLRenderEngine.h +++ b/libs/renderengine/skia/SkiaGLRenderEngine.h @@ -61,7 +61,6 @@ public: 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 @@ private: 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 5d10b6f8be..160a18698e 100644 --- a/libs/renderengine/skia/SkiaRenderEngine.h +++ b/libs/renderengine/skia/SkiaRenderEngine.h @@ -45,7 +45,6 @@ public: 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 e66fee117c..bbab792dba 100644 --- a/libs/renderengine/tests/Android.bp +++ b/libs/renderengine/tests/Android.bp @@ -29,6 +29,8 @@ cc_test { ], 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 0000000000..0e93c88171 --- /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 0000000000..4737335d40 --- /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 31598e3cc3..7c70a748b5 100644 --- a/libs/renderengine/tests/RenderEngineTest.cpp +++ b/libs/renderengine/tests/RenderEngineTest.cpp @@ -3034,6 +3034,23 @@ TEST_P(RenderEngineTest, r8_respects_color_transform_when_device_handles) { 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 31d331bb98..9c5a129213 100644 --- a/services/inputflinger/reader/InputReader.cpp +++ b/services/inputflinger/reader/InputReader.cpp @@ -47,8 +47,8 @@ InputReader::InputReader(std::shared_ptr<EventHubInterface> eventHub, 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 dc5fcec1df..a9a4c71c02 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 @@ void CursorInputMapper::configure(nsecs_t when, const InputReaderConfiguration* 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 @@ void CursorInputMapper::configure(nsecs_t when, const InputReaderConfiguration* } } - 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 c1934ffbbf..637b1cb263 100644 --- a/services/inputflinger/reader/mapper/TouchInputMapper.cpp +++ b/services/inputflinger/reader/mapper/TouchInputMapper.cpp @@ -1657,9 +1657,8 @@ void TouchInputMapper::cookAndDispatch(nsecs_t when, nsecs_t readTime) { 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 bda7755513..a26a0bcf67 100644 --- a/services/inputflinger/tests/InputReader_test.cpp +++ b/services/inputflinger/tests/InputReader_test.cpp @@ -375,6 +375,11 @@ public: 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 @@ protected: } 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 @@ TEST_F(KeyboardInputMapperTest, Process_SimpleKeyPress) { 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 @@ TEST_F(KeyboardInputMapperTest, Process_ShouldUpdateMetaState) { 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 @@ TEST_F(KeyboardInputMapperTest, Process_LockedKeysShouldToggleMetaStateAndLeds) 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 @@ TEST_F(KeyboardInputMapperTest, NoMetaStateWhenMetaKeysNotPresent) { 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 @@ TEST_F(KeyboardInputMapperTest, Process_LockedKeysShouldToggleAfterReattach) { 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 @@ TEST_F(KeyboardInputMapperTest, Process_toggleCapsLockState) { 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 @@ TEST_F(KeyboardInputMapperTest, Process_LockedKeysShouldToggleInMultiDevices) { 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 @@ TEST_F(CursorInputMapperTest, Process_PointerCapture) { 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 5846e674f5..db2fd1b500 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h @@ -267,6 +267,9 @@ public: // 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 0feb9f7fb7..31c51e6a8d 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h @@ -108,6 +108,7 @@ public: 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 61be983480..c65d467b0d 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h @@ -162,6 +162,8 @@ struct OutputCompositionState { 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 fa86076b44..cb9fbad8dd 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h @@ -132,6 +132,7 @@ public: 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 ec8673177f..4c30f99610 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -1488,6 +1488,10 @@ void Output::setPredictCompositionStrategy(bool predict) { } } +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 3b85e3b5b0..948c0c90bf 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp @@ -66,6 +66,9 @@ void OutputCompositionState::dump(std::string& out) const { 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 744561c38d..5ffbb7ff91 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp @@ -320,6 +320,17 @@ void OutputLayer::updateCompositionState( ? 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 f96570a36e..7038e8cda1 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp @@ -658,6 +658,23 @@ TEST_F(OutputLayerUpdateCompositionStateTest, setsOutputLayerColorspaceCorrectly 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 42c8b37710..3a3c91e518 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp @@ -248,6 +248,20 @@ TEST_F(OutputTest, setCompositionEnabledSetsDisabledAndDirtiesEntireOutput) { } /* + * 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 52529d6b04..a915b615d9 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -89,6 +89,7 @@ DisplayDevice::DisplayDevice(DisplayDeviceCreationArgs& args) } 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 3a92ca49a2..e1eec8b97e 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 @@ std::optional<compositionengine::LayerFE::LayerSettings> Layer::prepareClientCom 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 d39176be14..e72e21ceb3 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -419,6 +419,9 @@ SurfaceFlinger::SurfaceFlinger(Factory& factory) : SurfaceFlinger(factory, SkipI 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 @@ void SurfaceFlinger::doCommitTransactions() { 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::markLayerPendingRemovalLocked(const sp<Layer>& layer) { 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 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScree 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 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScree 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 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScree BlurSetting::Disabled : compositionengine::LayerFE::ClientCompositionTargetSettings:: BlurSetting::Enabled, - DisplayDevice::sDefaultMaxLumiance, + isHdrDataspace(dataspace) ? displayBrightnessNits : sdrWhitePointNits, }; std::vector<compositionengine::LayerFE::LayerSettings> results = @@ -6795,6 +6804,7 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScree 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 74e0407215..c70e1749ff 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -349,6 +349,12 @@ public: // 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 f9b31853fc..6a7d8b8ca5 100644 --- a/services/surfaceflinger/tests/ScreenCapture_test.cpp +++ b/services/surfaceflinger/tests/ScreenCapture_test.cpp @@ -112,7 +112,7 @@ TEST_F(ScreenCaptureTest, SetFlagsSecureEUidSystem) { 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 @@ TEST_F(ScreenCaptureTest, CaptureChildSetParentFlagsSecureEUidSystem) { 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 @@ TEST_F(ScreenCaptureTest, CaptureBufferLayerWithoutBufferFails) { 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 @@ TEST_F(ScreenCaptureTest, CaptureOffscreen) { 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 60cffb19df..8ce63bc64c 100644 --- a/services/surfaceflinger/tests/TransactionTestHarnesses.h +++ b/services/surfaceflinger/tests/TransactionTestHarnesses.h @@ -79,7 +79,8 @@ public: 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 ee7e92cbf6..f879430db0 100644 --- a/services/surfaceflinger/tests/utils/ScreenshotUtils.h +++ b/services/surfaceflinger/tests/utils/ScreenshotUtils.h @@ -60,7 +60,8 @@ public: 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 @@ public: 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 @@ public: 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 @@ public: private: sp<GraphicBuffer> mOutBuffer; + bool mContainsHdr = mContainsHdr; uint8_t* mPixels = nullptr; }; } // namespace |