fix(force invert): fix white-on-white text when text is drawn against a fill rect in the same RenderNode This fixes issues with custom Views that draw background and text in the same View/RenderNode. Instead of trying to force the individual paints to be dark, we simply invert the whole view. Adds a new hasFill() member to DisplayList and Canvas so that we record when the RenderNode has a call that fills the background. Also adds a new Container usage hint type, for this exact situation. Bug: 282821643 Bug: 293883260 Test: RenderNodeTests.cpp: mmm -j8 frameworks/base/libs/hwui && adb push $ANDROID_PRODUCT_OUT/data/nativetest/hwui_unit_tests/hwui_unit_tests /data/nativetest/hwui_unit_tests/hwui_unit_tests && adb shell /data/nativetest/hwui_unit_tests/hwui_unit_tests Change-Id: Ia0fe9c739bf07c1f3a8508d3f295a1dc54ee48f9
diff --git a/libs/hwui/CanvasTransform.cpp b/libs/hwui/CanvasTransform.cpp index cd4fae8..b667daf 100644 --- a/libs/hwui/CanvasTransform.cpp +++ b/libs/hwui/CanvasTransform.cpp
@@ -80,6 +80,19 @@ static void applyColorTransform(ColorTransform transform, SkPaint& paint) { if (transform == ColorTransform::None) return; + if (transform == ColorTransform::Invert) { + auto filter = SkHighContrastFilter::Make( + {/* grayscale= */ false, SkHighContrastConfig::InvertStyle::kInvertLightness, + /* contrast= */ 0.0f}); + + if (paint.getColorFilter()) { + paint.setColorFilter(SkColorFilters::Compose(filter, paint.refColorFilter())); + } else { + paint.setColorFilter(filter); + } + return; + } + SkColor newColor = transformColor(transform, paint.getColor()); paint.setColor(newColor);
diff --git a/libs/hwui/CanvasTransform.h b/libs/hwui/CanvasTransform.h index 291f4cf..288dca4 100644 --- a/libs/hwui/CanvasTransform.h +++ b/libs/hwui/CanvasTransform.h
@@ -29,12 +29,15 @@ Unknown = 0, Background = 1, Foreground = 2, + // Contains foreground (usually text), like a button or chip + Container = 3 }; enum class ColorTransform { None, Light, Dark, + Invert }; // True if the paint was modified, false otherwise
diff --git a/libs/hwui/DisplayList.h b/libs/hwui/DisplayList.h index 8c180da..b1c5bf4 100644 --- a/libs/hwui/DisplayList.h +++ b/libs/hwui/DisplayList.h
@@ -145,6 +145,8 @@ return mImpl && mImpl->hasText(); } + [[nodiscard]] bool hasFill() const { return mImpl && mImpl->hasFill(); } + void applyColorTransform(ColorTransform transform) { if (mImpl) { mImpl->applyColorTransform(transform);
diff --git a/libs/hwui/RecordingCanvas.cpp b/libs/hwui/RecordingCanvas.cpp index ff0d8d74..3b694c5 100644 --- a/libs/hwui/RecordingCanvas.cpp +++ b/libs/hwui/RecordingCanvas.cpp
@@ -718,6 +718,27 @@ return (value & (value - 1)) == 0; } +template <typename T> +constexpr bool doesPaintHaveFill(T& paint) { + using T1 = std::remove_cv_t<T>; + if constexpr (std::is_same_v<T1, SkPaint>) { + return paint.getStyle() != SkPaint::Style::kStroke_Style; + } else if constexpr (std::is_same_v<T1, SkPaint&>) { + return paint.getStyle() != SkPaint::Style::kStroke_Style; + } else if constexpr (std::is_same_v<T1, SkPaint*>) { + return paint && paint->getStyle() != SkPaint::Style::kStroke_Style; + } else if constexpr (std::is_same_v<T1, const SkPaint*>) { + return paint && paint->getStyle() != SkPaint::Style::kStroke_Style; + } + + return false; +} + +template <typename... Args> +constexpr bool hasPaintWithFill(Args&&... args) { + return (... || doesPaintHaveFill(args)); +} + template <typename T, typename... Args> void* DisplayListData::push(size_t pod, Args&&... args) { size_t skip = SkAlignPtr(sizeof(T) + pod); @@ -736,6 +757,14 @@ new (op) T{std::forward<Args>(args)...}; op->type = (uint32_t)T::kType; op->skip = skip; + + // check if this is a fill op or not, in case we need to avoid messing with it with force invert + if constexpr (!std::is_same_v<T, DrawTextBlob>) { + if (hasPaintWithFill(args...)) { + mHasFill = true; + } + } + return op + 1; }
diff --git a/libs/hwui/RecordingCanvas.h b/libs/hwui/RecordingCanvas.h index 4f54ee2..afadbfd 100644 --- a/libs/hwui/RecordingCanvas.h +++ b/libs/hwui/RecordingCanvas.h
@@ -110,7 +110,7 @@ class DisplayListData final { public: - DisplayListData() : mHasText(false) {} + DisplayListData() : mHasText(false), mHasFill(false) {} ~DisplayListData(); void draw(SkCanvas* canvas) const; @@ -121,6 +121,7 @@ void applyColorTransform(ColorTransform transform); bool hasText() const { return mHasText; } + bool hasFill() const { return mHasFill; } size_t usedSize() const { return fUsed; } size_t allocatedSize() const { return fReserved; } @@ -192,6 +193,7 @@ size_t fReserved = 0; bool mHasText : 1; + bool mHasFill : 1; }; class RecordingCanvas final : public SkCanvasVirtualEnforcer<SkNoDrawCanvas> {
diff --git a/libs/hwui/RenderNode.cpp b/libs/hwui/RenderNode.cpp index 3e131bc..0b42c88 100644 --- a/libs/hwui/RenderNode.cpp +++ b/libs/hwui/RenderNode.cpp
@@ -427,7 +427,13 @@ children.push_back(node); }); if (mDisplayList.hasText()) { - usage = UsageHint::Foreground; + if (mDisplayList.hasFill()) { + // Handle a special case for custom views that draw both text and background in the + // same RenderNode, which would otherwise be altered to white-on-white text. + usage = UsageHint::Container; + } else { + usage = UsageHint::Foreground; + } } if (usage == UsageHint::Unknown) { if (children.size() > 1) { @@ -453,8 +459,13 @@ drawn.join(bounds); } } - mDisplayList.applyColorTransform( - usage == UsageHint::Background ? ColorTransform::Dark : ColorTransform::Light); + + if (usage == UsageHint::Container) { + mDisplayList.applyColorTransform(ColorTransform::Invert); + } else { + mDisplayList.applyColorTransform(usage == UsageHint::Background ? ColorTransform::Dark + : ColorTransform::Light); + } } void RenderNode::pushStagingDisplayListChanges(TreeObserver& observer, TreeInfo& info) {
diff --git a/libs/hwui/pipeline/skia/SkiaDisplayList.h b/libs/hwui/pipeline/skia/SkiaDisplayList.h index e5bd5c9..b9dc1c4 100644 --- a/libs/hwui/pipeline/skia/SkiaDisplayList.h +++ b/libs/hwui/pipeline/skia/SkiaDisplayList.h
@@ -96,6 +96,8 @@ bool hasText() const { return mDisplayList.hasText(); } + bool hasFill() const { return mDisplayList.hasFill(); } + /** * Attempts to reset and reuse this DisplayList. *
diff --git a/libs/hwui/tests/unit/RenderNodeTests.cpp b/libs/hwui/tests/unit/RenderNodeTests.cpp index 8273524..e727ea8 100644 --- a/libs/hwui/tests/unit/RenderNodeTests.cpp +++ b/libs/hwui/tests/unit/RenderNodeTests.cpp
@@ -331,3 +331,31 @@ EXPECT_EQ(uirenderer::Rect(0, 0, 200, 400), info.layerUpdateQueue->entries().at(0).damage); canvasContext->destroy(); } + +TEST(RenderNode, hasNoFill) { + auto rootNode = + TestUtils::createNode(0, 0, 200, 400, [](RenderProperties& props, Canvas& canvas) { + Paint paint; + paint.setStyle(SkPaint::Style::kStroke_Style); + canvas.drawRect(10, 10, 100, 100, paint); + }); + + TestUtils::syncHierarchyPropertiesAndDisplayList(rootNode); + + EXPECT_FALSE(rootNode.get()->getDisplayList().hasFill()); + EXPECT_FALSE(rootNode.get()->getDisplayList().hasText()); +} + +TEST(RenderNode, hasFill) { + auto rootNode = + TestUtils::createNode(0, 0, 200, 400, [](RenderProperties& props, Canvas& canvas) { + Paint paint; + paint.setStyle(SkPaint::kStrokeAndFill_Style); + canvas.drawRect(10, 10, 100, 100, paint); + }); + + TestUtils::syncHierarchyPropertiesAndDisplayList(rootNode); + + EXPECT_TRUE(rootNode.get()->getDisplayList().hasFill()); + EXPECT_FALSE(rootNode.get()->getDisplayList().hasText()); +}