Skip to content

Add linear and radial gradients for shape fills - #123

Open
scorsin-oai wants to merge 2 commits into
Snapchat:mainfrom
scorsin-oai:simon/260723-shape-fill-gradients
Open

Add linear and radial gradients for shape fills#123
scorsin-oai wants to merge 2 commits into
Snapchat:mainfrom
scorsin-oai:simon/260723-shape-fill-gradients

Conversation

@scorsin-oai

Copy link
Copy Markdown
Collaborator

Description

This change adds support for fillGradient on the shape element.

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/snap-drawing SnapDrawing library labels Jul 23, 2026
@scorsin-oai

Copy link
Copy Markdown
Collaborator Author

Will add iOS impl before merging the PR

@github-actions github-actions Bot added the platform/ios iOS-specific label 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 & Export ✅ success
Valdi Smoke Tests ✅ success
API Surface Check ✅ success
Linux: Build Compiler ✅ success
macOS: C++ & Platform Tests ✅ success
Linux: C++ Tests ✅ success
Snapshot Tests ✅ success

All Bazel configuration and CI tests passed!

The build system and core tooling are working correctly.

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

Workflow: Valdi CI

view.resetFillGradient()
} else if (valdiGradient.colors.size == 1) {
view.resetFillGradient()
view.setFillColor(valdiGradient.colors[0])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like the one-color path replaces the shape's stored fillColor. Android calls setFillColor() here, while iOS similarly assigns the gradient color to _fillColor. When the gradient is removed, the reset path restores the gradient color rather than the original bound fill color.

tested this by starting with fillColor="green", applying a one-color red gradient, and then removing the gradient. The shape stayed red on both Android and iOS. Just wanted to double check if that was intended

)
}
GradientDrawable.RADIAL_GRADIENT -> {
fillPaint.shader = RadialGradient(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When either shape dimension is zero, the calculated radius is also zero. Android's RadialGradient requires a radius greater than zero, so constructing the shader in that state crashes.

I double-checked this by changing the shape's width and height to zero independently. Android crashed in both cases, but iOS handled them.

Since zero bounds can occur temporarily during layout, could we skip creating the shader until valid bounds return?

fillPaint.shader = RadialGradient(
fillGradientBounds.centerX(),
fillGradientBounds.centerY(),
min(fillGradientBounds.width(), fillGradientBounds.height()) / 2.0f,

@afan-sc afan-sc Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like Android calculates one circular radius using min(width, height) / 2, while iOS and SnapDrawing scale the radial gradient with the non-square bounds. This seems to produce different results for rectangular shapes. Tested the same red-to-blue gradient in a 2:1 shape. Android rendered a circle, while iOS and SnapDrawing rendered an ellipse.

Is the Android-specific behavior intentional? If not, could we align it with the other backends and add a non-square radial test?

auto fillPaint = _fillPaint;
if (_fillGradientWrapper.hasGradient()) {
fillPaint.setColor(Color::black());
_fillGradientWrapper.update(path.getBounds().value_or(drawingContext.drawBounds()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like SnapDrawing sizes the gradient using the path bounds, while Android and iOS use the full shape layer. This can produce different results when the path does not occupy the complete layer.

I double-checked with a 120×100 layer whose path only occupies the top half. The Android and iOS versions ended around purple, while SnapDrawing reached blue.

Is the path-relative behavior expected here? If not, would using the drawing/layer bounds make the backends more consistent?

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

Labels

area/runtime Valdi runtime (C++/native) area/snap-drawing SnapDrawing library platform/ios iOS-specific

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants