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

feat: color: add channel properties to color plus linear-mix #4796

Merged
merged 1 commit into from Mar 12, 2024

Conversation

flukejones
Copy link
Contributor

Add extra properties to the color type.

  • red
  • green
  • blue
  • alpha

Add extra method to the color type

  • linear-mix This provides a simple linear mixing of two ARGB colors with a scaling factor

@flukejones
Copy link
Contributor Author

This is a much cleaned up version of the previous PR with extraneous work pulled out to a separate branch. I think everything is covered, but lets see what the pipeline tests say.

@flukejones
Copy link
Contributor Author

I'm not sure what this is about:

 /tmp/.tmpmBbGeV.cpp:268:800: error: invalid use of member function ‘uint8_t slint::Color::green() const’ (did you forget the ‘()’ ?)
  268 |                             return [&]{ auto tmp_root_window_1_b2 = self->root_window_1_b2.get();;auto tmp_root_window_1_linearmix = self->root_window_1_linearmix.get();;return ((((((((self->root_window_1_b1.get() == tmp_root_window_1_b2) && (tmp_root_window_1_b2 == self->root_window_1_b5.get())) && (self->root_window_1_b3.get() == slint::Color::from_argb_encoded(+4.278190335e9))) && (slint::Color::from_argb_encoded(+4.29490176e9) == self->root_window_1_r4.get())) && (self->root_window_1_y1.get() == slint::Color::from_argb_uint8(std::clamp(static_cast<float>(1) * 255., 0., 255.), std::clamp(static_cast<int>(255), 0, 255), std::clamp(static_cast<int>((1 * 255)), 0, 255), std::clamp(static_cast<int>(0), 0, 255)))) && (tmp_root_window_1_linearmix.red == 111)) && (tmp_root_window_1_linearmix.green == 156)) && (tmp_root_window_1_linearmix.blue == 68)); }();

Does this mean that the methods and struct fields are in conflict?

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 the PR. Some comment inside

internal/core/graphics/color.rs Outdated Show resolved Hide resolved
docs/reference/src/language/syntax/types.md Outdated Show resolved Hide resolved
docs/reference/src/language/syntax/types.md Outdated Show resolved Hide resolved
@flukejones flukejones force-pushed the feat/color_methods branch 2 times, most recently from 35f3557 to f9d61e0 Compare March 8, 2024 23:30
@ogoffart
Copy link
Member

Do you mean adding an example in code? It's pretty simple - if I change this from interpolate to mix I get a jump when the slider switches to the next colour.

Sorry to insist, but i still don't understand what you mean. (I cannot try your example since it needs a custom Slint)
Would you have a screenshot that shows the difference?

(I've replicated a way to compare different way to mix color currently afailable, but they seem similar: link)

@flukejones
Copy link
Contributor Author

@ogoffart I must be losing my mind... I could have sworn one was smoother when stepping from one colour to the next. The mix() gives a nicer blend for sure. It seems I've done something somwhere in my own code that now they both give the same effect.

A screenshot doesn't work here as it is the visible movement from one colour to the next in a two colour mix(n, n+1) selected from an array..

I am now non-plussed on whether this part stays in or not. If you prefer to remove it I'm okay with that. Though I do recall the mix() had a lot more calculation done compared to interpolate so maybe that could be a helpful distinction?

@ogoffart
Copy link
Member

Yes, please remove the interpolate function, as it is a duplicate of mix and there should be good explanation of the difference in the docs as to when to use one or another. But I think it is not needed to have it.

Add extra properties to the `color` type.
- `red`
- `green`
- `blue`
- `alpha`
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. Looks good.

@ogoffart ogoffart merged commit 8c60cc7 into slint-ui:master Mar 12, 2024
36 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

2 participants