[ganesh] Fix clip containment misjudgment due to integer bounds rounding

[Jason Bourne]

[Skia Issue] Residual 1px line at view edge after scale animation: ganesh ClipStack drops the device clip restriction due to integer-rounded containment checks

1. Summary

On Android 14 and later (still present on A16 and current mainline), a View that plays a scale animation with overshoot (scale > 1.0) can leave a permanent 1px residual line just outside its bounds. The residue appears only under partial redraw, on the side where the View is flush against its parent's edge.

Root cause: in external/skia/src/gpu/ganesh/ClipStack.cpp, the clip-element redundancy checks ClipStack::SaveRecord::contains(const RawElement&) and ClipStack::RawElement::contains(const RawElement&) prove "element B is contained in element A, so A is redundant and can be dropped" using integer bounds that are produced by round()-ing the float device bounds to pixel centers. For a non-AA rect clip element, that rounding can snap an edge inward by up to ~0.5px (e.g. 39.77 → 40), making the element look strictly tighter than its float geometry really is. As a result, HWUI's device clip restriction (the damage rect, injected by SkiaPipeline::renderFrameImpl via androidFramework_setDeviceClipRestriction) is wrongly treated as redundant and dropped. The draw op then runs completely unclipped, and its coverage-AA edge pixels are rasterized outside the damage rect. Since partial redraw never repaints that column, the pixels persist as a visible 1px line.

2. Affected versions

Version	Status
Android 12 (GrReducedClip)	Not affected — the old clip path reduces all rect clips into one hard scissor by intersection; there is no "prove redundant and drop" logic, so the damage boundary can never be dropped.
Android 14 (Skia M112+, new ganesh ClipStack)	Affected (verified on device)
Android 16 (Baklava)	Affected (source inspected, identical code)
AOSP mainline (android17-release)	Affected (source inspected, identical code)

3. Repro

A minimal standalone APK is attached (GreenLineRepro.apk, sources included).

Demo design (1:1 replica of a real production toggle):

• A 96dp × 96dp ImageButton with a rounded-corner (20dp) selector background (selected = opaque green #27af6e, default = translucent gray #b8ced2db), placed flush against the left edge of its parent row inside a card with a static background.
• The button plays a press-scale animation driven by drawableStateChanged(): press → shrink (170ms), release → damped oscillation that overshoots to scale ≈ 1.006 (500ms). The animation only calls setScaleX/Y, exactly like production code.

Steps:

1. Launch the app.
2. Repeatedly tap the green power button ~10 times (each tap plays shrink + overshoot).
3. Keep the button held (or simply look at it afterwards): a 1px green vertical line remains at the button's original left edge (x = button.left − 1), and never disappears.

Pixel forensics (row through button center, before/after taps):

x=1022:  (22,24,29)    background, clean
x=1023:  (22,24,29) → (91,185,143)   ← the residual line (light green)
x=1024+: (22,27,32)    card background, clean
x=1032+: (39,175,110)  button body (held = shrunk), pure green #27af6e

4. Root cause analysis

The failure is a conjunction of two clip-stack decisions; both stems from the same integer rounding hazard.

4.1 The mechanism, step by step

1. Overshoot pushes the AA fringe out of the button's integer bounds. At scale ≈ 1.006 the button's left edge moves from 40.0 to 39.71 (window space). The rounded-rect op's coverage AA therefore covers ~29% of pixel column x=39. (Painting this AA fringe outside a View's bounds is normal and by design — it is what makes animated edges look smooth; Android 12 does exactly the same.)

2. The device clip restriction should scissor that fringe off. The damage rect of the overshoot frame is [40,134,137,232] (left edge clipped to the parent, which the button is flush against). HWUI injects it as the device clip restriction: "this frame may only write inside the damage rect." Pixel column 39 is outside, and is also not in any later frame's damage, so anything written there is permanent.

3. ClipStack drops the restriction — bug. The button has clipToBounds, so its own clip element is added to the stack. Its float edge is 39.77, but the element's integer bounds are produced by GrClip::GetPixelIBounds with GrAA::kNo, which round()s to pixel centers → [40,135,136,231]. Both SaveRecord::contains(RawElement) (used by addElement → replaceWithElement) and RawElement::contains(RawElement) (used by updateForElement → markInvalid) only compare these integer bounds: [40,135,136,231] ⊆ [40,134,137,232] → "contained" → the device clip restriction is discarded as redundant. The float geometry (39.77 < 40) is never consulted.

4. The button's own clip is then dropped too (RawElement::contains(const Draw&) judges it kBOnly because the draw geometry equals the clip shape — this part is intentional, see §6). The op now executes with no clip at all.

   Decision log captured on device (instrumented ClipStack::apply), overshoot frame:

   apply enter op=CircularRRectOp bounds=[39.71 134.71 136.29 231.29] aa=1
     csOuter=[40 135 136 231] elements=2
     elem(i=1) button clipToBounds, shape=[39.77 134.77 136.23 231.23], int=[40 135 136 231]
     (i=0 device clip restriction [40 134 137 232] — already invalidated, skipped)
   → apply early kUnclipped (scissorIsNeeded=0)   ← completely unclipped
   

5. The leaked pixel is never repainted. Damage is clipped to the parent's bounds at x=40 (clipDamageToBounds), so column 39 is in no frame's damage. Every tap's overshoot adds another partial-coverage write, accumulating to the visible line (final color ≈ green + press-highlight, see §3). The line only appears on the left because that is the only side where the button is flush with the parent's edge — on the other three sides the overshoot pixels land inside the damage rect and are repainted.

4.2 Why Android 12 is unaffected

A12's GrReducedClip reduces all rect clips into one hard scissor by intersection ([40,134,137,232] ∩ [40,135,136,231] = [40,135,136,231]). An intersection needs no containment proof and cannot be "optimized away", so the damage boundary always holds. The new ganesh ClipStack (A13+/A14) introduced the element stack with redundancy elimination, and the rounding-based proof breaks it at sub-pixel boundaries.

5. Proposed fix

https://skia-review.googlesource.com/c/skia/+/1310996

6. Design note: contains(const Draw&)'s hardcoded mixed-aa=false is intentional

While analyzing, we also examined RawElement::contains(const Draw&), which passes /* mixed-aa */ false to shape_contains_rect and compares the draw's analytic bounds. This must not be "fixed": an AA draw's ±0.5px fringe beyond its analytic bounds is normal, expected rendering (it is what makes animated view edges smooth). Counting the fringe in the redundancy check would keep every such clip and snap edges to integer pixels — a global edge-aliasing regression during any fractional-scale animation. The residual line is not caused by the fringe existing, but by the fringe being written outside the damage rect, which only the device clip restriction prevents. That is why the fix belongs to the two containment checks above.

7. Attachments

Attachment	Description
GreenLineRepro.apk	Standalone minimal repro app (18KB, no dependencies; install and repeatedly tap the power switch, then hold it pressed)
GreenLineRepro_src.zip	Full sources of the repro app (MainActivity, PressScaleHelper, PressScaleImageButton, layouts, drawables, manifest)
demo_key_code.md	Quick-browse key sources of the repro app (same content as the zip, in readable form)
screenshot_before_residual.png	Symptom: residual 1px green line at the button's original left edge (x=1023) after repeated taps
screenshot_after_fixed.png	Same repro flow with the proposed fix applied: no residual line
screenshot_closeup_annotated.png	Close-up of the residual line with the detection strip annotated

8. Environment used for verification

• The same residual is 100% reproducible on Android 16 & Android 14; Android 12 devices never show it.

Could you please review the fix patch provided and determine if there are any risks associated with it or provide a new way to fix this issue?

[Michael Ludwig]

Can you please file this report with our issue tracker in issues.skia.org. I agree that there is a bug here, but I'm not convinced that it is step #3 that is the cause (this is combining both non-AA rects, so it should be valid to use the rounded pixel coordinates).  I believe the issue is that when it's testing the draw later on, it's assuming that the floating point geometry comparison is sufficient even though it's a mixed-aa case. I need to page this behavior back in to see what needs to be adjusted in contains(const Draw&) instead.