Skip to content

Add transform origins and CSS-style composed transforms - #124

Open
scorsin-oai wants to merge 1 commit into
Snapchat:mainfrom
scorsin-oai:simon/260723-css-transforms
Open

Add transform origins and CSS-style composed transforms#124
scorsin-oai wants to merge 1 commit into
Snapchat:mainfrom
scorsin-oai:simon/260723-css-transforms

Conversation

@scorsin-oai

Copy link
Copy Markdown
Collaborator

Description

This change adds support for the transform attribute which works like on Web and allows an arbtrary set of transforms to be applied in sequence. It also adds suport for the transformOrigin attribute which tells how the transform should be applied.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • Documentation improvement
  • Performance optimization
  • Test improvement
  • Other (please describe)

Testing

  • Tests pass locally (bazel test //...)
  • Added/updated tests for changes (if applicable)
  • Tested on multiple platforms (iOS/Android/Web/macOS as applicable)
  • Manual testing performed (describe below)

Testing Details

Checklist

  • Code follows project style guidelines
  • Documentation updated (if needed)
  • No breaking changes (or documented in description)
  • Commit messages follow conventional format
  • No secrets, API keys, or internal URLs included

Related Issues

Additional Context

@github-actions github-actions Bot added area/runtime Valdi runtime (C++/native) area/docs Documentation platform/ios iOS-specific platform/android Android-specific labels Jul 23, 2026
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown

⚠️ Bazel & CI Test Results

Test Suite Result
Linux: Build Compiler ✅ success
API Surface Check ✅ success
Linux: C++ Tests ✅ success
Valdi Smoke Tests ✅ success
Linux: Build & Export ✅ success
Snapshot Tests ✅ success
macOS: C++ & Platform Tests ❌ failure

Some tests failed. Please check the workflow logs for details.

🚀 Bazel remote cache is now enabled - future builds will be faster!

Workflow: Valdi CI

Comment on lines 300 to 304
setTransformElement(view,
resolvedTranslationX,
translationX,
animator,
TRANSLATION_X_KEY,
{ translationX },

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

setTransformElement declares getValue: View.() -> Float. Kotlin resolves an unqualified identifier in the lambda body to the outer local val translationX (target value), not this.translationX on the View receiver. The animator's "current value" always equals the target, so the delta is zero and animation is skipped.

Same shadowing bug repeats for translationY, scaleX, scaleY, rotation.

iOS uses a different binder path (UntypedAttributeHandlerDelegate in SCValdiAttributesBinder.mm) that doesn't have this shadow, so it animates fine.

setTransformElement(view,
        translationX,
        animator,
        TRANSLATION_X_KEY,
        { translationX },              // <-- reads the LOCAL val, not view.translationX
        { view.translationX = it },

Apply to all five setTransformElement call sites in applyTransform.

return postprocessResolvedTransform(resolvedTransform.moveValue(), resolvedWidth, resolvedHeight, isRightToLeft);
}

Result<Value> TransformAttributes::postprocessViewNode(ViewNode& viewNode, const Value& value) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The CSS transform string (and its percentage resolution) is processed exclusively via the C++ TransformAttributes::postprocessViewNode postprocessor. Two severe defects follow:

  1. Stale absolute pixels on resize. postprocessViewNode resolves percentage-based translations and transformOrigin strings into an array of fixed double values (pixels) based on the ViewNode's current calculatedFrame. Because CSS attributes are evaluated before layout, getCalculatedFrame() is initially 0x0, so percentages evaluate to 0. When the ViewNode is later resized during layout (calculatedSizeDidChange), transformComposite is not marked dirty. The native view never receives the updated absolute values, breaking responsive transformations.
  2. Hit-testing and bounds disjoint. Unlike the individual translationX and translationY attributes - which are explicitly intercepted in ViewNodeAttributesApplier::processAttributeChange to update the ViewNode's internal bounds - the transform string attribute has no such interception (no id == DefaultAttributeTransform handler). Translations applied via the transform string therefore bypass the node's internal state entirely. Touch hit-testing misses the visual element and scroll-culling clips it prematurely.

Suggested fix: TransformAttributes should parse the strings into a structured representation that preserves percentages, and push that down to the native layer to handle dynamically on resize (similar to how BorderRadius and gradients work). Additionally, ViewNode must parse the transform string to
update its internal _translationX and _translationY states.

Verified via code_search on ViewNodeAttributesApplier::processAttributeChange confirming no handling for DefaultAttributeTransform exists, and checking ViewNode.cpp confirming postprocessors are not re-evaluated on calculatedSizeDidChange.

b * other.e + d * other.f + f);
}

Result<TransformComponents> TransformMatrix::decompose() const {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Where: valdi/src/valdi/runtime/Attributes/TransformAttributes.cppTransformMatrix::decompose() (near line 133).

Symptom: The decompose() algorithm rejects scale factors near zero, returning Error("transform scaleX must not resolve to zero"). This breaks transform="scale(0)" strings, which are common in enter/exit animations.

Cause: When the X-axis is squashed (scaleX=0), std::hypot(a, b) becomes 0, so the algorithm cannot use it as a divisor for the Y-axis length and rotation. However, c and d still describe the scaled and rotated Y-axis correctly.

Suggested fix: extract scaleY and rotation directly from the Y-axis components in the degenerate case instead of failing.

if (std::abs(resolvedScaleX) < epsilon) {
    auto resolvedScaleY = std::hypot(c, d);
    auto resolvedRotation = std::atan2(-c, d);
    return TransformComponents(e, f, 0.0, resolvedScaleY, resolvedRotation);
}

return ResolvedTransform(components, origin);
}

Value postprocessResolvedTransform(ResolvedTransform resolvedTransform,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When isRightToLeft is true, postprocessResolvedTransform correctly negates translationX and rotation, but fails to horizontally mirror origin.x.

Cause: Because Valdi dynamically simulates transformOrigin by injecting physical translation offsets (adjustedTranslationX), failing to mirror the origin causes rotation/scaling to pivot around the wrong edge in RTL. For example, transformOrigin = "top left" pivots around the physical left edge in both LTR and RTL. Combined with the negated rotation angle, the view swings up and right (instead of out and left), completely breaking the trajectory and introducing large Y-axis displacement.

Suggested fix: mirror origin.x against the view's width in the RTL branch.

if (isRightToLeft) {
    components.translationX *= -1.0;
    components.rotation *= -1.0;
    origin.x = width - origin.x;
}

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

Labels

area/docs Documentation area/runtime Valdi runtime (C++/native) platform/android Android-specific platform/ios iOS-specific

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants