summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Leon Scroggins III <scroggo@google.com> 2022-04-06 14:38:22 -0400
committer Leon Scroggins III <scroggo@google.com> 2022-04-29 15:07:47 -0400
commit191b877ec806dd0d5f9a3f15322dff8d6a5a1ee6 (patch)
treeee411f27d5a3eb61ea3297f49cc6a0bb3c276cb9
parentc8c333259715252d81c3c4c28cdf64fd3c148b7b (diff)
Improve printing of Display- and Layer- Settings
In order to add any shaders to our cache warming code, we need to look at the logcat to see what settings created a particular shader. Update the PrintTo methods to be simpler and provide more information. Add fields that were missing from operator==, and add operator!= where necessary. Add tests for the missing fields. LayerSettings: - If a matrix is Identity, simply print "I". This cuts down on a lot of noise. Do the same for DisplaySettings. - Similarly, if the ShadowSettings or StretchEffect is the default, skip it. - PixelSource: only print the Buffer OR the SolidColor; only one is respected anyway. - Reorder printing to line up with the order of declaration. - Remove 'Defining PrintTo helps with Google Tests.' This method is more generally useful. SkiaGLRenderEngine: - Set kFlushAfterEveryLayer to kPrintLayerSettings. These are compile-time constants, and they're typically modified together. - The LayerSettings include several lines of text. Instead of splitting them up at an arbitrary boundary, splitting words, break them up based on the newline character. This makes the logs much easier to read. - Move this into a common function so that we can also print the DisplaySettings, which may impact what shaders are compiled. Bug: 190487656 Bug: 222355787 Test: perfetto Test: LayerSettingsTest and DisplaySettingsTest Change-Id: Ifc464779ed368d2edad7fc6adab53e9b368fd7cb
-rw-r--r--libs/renderengine/include/renderengine/DisplaySettings.h47
-rw-r--r--libs/renderengine/include/renderengine/LayerSettings.h58
-rw-r--r--libs/renderengine/include/renderengine/PrintMatrix.h36
-rw-r--r--libs/renderengine/skia/SkiaGLRenderEngine.cpp31
-rw-r--r--libs/renderengine/tests/Android.bp2
-rw-r--r--libs/renderengine/tests/DisplaySettingsTest.cpp69
-rw-r--r--libs/renderengine/tests/LayerSettingsTest.cpp33
7 files changed, 244 insertions, 32 deletions
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/SkiaGLRenderEngine.cpp b/libs/renderengine/skia/SkiaGLRenderEngine.cpp
index 76ae2fc38c..e6acebba28 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);
@@ -744,6 +744,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,
@@ -853,18 +870,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/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