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

Compute gradients in Oklab color space #2055

Merged
merged 13 commits into from
Sep 8, 2023
Merged

Conversation

matze
Copy link
Contributor

@matze matze commented Aug 29, 2023

This is the simple port of https://www.shadertoy.com/view/ttcyRS to compute quad gradients in Oklab as discussed in #2043. I have very little experience with iced internals nor wgsl in general, so don't hesitate to redo as you see fit.

@matze
Copy link
Contributor Author

matze commented Sep 2, 2023

I added a small example to experiment with the outcome. What I also noticed is that the stop order is somewhat confusing because the first stop at 0.0 does not denote the left but the right side. Not sure if that is intentional or a bug …

@GyulyVGC
Copy link
Contributor

GyulyVGC commented Sep 2, 2023

What I also noticed is that the stop order is somewhat confusing because the first stop at 0.0 does not denote the left but the right side. Not sure if that is intentional or a bug …

It depends on the specified angle.
If you want 0.0 to denote the left, you can add/subtract 180 degrees.
I'm agree with you that an angle of zero should have 0.0 on the left though.

@hecrj hecrj added feature New feature or request rendering addition labels Sep 3, 2023
@hecrj hecrj added this to the 0.11.0 milestone Sep 3, 2023
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

We need to enable OkLab gradients only if the web-colors feature is enabled.

This can be checked at compile-time with the GAMMA_CORRECTION constant. If it's true, the new OkLab shader should be used. If it's false, the current shader should be used instead.

@hecrj
Copy link
Member

hecrj commented Sep 7, 2023

Let me give that a shot.

Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

I have split the shader code and used a bit of macro magic to conditionally reassemble it based on the color::GAMMA_CORRECTION value.

I have also simplified the example a bit!

Hopefully I didn't break anything. Give it a test, let me know, and we can merge this!

@hecrj
Copy link
Member

hecrj commented Sep 7, 2023

@bungoboingo ^

@bungoboingo
Copy link
Contributor

LGTM, tested on MacOS (m1) & windows 10, no issues.

@matze
Copy link
Contributor Author

matze commented Sep 7, 2023

Hopefully I didn't break anything. Give it a test, let me know, and we can merge this!

Also from my side, thanks for picking it up. Since you also do the phase shift by pi to have start on the left side, do you consider it … weird as well?

An angle of 0 radians will "point" to the top-center of a `Rectangle` and
then increase clockwise.
@hecrj
Copy link
Member

hecrj commented Sep 8, 2023

@matze You are right. In the web, it seems an angle of 0 "points up".

@bungoboingo I have fine-tuned/simplified the Radians::to_direction logic to reproduce this behavior in 90cbab1. Does the new logic make sense? Or have I misunderstood the purpose of the helper? I think it works, but my trigonometry is rusty (heh).

@bungoboingo
Copy link
Contributor

bungoboingo commented Sep 8, 2023

I'm fine with adjusting it to be more consistent with expectations, I went off this 👇 diagram for current behavior, although maybe OP means it should be flipped e.g. stop 0.0 with 0 angle = left? Math appears to be correct to me!

image

@hecrj
Copy link
Member

hecrj commented Sep 8, 2023

@bungoboingo Makes sense! We have to keep in mind that the Y axis is inverted in iced, so the angle increasing clock-wise seems correct.

I think the angle defining the end position of the gradient makes more sense than the start. Now it's just a matter of choosing whether a 0 angle denotes the (1, 0) position or the (0, 1). Since it seems trivial, let's just do what CSS does; for familiarity.

@hecrj hecrj merged commit 89d9c45 into iced-rs:master Sep 8, 2023
11 checks passed
@hecrj hecrj added the styling label Sep 8, 2023
@matze matze deleted the perceptual-gradient branch September 8, 2023 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants