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

Fixes #4 Shader for color mappings #5

Merged
merged 3 commits into from
Aug 19, 2023

Conversation

freydev
Copy link
Contributor

@freydev freydev commented Aug 17, 2023

Here's a shader that renders color mappings extremely quickly.
There's just one issue: I haven't yet figured out how to draw a white pixel for the P3 boundary. So, I just slightly increased the color gamma beyond the sRGB space. So maybe this okay.

@freydev freydev changed the title Fixes dokozero/okcolor#4 Shader for color mappings Fixes #4 Shader for color mappings Aug 17, 2023
ui/App.tsx Outdated
@@ -1212,7 +1256,7 @@ export const App = function() {
<div class="u-flex u-items-center u-justify-between u-px-16 u-mt-18">

<div class="c-fill-stroke-selector" ref={fillOrStrokeSelector} onClick={fillOrStrokeHandle} data-has-fill="true" data-has-stroke="true" data-active="fill" >

Copy link

Choose a reason for hiding this comment

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

Sending PR with code style changes together with feature is not cool.

It is always better to keep the current formatting and send them in another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, it was some auto-changes, fixed

@ai
Copy link

ai commented Aug 17, 2023

There is a way to draw P3 colors in WebGL context (for instance, Three.js use it)

I found docs like:
https://ccameron-chromium.github.io/webgl-examples/p3.html

Do they help you?

As I understand there are 2 modes:

  1. We have Figma in sRGB mode. In this case there is no sense to render P3 space.
  2. We have Figma in P3 mode. In this case we should start <canvas> in the same P3 mode and RGB will be values in P3, not sRGB.

@freydev freydev force-pushed the feature/uv_color_optimization branch 2 times, most recently from 7ebf61e to 87405d7 Compare August 17, 2023 11:59
@freydev freydev force-pushed the feature/uv_color_optimization branch from 87405d7 to bc35ea7 Compare August 17, 2023 12:02
@freydev
Copy link
Contributor Author

freydev commented Aug 17, 2023

Yes, I've seen that documentation. There was no issue with displaying P3 colors. I added the canvas mode switch following the profile, but the problem was in displaying the boundary between sRGB and P3. I've figured out how to calculate this boundary in the shader. Now, everything is working as planned.

@ai
Copy link

ai commented Aug 17, 2023

@dokozero we are ready to check out PR! 🎉

@dokozero
Copy link
Owner

Hello,

Thanks a lot for your help @freydev, I tested your code and indeed it's superfast! From an average of 20ms to render one hue to 0.1ms and sometimes even less 🎉 (used performance.measure()).

The render is also much nicer on a 2x screen, that's perfect.

I know that Figma also uses WebGL for low level rendering and I can see why, we can now even use a value below 1 for LOW_RES_FACTOR_OKLCH, I might just rename it to RES_FACTOR_OKLCH 😉 .

However, I set it up to 0.25 instead because with 0.2 on a 1x screen, I get a result that is too pixelated. With 0.25, the result is good on both 1x and 2x screens:

res-factor-tests

I also modified this in the fragment shader code, from 0.002 to 0.003:

if (abs(c - maxChroma) < 0.003) {
    col = vec3(1, 1, 1);
}

With 0.002, the sRGB limit is discontinued (visible with a res factor of 1), leading to pixels not totally white with a smaller factor like 0.25.

But before merging, I want to check with you that my updates are OK? Did some tests and I don't see any problem from this change.

@freydev
Copy link
Contributor Author

freydev commented Aug 18, 2023

@dokozero You're right, I noticed that the white line separator is poorly displayed on different screens due to supersampling.

Yes, this is the gap in which you need to draw a white dot, it can be changed in any direction,

if (abs(c - maxChroma) < 0.003) {
    col = vec3(1, 1, 1);
}

But sometimes there was more than one dot in this interval, it was a little annoying

Check again, I came up with a new solution, rendering the separator just with an svg element, it slightly reduced performance, but the white line became much clearer on any screen.

I also fixed the wrong gamma correction to 2.2.

@freydev freydev force-pushed the feature/uv_color_optimization branch from aa8c24b to ca4a98f Compare August 18, 2023 10:35
@dokozero
Copy link
Owner

Thanks, I checked and it's better 👍 but indeed we increased a bit the rendering time (average of 2ms) which is still low so good for me.

If it's good for you I can merge the pull request.

@ai
Copy link

ai commented Aug 18, 2023

I want to write a tweet about this amazing PR. @freydev do you have twitter/mastodon account?

@freydev
Copy link
Contributor Author

freydev commented Aug 18, 2023

@dokozero Of course, this completely solves the problem now, I will continue to think about how to return to 0.1ms because the topic with rendering optimization is very exciting.

And I will follow your plugin, if you still need help, it's an honor to take part in the promotion of a new color format on the web, thank you and martians for that.

@ai yes, @futuramafrey, but I hardly use it

@dokozero dokozero merged commit c3907dd into dokozero:develop Aug 19, 2023
@dokozero
Copy link
Owner

@freydev Merge is done and the OkColor@5.0 is live 🚀 . Many thanks again for your welcome help and this impressive PR!
Btw I didn't find your nickname on Figma (for the release changelog on the Figma page), you can give me if you want to include it.

@ai Also thanks to have made this collaboration possible 🤝 . I will wait for your tweet and do one from my account after with a link to it.

@dokozero dokozero added the enhancement New feature or request label Aug 19, 2023
@freydev
Copy link
Contributor Author

freydev commented Aug 19, 2023

@dokozero actually I haven't used community profile in Figma before, maybe now is the time, I registered same name @freydev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants