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());
+}