Skip to content

Enable WAAPI acceleration for backgroundColor and color#3763

Open
mattgperry wants to merge 1 commit into
mainfrom
advisor/003-color-waapi
Open

Enable WAAPI acceleration for backgroundColor and color#3763
mattgperry wants to merge 1 commit into
mainfrom
advisor/003-color-waapi

Conversation

@mattgperry

Copy link
Copy Markdown
Collaborator

Implements plan 003 (plans/003-color-waapi-acceleration.md).

Root cause

Color animations (backgroundColor, color, …) were deliberately excluded from WAAPI acceleration — a code comment deferred them "until we implement support for linear() easing". That blocker is resolved: Motion already generates linear() easing strings for unsupported easings/springs and wires them into the WAAPI path (map-easing.ts:14-16, spring.ts:433). As a result, plain color animations fell to the per-frame JS path, re-rendering the element's styles every frame on the main thread.

Approach

Add "backgroundColor" and "color" (camelCase Motion value names) to acceleratedValues, replacing the dead dash-case // "background-color" comment (which never would have matched the camelCase name anyway). supports/waapi.ts needed no change — the existing acceleratedValues.has(name) branch covers it.

Honest framing: color is paint-bound, so this is a main-thread offload, not free compositor rendering. Chrome additionally composites solid-color background-color animations on top of that.

