Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

shadow and slider #819

Draft
wants to merge 33 commits into
base: main
Choose a base branch
from
Draft

shadow and slider #819

wants to merge 33 commits into from

Conversation

Cupnfish
Copy link
Contributor

shadow.mp4

This is a simple implementation with many areas that can still be improved. It serves as a reference for those who want to implement corresponding functionalities in the future. Currently, I haven't thoroughly researched the testing part, and the tests are mostly not passing. However, it can run a simple example

- add `corner_radius` field to `ShadowStyle` struct to allow custom shadow corner radius
- update `shadow` method to accept `corner_radius` parameter
- modify shadow rendering logic to use `draw_blurred_rounded_rect_in` with specified corner radius
- update tests to include `None` for `corner_radius` parameter
- add snapshot tests for label box with shadow, shadow with border, and shadow with rounded corners
snooowfire and others added 3 commits January 11, 2025 00:14
- Added support for vertical orientation in slider widget
- Implemented hover glow effect with customizable color, blur, and spread radius
- Added track color customization
- Introduced step functionality for precise value selection
- Improved slider interaction with grab anchor and editing state tracking
- Enhanced visual appearance with rounded corners for thumb and track
- Added comprehensive test cases for new slider functionality
- Updated shadow example to demonstrate new slider capabilities
- Improved documentation and code organization for slider widget
@DJMcNab
Copy link
Member

DJMcNab commented Jan 14, 2025

A few notes:

  • What is @snooowfire's role in this PR?
  • It might be worth splitting this PR into two PRs the slider (first?) and the shadow.
  • Shadow might be able to use draw_blurred_rounded_rect (optionally, because it's somewhat expensive).

@PoignardAzur
Copy link
Contributor

It is?

@DJMcNab
Copy link
Member

DJMcNab commented Jan 14, 2025

Sure - the per-pixel maths is significantly more expensive than just blitting a solid colour. Unless you mean that we should have a fast-path in Vello (i.e. splitting blurred_rounded_rect into the solid part and the non-solid part), which I guess would be not too unreasonable

@PoignardAzur
Copy link
Contributor

Sure - the per-pixel maths is significantly more expensive than just blitting a solid colour.

Oh, I thought you meant it had some sort of hidden cost.

This specific use-case is the one we added blurred rectangles for, so I wouldn't worry about the per-pixel cost.

Comment on lines +594 to +606
let shadow_rect = size
.to_rect()
.inset(-shadow.spread_radius)
.to_rounded_rect(corner_radius);

scene.draw_blurred_rounded_rect_in(
&shadow_rect,
Affine::translate(shadow.offset),
shadow_rect.rect(),
shadow.color,
shadow.corner_radius.unwrap_or(corner_radius.top_left),
shadow.blur_radius,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you using the clipped version of the blurred rect? The _in version. The unclipped version is how I would expect it to look.
image

Copy link
Member

Choose a reason for hiding this comment

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

(Oh... It already is using blurred_rounded_rect... Oops)

@Cupnfish
Copy link
Contributor Author

@snooowfire is my secondary account. Some of the code was developed on my WSL, where I logged in with my secondary account's git. I think it’s indeed worth separating into two PRs. There are still many tests to write for the slider part, but I’m not very familiar with how to properly implement tests for masonry. If I have time, I will prioritize splitting out and submitting the slider implementation first, followed by the shadow implementation.

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.

5 participants