feat(animations): hierarchical animation event tree with spring physics#291
Open
fuddlesworth wants to merge 4 commits intov3from
Open
feat(animations): hierarchical animation event tree with spring physics#291fuddlesworth wants to merge 4 commits intov3from
fuddlesworth wants to merge 4 commits intov3from
Conversation
fuddlesworth
added a commit
that referenced
this pull request
Apr 4, 2026
… physics, and accessibility Fix 20 issues found during PR #291 review: Critical: add animationProfilesGroup to managedGroupNames so reset clears animation profiles; replace tr() with PzI18n::tr() in settingscontroller; replace qsTr() with i18n() in CurvePresets singleton; fix empty presets not persisting on save; add ConfigDefaults accessor for profile data key. Bugs: wire initialVelocity into all three spring damping cases; harden estimatedDuration with forward scan requiring consecutive settled samples; fix CurveEditorDialog onApplied to use _workingCurve; wrap springLabel strings in i18n(). Style: use fuzzy compare for styleParam in AnimationProfile::operator==; use ConfigDefaults for spring fallback values; replace hardcoded colors, spacing, and fonts with Kirigami theme values; add Accessible.name to interactive QML components; fix unicode star i18n; fix dead binding in AnimationsGeneralPage. Co-Authored-By: claude-flow <[email protected]>
fuddlesworth
added a commit
that referenced
this pull request
Apr 4, 2026
Address 29 issues from PR #291 review across 34 files: Critical: fix dangling shader pointers on reconfigure, validate windows before unredirect in prePaintScreen, align spring critical-damping threshold between JS preview (was 0.001) and C++ runtime (1e-9). High: cache estimatedDuration() per-animation instead of O(2000) scan per-frame, guard pixelate shaders against zero texture dimensions, clamp spring stiffness to prevent division by zero, fix file:// URL decoding for paths with spaces/special characters. Medium: add cross-reference comments for duplicated enums/functions across kwin-effect and daemon, replace hasOpacityAnimations() O(N) scan with O(1) counter, extract shared applyGeometryInterpolation() from 3 transform methods, fix validateParams to use type-aware checks instead of fragile round-trip equality, convert map keys to QLatin1String, extract AnimationSubPageHeader.qml from 5 pages, extract serializeRotationWithAnimation() and resolvedProfileOrDefault() helpers in snap engine. Low: EasingPreview imports shared EasingCurve.js module, named timingMode constants in CurvePresets, Accessible.name on all interactive elements, Kirigami.Units.gridUnit for all dimensions, fix redundant ternary, regex-based normalizeCurve, add missing "Use as Default" for user spring presets, rename constant to PascalCase, optimize glitch shader to 3 texture lookups. Co-Authored-By: claude-flow <[email protected]>
443bd63 to
6d402a8
Compare
fuddlesworth
added a commit
that referenced
this pull request
Apr 4, 2026
Address all findings from full PR #291 review: - Extract shared AnimationStyle + TimingMode enums into header-only src/common/animationstyle.h, eliminating duplication between core and kwin-effect (DRY-1) - Clamp spring params (dampingRatio, stiffness, epsilon) via qBound in both fromJson() paths to prevent NaN from malformed JSON (BUG-2) - Add std::isfinite guard in SpringAnimation::evaluate() (SUG-2) - Clamp epsilon >= 0.00001 in isSettled() to prevent infinite loops (BUG-3) - Mark redirected custom-shader windows translucent in prePaintWindow (BUG-1) - Validate shader file paths against allowed directories (ISSUE-1) - Pre-compute cachedSpringDuration in startAnimation() instead of lazy computation on compositor thread (ISSUE-4) - Report estimated duration for spring animations instead of 0 (ISSUE-3) - Move userAnimationPresetsChanged signal from ISettings to Settings (ISSUE-6) - Add static_assert guard for bounce decay ratio (ISSUE-7) - Replace hardcoded pixel sizes with Kirigami.Units.gridUnit in EasingPreview, AnimationsPresetsPage, CurveEditorDialog (KIRIGAMI-1/2/3) - Add Accessible.name/role on interactive elements in EasingPreview, SpringPreview, AnimationsGeneralPage (A11Y-1/2) - Fix FileDialog URL regex in AnimationEventCard (BUG-4) - Guard Math.sqrt(stiffness) against <= 0 in SpringPhysics.js (ISSUE-5) - Add 37 unit tests for AnimationProfile, AnimationProfileTree, SpringParams Co-Authored-By: claude-flow <[email protected]>
fuddlesworth
added a commit
that referenced
this pull request
Apr 5, 2026
Fixes 28 issues from PR #291 review: Bugs: spring velocity sign inversion, negative ring buffer index, velocity index not reset on drag start, unchecked TimingMode/duration/ styleParam from JSON, elastic easing division-by-zero. Edge cases: near-critical omegaD guard, clamped initialVelocity from JSON, spring duration cap 10s→5s, tree resolution cycle guard, resolvedProfile default fallback, dampingRatio≤0 guard, epsilon slider step alignment. DRY: resolvedProfileOrDefault() helper in WTA, AnimationSubPage.qml base component, preset filtering helpers, ConfigDefaults for spring defaults, QLatin1String consistency in shader registries and geometryutils. Style: per-window usesOpacity(), showStyleSelector:false on fade/ border/preview, Kirigami.Units for hardcoded pixels, Accessible.name on sliders, required properties on SpringPreview/CurveThumbnail. Co-Authored-By: claude-flow <[email protected]>
fuddlesworth
added a commit
that referenced
this pull request
Apr 6, 2026
…DRY, conventions Address 28 issues found during PR #291 review: Critical: fix resolvedProfile() skipping default-fill when enabled is set, canonicalize shader paths to prevent directory traversal. Major: clamp popin scale for spring overshoot, clamp negative dampingRatio, add GLSL sampler/texcoord0 declarations, delete duplicate kwin-effect/shaders animation copies (registry is sole source of truth), use ConfigKeys accessors in serialization, fix indexOf preset deletion bug. Medium: cache defaultTree(), validate easingCurve/geometry/shaderPath in fromJson(), fix isComplete() for pending animations, remove dead const_cast, add shader list refresh, bounces stepSize, event name validation. Minor: accessibility names on spring sliders, Kirigami.Units for spacing, typed properties, i18n compliance, font normalization, DRY helpers, ConfigDefaults for all bounds, registry geometry validation. Co-Authored-By: claude-flow <[email protected]>
fuddlesworth
added a commit
that referenced
this pull request
Apr 6, 2026
…s, DRY, conventions Address 30 issues from full PR #291 review across bugs, security, edge cases, DRY violations, and convention compliance. Bugs: fix integer overflow crash in drag tracker, validateParams querying wrong shader map, easing presets not updating profile tree, stale QML slider bindings, broken combo binding, isValid() hiding, refresh() bypassing shadersEnabled gate. Security: reject absolute shader paths, add trailing separator to path prefix check, validate D-Bus animation JSON with key whitelist. DRY: extract fillDefaultsFrom(), resolvedProfileOrDefault(), applySpringAsDefault(), AnimationEvents:: constants; delegate overlayShaderInfoToVariantMap to base class. Conventions: ConfigKeys→ConfigDefaults, QStringLiteral→QLatin1String for JSON keys, virtual isValid(), Accessible.name on buttons, corrected ParameterInfo slot comment, added geometry field tests. Co-Authored-By: claude-flow <[email protected]>
fuddlesworth
added a commit
that referenced
this pull request
Apr 8, 2026
…entions Critical: - Fix wrong event names in KWin drag-drop path (zoneSnapIn→snapIn) — snap animations now correctly use per-event profiles instead of global fallback - Thread-safe default vertex shader baking via std::call_once (render thread race) - Null m_sourceTexture in releaseRhiResources + null-check (dangling pointer) - Register animationShaderSubdivisions and userAnimationPresetsJson in D-Bus adaptor Architecture: - Consolidate triple-redundant tree into single constexpr TreeEdge[] source of truth - Remove misleading virtual from BaseShaderInfo (stored by value, never polymorphic) - Eliminate double metadata.json parse via output parameter on loadBaseMetadata - Fix setProgress defeating partial UBO optimization (remove spurious uniformsDirty) - Fix spring physics sign inconsistency in SpringPhysics.js (+v0→-v0 matching C++) DRY: - mergeFrom/fillDefaultsFrom consolidated via mergeOptionalField<T> template - Extract setGeometryKeys() and rotationEntryToJson() helpers (4x dedup) - Fix QStringLiteral→QLatin1String for JSON keys in rectToJson Conventions: - Rename geometry→geometryMode field across AnimationProfile, AnimationParams, WindowAnimation, AnimationShaderInfo (JSON key unchanged) - Rename animShaderInfoToVariantMap→animationShaderInfoToVariantMap - Add PLASMAZONES_SHADERS_ENABLED guard to AnimationShaderRegistry constructor - qWarning→qCWarning(lcCore) in animation shader registry - Add Kirigami.Theme color-change connections to SpringPreview/EasingPreview - Unique Accessible.name on CurveThumbnail instances in presets page - Remove deprecated AnimationsFadePage.qml alias - Remove unnecessary Item wrappers around SettingsCard - Fix stale doc comments referencing old event names - Add cycle guard to tree resolution loops 68/68 tests passing. Co-Authored-By: claude-flow <[email protected]>
fuddlesworth
added a commit
that referenced
this pull request
Apr 8, 2026
…entions - Shared tree hierarchy: create animationtreedata.h as single source of truth for animation event tree edges, eliminating duplication between animationprofile.cpp and plasmazoneseffect.cpp - ConfigKeys alignment: AnimationParams::fromJson now uses ConfigKeys accessors instead of inline QLatin1String literals - Default tree fallback: resolveAnimationFromCachedTree fills missing fields with defaults matching the daemon's defaultTree() - Bounce off-by-one: clamp arc parameter u to [0,1] via qBound - Overlay style handling: explicit switch cases for FadeIn/SlideUp/ScaleIn with debug log and morph fallback in WindowAnimator::applyTransform - usesOpacity: include overlay styles in opacity check - Opacity count guard: qMax(0,...) after every decrement to prevent negative m_opacityAnimationCount - File-watching dedup: extract watchShaderDirectory() and watchAllShaderDirectories() helpers in BaseShaderRegistry - validateParams delegation: ShaderRegistry delegates to base validateParameterValue() instead of reimplementing type checks Co-Authored-By: claude-flow <[email protected]>
fuddlesworth
added a commit
that referenced
this pull request
Apr 8, 2026
…entions Critical: fix singleton race (s_instance set before refresh), add disabled-shaders guard to AnimationShaderRegistry::refresh(). Major: return default profile when settings null, restore virtual BaseShaderInfo destructor, guard spurious curveEdited on startup, prevent stale-index preset deletion race. Minor: DRY geometry modes, restore sentinel, styleParam bounds, include order, shared AnimatedBoxTrack component, easing preset helper, dirIndex bounds, D-Bus array size cap, fix corrupted Unicode, derive test event count from AnimationTreeEdgeCount. Nits: remove redundant operator!=, unused QtMath include, tooltip delay, rename elasticAmplitude, theme color repaint handler. Co-Authored-By: claude-flow <[email protected]>
…liation Rebase feature/animation-system onto origin/v3 via squash-merge. v3 had 175 commits with parallel animation-system work (elastic/bounce curves, cubic bezier editor, compositor-common shared library, typed D-Bus structs) that deeply conflicted with the 33 commits on the feature branch. Resolution strategy: - Daemon-core/D-Bus/snap paths: take v3's version. v3 migrated to typed parameters on applyGeometryRequested and resnap signals, incompatible with the feature branch's JSON-with-animation-metadata pipeline. - kwin-effect: take v3's compositor-bridge-refactored version. Feature's animation integration was replaced with v3's simpler animator. - Overlay service: merged — keep v3's VS-aware zone selector behavior, add feature's writeAnimationProperties() for OSD/snap-assist show paths. - Animation config layer (AnimationProfile, settings, QML pages, presets, shader data/animations/*): fully preserved from feature branch. Known debt: kwin-effect currently uses v3's simpler animator (cubic bezier + elastic + bounce). Spring physics, per-event animation profiles, style transforms (Morph/Slide/Popin/SlideFade), and custom-shader vertex/fragment paths from the feature branch are NOT re-integrated on the effect side. The config/UI layer is fully wired but the effect side falls back to v3's simpler rendering. Backup branch backup/animation-system-pre-v3-rebase preserves the pre-rebase tip (7f58baa) for reference. Co-Authored-By: claude-flow <[email protected]>
…vent routing
Port the feature-branch animation system additions onto v3's kwin-effect
baseline, replacing the cubic-bezier-only animator with a full styled
animation pipeline driven by per-event profiles resolved on the daemon side.
## kwin-effect/windowanimator.{h,cpp}
- Add `SpringAnimation` (damped harmonic oscillator; underdamped / critical
/ overdamped branches) with physics evaluation, settling detection, and
`estimatedDuration()` forward-scan cached at start.
- Add `AnimationParams` parsed from resolved AnimationProfile JSON via the
existing ConfigKeys accessors.
- Add `ManagedAnimation` (local superset of compositor-common's
WindowAnimation) holding `std::variant<EasingCurve, SpringAnimation>`
timing plus style/styleParam/shader metadata. Kept local to avoid
pushing per-effect extensions into compositor-common.
- New `startAnimation(…, AnimationParams)` overload; legacy signature
delegates to it with default Morph params.
- Style-aware `applyTransform`: dispatches to Morph / Slide / Popin /
SlideFade (with opacity for the latter three). Overlay-only styles
fall back to Morph with a debug log.
- Track `m_opacityAnimationCount` so callers can enable translucent
rendering only when needed.
- Overshoot-aware `animationBounds()` samples 50 points along the
trajectory for elastic / bounce / underdamped-spring curves, and uses
`AnimationMath::computeOvershootBounds` as the fast path for
monotonic curves.
## Per-event profile routing
- New Settings D-Bus method `resolveAnimationProfile(eventName) -> JSON`.
Leverages the existing `resolvedProfileOrDefault` tree walker so
compositor plugins don't duplicate the profile-tree logic.
- `PlasmaZonesEffect::refreshAnimationProfiles()` fetches snapIn /
snapOut / snapResize / layoutSwitchIn / layoutSwitchOut via async
D-Bus at startup and on `settingsChanged`, caching parsed
`AnimationParams` keyed by event name.
- `applySnapGeometry` gains an optional `animationEvent` parameter;
`resolvedAnimationParams()` looks it up in the cache (falling back to
a Morph default) and passes it to `startAnimation`.
- Callers tagged with semantic context:
* `slotApplyGeometryRequested`: `snapResize` for size-only restores,
`snapOut` for empty-zone float-outs, `snapIn` otherwise.
* `slotApplyGeometriesBatch`: `layoutSwitchIn` for resnap / rotate,
`snapIn` for explicit snap_all.
* Deferred-apply lambda threads the event name through the
`windowFinishUserMovedResized` continuation.
## Known scope limits
- Custom GLSL shader pipeline (`AnimationStyle::Custom`) is NOT
reintegrated — it requires switching `PlasmaZonesEffect` to
`KWin::OffscreenEffect` and ~200 lines of shader loading /
redirection infrastructure. `applyTransform` treats Custom as an
alias for Morph so configured profiles still animate, just without
the bespoke shader. Tracked as follow-up.
- The `shaderPath` / `vertexShaderPath` / `shaderSubdivisions` fields
are still parsed and stored on `ManagedAnimation` so the Custom path
can be layered in later without another config-round migration.
Build clean, 78/78 tests pass.
Co-Authored-By: claude-flow <[email protected]>
7f58baa to
0596a68
Compare
…ipeline Complete the animation system reintegration by porting the custom GLSL shader pipeline onto v3's effect. Switches PlasmaZonesEffect from KWin::Effect to KWin::OffscreenEffect so windows can be redirected through a per-animation fragment / vertex shader. ## Base class switch - PlasmaZonesEffect now inherits OffscreenEffect. Constructor chains to OffscreenEffect(). `supported()` delegates to OffscreenEffect::supported() so hosts without OpenGL compositing correctly fail the probe. - Destructor calls `clearAnimationShaders()` before teardown to unredirect all active windows and release cached GLShader objects in a safe order (redirected windows must unredirect before the shaders go away). ## Shader cache + redirection helpers - `loadAnimationShader(frag, vert)` — path-traversal-guarded shader loader that prefers `*_kwin.frag` variants next to `effect.frag` (KWin's ShaderManager path vs. the QML UBO path). Caches compiled GLShaders keyed by "frag|vert" in a `std::map<QString, unique_ptr<GLShader>>`. - `redirectAnimatedWindow` / `unredirectAnimatedWindow` — thin wrappers over OffscreenEffect::redirect + setShader that also track which windows are currently redirected via `m_redirectedWindows`. - `clearAnimationShaders` — teardown helper (unredirect-then-clear-cache). - `resolveBuiltinShaderPaths(AnimationParams&)` — populates shaderPath / vertexShaderPath from `plasmazones/animations/<style>/` for Morph / Slide / Popin / SlideFade when installed; no-op for None / Custom / overlay styles, and silently falls back to pure C++ transforms when the shader bundle isn't on disk. ## Per-frame uniform binding - `apply()` override: called by OffscreenEffect during drawWindow for redirected windows. Subdivides the quad list when vertex shaders need more deformation detail, then binds the standard pz_* uniforms (progress, duration, styleParam, window_size, start/target pos & size in pixels, normalized source/target rects on the virtual screen). - paintWindow routes redirected windows through `applyGeometryOnly` so the C++ side applies translate/scale only — the fragment shader owns opacity / fade output and would otherwise double-fade. ## Animator additions - `applyGeometryOnly(window, data)` — geometry-only variant used on the shader path. Preserves the Popin center-scale and SlideFade partial- slide semantics from the C++ transforms so vertex shaders see the same pre-transform state. - `animationInfo(window)` returning a new nested `AnimationInfo` struct (progress, effective duration for springs, styleParam, start pos/size, target geometry, shader paths, subdivisions). Feeds the uniform binder in `apply()`. - New `animationFinished(EffectWindow*)` signal emitted from `advanceAnimations()` when an entry completes. The effect listens and calls `unredirectAnimatedWindow` so the OffscreenEffect texture redirection tears down in lockstep with the animator state. ## applySnapGeometry wiring - After resolving AnimationParams per event, call `resolveBuiltinShaderPaths(params)` so stock styles opt into shader rendering. If `params.shaderPath` is non-empty, redirect the window before falling through to `repaintSnapRegions`. - Every `removeAnimation` call site is paired with `unredirectAnimatedWindow` (mid-flight redirect, no-animation fallback, window-close slot, and windowDeleted lambda) to avoid leaking redirect state. Build clean, 78/78 tests pass. Shader bundle lives under `data/animations/<style>/effect.frag` and is installed by the existing `install(DIRECTORY data/animations/ ...)` rule. Co-Authored-By: claude-flow <[email protected]>
Promote kwin-effect's local ManagedAnimation struct into compositor-common's WindowAnimation, collapsing the duplicate so any future compositor plugin (Wayfire, Hyprland, ...) inherits the same styled animation state without re-deriving it from an Effect-specific superset. Follow-up from the v3 reintegration commit (b4351d0). ## compositor-common - easingcurve.h now includes `../common/animationstyle.h` and `../common/springparams.h` so SpringAnimation / AnimationStyle are visible to the shared struct. These headers stay in src/common/ so daemon code that already includes them isn't disturbed; compositor-common uses relative-path includes rather than adding new include directories. - New SpringAnimation struct (damped harmonic oscillator) — moved out of kwin-effect verbatim. Underdamped / critically damped / overdamped branches with the same near-zero and finite-value guards. - WindowAnimation replaces the plain `EasingCurve easing` field with `std::variant<EasingCurve, SpringAnimation> timing`, and gains cachedSpringDuration, style, styleParam, shaderPath, vertexShaderPath, and shaderSubdivisions. Helpers isSpring() / usesOpacity() expose the state needed by paint-path dispatch. updateProgress() / isComplete() branch on the variant. ## kwin-effect - Deletes the local SpringAnimation / ManagedAnimation definitions and their implementations (~170 lines). `src/common/*` includes go away too, since WindowAnimation now pulls them in via easingcurve.h. - WindowAnimator::m_animations and every private transform method take `WindowAnimation` again — no behavioral change, just a rename. The `AnimationInfo` snapshot, `applyGeometryOnly`, per-style transforms, and overshoot-aware animationBounds() continue to work unchanged against the unified struct. - AnimationParams stays local because `fromJson` depends on ConfigKeys accessors that live in the daemon module; it already represents the DTO boundary between daemon JSON and effect runtime state. ## animation_math / tests - createSnapAnimation() now sets `anim.timing = config.easing` (assigns into the variant slot). Functional behavior is identical. - test_compositor_common only inspects startPosition / startSize / targetGeometry / duration on the returned animation, so no test touch-up was needed. 78/78 tests pass. Net diff: -52 lines. Co-Authored-By: claude-flow <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements Phase 1 of the Extended Window Animation System (Discussion #240), combining niri-inspired spring physics with Hyprland-style hierarchical event tree configuration.
std::varianttiming, with 3 presets (Snappy/Smooth/Bouncy)docs/media/mockups/Animation Event Tree
New Files (27)
src/core/animationprofile.h/cpp— AnimationProfile, SpringParams, AnimationProfileTreesrc/settings/qml/Animations*.qml(8 pages) — General, Presets, Snap, Fades, Layout, Border, Preview, Shaderssrc/settings/qml/AnimationEventCard.qml— reusable per-event override cardsrc/settings/qml/CurveEditorDialog.qml— full curve/spring editor popupsrc/settings/qml/CurvePresets.qml— centralized preset singletonsrc/settings/qml/CurveThumbnail.qml— mini curve preview with elastic/bounce renderingsrc/settings/qml/SpringPreview.qml— spring physics visualizationdocs/media/mockups/Test plan
cmake --build build --parallel $(nproc)cd build && ctest --output-on-failure🤖 Generated with claude-flow