Scope kept minimal:

  • The browser-only-color path (oklch/lab/lch/color-mix) is unchanged — that branch is correctness (the JS path can't parse those formats), not perf.
  • Remaining colors (borderColor family, fill/stroke) intentionally left out; SVG is already excluded by the HTMLElement check.

Tests

  • Unit (the real regression gate, per CLAUDE.md): extended supports/__tests__/waapi.test.ts — standard-keyframe backgroundColor/color are now eligible; onUpdate kill-switch still applies; a color not in the set (borderTopColor) is ineligible with standard keyframes but eligible with oklch. The two new positive assertions were watched failing first.
  • E2E (Playwright, real Chromium): animate-color-accelerated.html asserts getAnimations().length > 0 (it's now a WAAPI animation) and a strictly-between mid-animation colour; animate-color-interrupt.html asserts an interrupt lands on the new target with no jump and no console errors.

Gates run

  • yarn build
  • motion-dom jest — 475 pass ✅
  • framer-motion test-client — 799 pass ✅
  • Playwright animate suite (chromium) — 58 pass ✅
  • Cypress color specs (oklch-color-animation, animate-style) on React 18 and 19 — 10/10 each ✅
  • Spring-on-color STOP-condition check (manual throwaway fixture): valid colours mid-animation, no NaN, settles on target ✅
  • yarn lint: fails only in pre-existing dev/react fixtures (zero diff vs main, untouched here); motion-dom is unlinted by yarn lint.

Deviations from the plan

None. No files outside the in-scope list were modified.

Notes for reviewers

  • No change to the hasBrowserOnlyColors branch semantics.
  • Safari de-accelerates linear() easing, so color animations with spring/custom easings run WAAPI-on-main-thread there — parity with today, worth a release-notes line.

🤖 Generated with Claude Code

Color animations were excluded from WAAPI acceleration pending linear()
easing support, which has since landed (map-easing/spring both generate
linear() strings). Plain color animations therefore fell to the per-frame
JS path, re-rendering styles every frame on the main thread.

Add "backgroundColor" and "color" (camelCase Motion value names) to
acceleratedValues so they route through WAAPI. This is a main-thread
offload (color is paint-bound, not free compositor rendering); Chrome
additionally composites solid-color background-color animations.

The browser-only-color path (oklch/lab/etc.) is unchanged. Remaining
colors (borderColor family, fill/stroke) are intentionally left out.

Implements plan 003.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jun 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR enables WAAPI acceleration for backgroundColor and color by adding them (as camelCase Motion value names) to the acceleratedValues set, replacing a dead dash-case comment that never would have matched. The pre-existing supportsBrowserAnimation checks (onUpdate kill-switch, HTMLElement guard, mirror/inertia/zero-damping bail-outs) all apply unchanged, so the scope is minimal.

  • Core change: two entries added to accelerated-values.ts; supports/waapi.ts is untouched.
  • Tests: unit tests cover the new positive paths, the onUpdate guard, and borderTopColor exclusion; two Playwright E2E fixtures verify WAAPI delivery and interrupt correctness on real Chromium.
  • Out of scope: borderColor family and fill/stroke remain excluded; the hasBrowserOnlyColors branch semantics are unchanged.

Confidence Score: 4/5

Safe to merge. The change is two lines in one file; all existing guards (onUpdate, HTMLElement, mirror/inertia/zero-damping) remain active and the hasBrowserOnlyColors branch is untouched.

The core change is minimal and correct — the old dash-case entry was dead code and the new camelCase entries are properly gated by all existing bail-outs. Unit tests confirm both directions, and real-Chromium Playwright tests cover WAAPI delivery and interrupt handling. The only note is that one mid-animation green-channel assertion uses strict toBe(0) rather than a tolerance, which is correct today but would give an opaque failure if the interpolation color space ever changes.

No files require special attention. tests/animate/animate.spec.ts has a minor strict-equality assertion on the green channel that would be slightly more resilient with a tolerance, but it is not incorrect.

Important Files Changed

Filename Overview
packages/motion-dom/src/animation/waapi/utils/accelerated-values.ts Adds "backgroundColor" and "color" (camelCase) to the accelerated values set, replacing the dead dash-case comment that never matched anything.
packages/motion-dom/src/animation/waapi/supports/tests/waapi.test.ts Adds unit tests for backgroundColor/color eligibility, onUpdate kill-switch, borderTopColor exclusion, and the browser-only-color path for non-accelerated properties.
tests/animate/animate.spec.ts Adds two Playwright E2E tests: mid-animation WAAPI assertion on backgroundColor and interrupt-lands-on-target verification; relies on page.reload() + 500ms / 1500ms timing windows.
dev/html/public/playwright/animate/animate-color-accelerated.html New Playwright fixture: 10s linear red-to-blue backgroundColor animation, used to assert getAnimations().length > 0 and mid-progress color values.
dev/html/public/playwright/animate/animate-color-interrupt.html New Playwright fixture: starts 10s red-to-blue animation, interrupts at 500ms with a 300ms green animation, asserts no jump and correct final color.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[supportsBrowserAnimation called] --> B{subject instanceof HTMLElement?}
    B -- No --> Z[return false]
    B -- Yes --> C{supportsWaapi?}
    C -- No --> Z
    C -- Yes --> D{acceleratedValues.has name?}
    D -- "Yes\nopacity / clipPath / filter / transform\nbackgroundColor / color" --> E{name === transform\nAND transformTemplate?}
    D -- No --> F{colorProperties.has name\nAND hasBrowserOnlyColors keyframes?}
    F -- "Yes\nborderColor etc. with oklch/lab/lch" --> E
    F -- No --> Z
    E -- Yes --> Z
    E -- No --> G{onUpdate set?}
    G -- Yes --> Z
    G -- No --> H{repeatDelay / mirror\n/ damping=0 / inertia?}
    H -- Any true --> Z
    H -- None --> Y[return true - WAAPI path]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[supportsBrowserAnimation called] --> B{subject instanceof HTMLElement?}
    B -- No --> Z[return false]
    B -- Yes --> C{supportsWaapi?}
    C -- No --> Z
    C -- Yes --> D{acceleratedValues.has name?}
    D -- "Yes\nopacity / clipPath / filter / transform\nbackgroundColor / color" --> E{name === transform\nAND transformTemplate?}
    D -- No --> F{colorProperties.has name\nAND hasBrowserOnlyColors keyframes?}
    F -- "Yes\nborderColor etc. with oklch/lab/lch" --> E
    F -- No --> Z
    E -- Yes --> Z
    E -- No --> G{onUpdate set?}
    G -- Yes --> Z
    G -- No --> H{repeatDelay / mirror\n/ damping=0 / inertia?}
    H -- Any true --> Z
    H -- None --> Y[return true - WAAPI path]
Loading

Reviews (1): Last reviewed commit: "Enable WAAPI acceleration for background..." | Re-trigger Greptile

Comment on lines +555 to +559
expect(r).toBeLessThan(255)
expect(r).toBeGreaterThan(0)
expect(b).toBeLessThan(255)
expect(b).toBeGreaterThan(0)
expect(g).toBe(0)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The g channel assertion uses strict equality. For a WAAPI animation from rgb(255,0,0) to rgb(0,0,255) on Chromium, sRGB linear interpolation keeps green at exactly 0 throughout, so this is correct today. If the animation path ever changes color space (e.g. perceptual interpolation), the strict toBe(0) will break with no obvious failure message. A small tolerance would make the intent clearer and guard against future regressions.

Suggested change
expect(r).toBeLessThan(255)
expect(r).toBeGreaterThan(0)
expect(b).toBeLessThan(255)
expect(b).toBeGreaterThan(0)
expect(g).toBe(0)
expect(r).toBeLessThan(255)
expect(r).toBeGreaterThan(0)
expect(b).toBeLessThan(255)
expect(b).toBeGreaterThan(0)
expect(g).toBeLessThan(10)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant