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

Implement #2655 - Add more easing formulas #3812

Merged
merged 11 commits into from Nov 3, 2023

Conversation

FoundationKen
Copy link
Contributor

WIP: I'm adding an example page to show all the easings visually.

This adds support for all easings from https://easings.net

@CLAassistant
Copy link

CLAassistant commented Oct 31, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@ogoffart ogoffart left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your PR.

Documentation would be nice, a paragraph for different easing curves should be added in https://github.com/slint-ui/slint/blob/master/docs/reference/src/language/syntax/animations.md
(idea: It could even have an example that animates something with all easing curves.)

@@ -2821,6 +2821,12 @@ fn compile_expression(expr: &llr::Expression, ctx: &EvaluationContext) -> String
"slint::cbindgen_private::EasingCurve(slint::cbindgen_private::EasingCurve::Tag::CubicBezier, {}, {}, {}, {})",
a, b, c, d
),
Expression::EasingCurve(EasingCurve::EaseInElastic) => "slint::cbindgen_private::EasingCurve()".into(),
Copy link
Member

Choose a reason for hiding this comment

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

you will have to pass something like slint::cbindgen_private::EasingCurve::Tag::EaseInElastic in the constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me know if that looks good now.

internal/core/animations.rs Outdated Show resolved Hide resolved
@FoundationKen
Copy link
Contributor Author

Thanks a lot for your PR.

Documentation would be nice, a paragraph for different easing curves should be added in https://github.com/slint-ui/slint/blob/master/docs/reference/src/language/syntax/animations.md (idea: It could even have an example that animates something with all easing curves.)

I added a list of all the types and a link to easings.net. I don't think these can be described in a paragraph, but let me know if you have another idea.

I haven't (yet) put anything dynamic in the help. Is there an example of embedding a slint demo in Markdown I can follow?

@ogoffart
Copy link
Member

ogoffart commented Nov 1, 2023

I haven't (yet) put anything dynamic in the help. Is there an example of embedding a slint demo in Markdown I can follow?

That's fine. The way to do it is just to have it in ```slint code fences like there is one earlier in that file, and it should render if the browser supports it. eg: https://slint.dev/releases/1.2.2/docs/slint/src/language/syntax/animations#animations
But I think the current animation page is good.

There are a few errors from CI:
The docs one is straightforward, just use the suggestion from the error
https://github.com/slint-ui/slint/actions/runs/6713806559/job/18246013206?pr=3812

error: this URL is not a hyperlink
   --> internal/core/animations.rs:156:37
    |
156 |     /// Easing curve as defined at: https://easings.net/#easeOutBounce
    |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<https://easings.net/#easeOutBounce>`
    |
    = note: bare URLs are not automatically turned into clickable links

The test you added is failing, I did not investigate why
https://github.com/slint-ui/slint/actions/runs/6713806559/job/18246009329?pr=3812

For the MCU cases, we need to import the Float trait as the error message says .
There is an example in some other modules. eg:

#[cfg(not(feature = "std"))]
use num_traits::Float;

And the C++ tests are also failling you have to construct the struct with its constructor

@ogoffart
Copy link
Member

ogoffart commented Nov 1, 2023

Actually, for C++, the best would be to get a constructor that initialize the tag:

slint/api/cpp/cbindgen.rs

Lines 601 to 605 in 2176523

config.export.body.insert(
"EasingCurve".to_owned(),
" constexpr EasingCurve() : tag(Tag::Linear), cubic_bezier{{0,0,1,1}} {}
constexpr explicit EasingCurve(EasingCurve::Tag tag, float a, float b, float c, float d) : tag(tag), cubic_bezier{{a,b,c,d}} {}".into()
);

Replace with something like:

config.export.body.insert( 
         "EasingCurve".to_owned(), 
         "    constexpr explicit EasingCurve(EasingCurve::Tag tag = Tag::Linear , float a = 0, float b = 0, float c = 1, float d = 1) : tag(tag), cubic_bezier{{a,b,c,d}} {}".into() 
     ); 

constexpr explicit EasingCurve(EasingCurve::Tag tag, float a, float b, float c, float d) : tag(tag), cubic_bezier{{a,b,c,d}} {}".into()
config.export.body.insert(
"EasingCurve".to_owned(),
" constexpr explicit EasingCurve(EasingCurve::Tag tag = Tag::Linear , float a = 0, float b = 0, float c = 1, float d = 1) : tag(tag), cubic_bezier{{a,b,c,d}} {}".into()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
" constexpr explicit EasingCurve(EasingCurve::Tag tag = Tag::Linear , float a = 0, float b = 0, float c = 1, float d = 1) : tag(tag), cubic_bezier{{a,b,c,d}} {}".into()
" constexpr EasingCurve(EasingCurve::Tag tag = Tag::Linear , float a = 0, float b = 0, float c = 1, float d = 1) : tag(tag), cubic_bezier{{a,b,c,d}} {}".into()

It shouldn't have been explicit, sorry to have mislead you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries...How can I test this locally? I'm not doing anything with CPP and Slint.

Copy link
Member

Choose a reason for hiding this comment

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

You can run the driver test like so.

cargo test -p  test-driver-cpp

But I'm not sure this test any of the new curves since they are not in the tests but in the gallery.
To build the gallery, you need cmake:

https://github.com/slint-ui/slint/blob/master/docs/building.md#c-api-build

Copy link
Member

@ogoffart ogoffart left a comment

Choose a reason for hiding this comment

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

Thanks a lot. This looks good to me.
It's missing a change log entry but we can add that separately.

Screenshot of the new gallery tab for the other reviewers:

image


Rectangle {
border-radius: 8px;
background: #F4F4F4;
Copy link
Member

Choose a reason for hiding this comment

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

This hardcoded color cause issues in dark mode, so i'll remove it.

@ogoffart ogoffart merged commit b205361 into slint-ui:master Nov 3, 2023
31 checks passed
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.

None yet

3 participants