Skip to content

wgpu: Apply color transform on gradient records #12573

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

relrelb
Copy link
Contributor

@relrelb relrelb commented Aug 5, 2023

Similar to #12549, just for the wgpu renderer. Note that some of #9821 is reverted for this purpose.

@relrelb relrelb requested a review from Dinnerbone August 5, 2023 10:19
@relrelb relrelb force-pushed the wgpu_gradient_cxform branch 3 times, most recently from 6c8b131 to d753ba3 Compare August 5, 2023 11:14
@Dinnerbone
Copy link
Contributor

Note that some of #9821 is reverted for this purpose.

😭
Is there no other way? That's such a big performance hit

@relrelb relrelb force-pushed the wgpu_gradient_cxform branch from d753ba3 to 2c257d4 Compare August 5, 2023 18:38
@relrelb
Copy link
Contributor Author

relrelb commented Aug 5, 2023

Note that some of #9821 is reverted for this purpose.

😭 Is there no other way? That's such a big performance hit

I'm afraid not, because it seems like the color transform must be taken into account while processing the gradient records. Since color transforms are available only during rendering, this cannot be performed during shape registration.

@Herschel
Copy link
Member

Herschel commented Aug 16, 2023

I think it is still possible to utilize the texture method by changing how the gradient info is stored in the texture, encoding the t value into the texture instead of the fully rendered gradient.

For example, start with a simple gradient with two colors:
red at ratio 10, green at 64. The raw colors are passed into the shader as they are in the PR.
We can encode the t values in a texture, so for example, texture(10, 0) = 0.0 and texture(64, 0) = 1.0. You can then sample the texture at the proper location to get the lerp value, and lerp between the transformed colors.

For a gradient with multiple colors, let's imagine:
red at 10, green at 64, blue at 250.

You can picture baking 0.0 -> 1.0 for each interval into the texture, along with the colors for that interval. So for example:

texture(10, 0) will be 0.0, with texture(10, 1) being solid red (color0) and texture(10, 2) being solid green (color1).
texture(157, 0) will be ~0.5 (halfway between 260 and 64), with texture(157, 1) being green (color0) and texture(157, 2) being blue.

However, this will result in some artifacting due to interpolating at the discontinuity from the end of one interval to the next, jumping directly from 1.0 to 0.0.

To fix this, instead of encoding the t value as discrete 0.0-1.0 intervals, you can picture encoding it as a big continuous interval from 0.0 - 2.0 (numColors - 1). Then, normalize that so it fits into 0.0 - 1.0. So 0.0 would be solid red, 0.5 would be solid green, and 1.0 would be solid blue.

Given the above example, texture(157, 0) would be ~0.75, which is halfway between 0.5 and 1.0 (colors 1 and 2).

The code is something like:

float sample = texture(t, 0) * (num_colors - 1);
t = fract( sample )
color0 = colors[ floor( sample ) ]
color1 = colors[ color0 + 1 ]
out = lerp( transform(color0), transform(color1), t)

As a bonus we don't have to encode the color indices separately, and no loop or branching is required to figure out the interpolation value.

My only concern was that if there is enough precision in the texture to accommodate packing an exotic 16-color gradient ratio into an 8-bit channel, but I think this would be okay because the Flash ratio itself is only an 8-bit value. Alternatively we could use a float texture or buffer directly, which might be the easiest + best.

Here's an ancient branch I had with this: 85f9694#diff-e630f4f0acc4a6d61f1f3ec1b5aa6cb5798d3054326b71e29ded3da7450f24bc
Admittedly I haven't thought this through fully or in a long while, may or may not be correct!

@relrelb relrelb force-pushed the wgpu_gradient_cxform branch 2 times, most recently from 2bd32d2 to caa9399 Compare August 26, 2023 11:24
@relrelb
Copy link
Contributor Author

relrelb commented Aug 26, 2023

Implemented @Herschel's suggestion and all tests pass on my end. From some reason they fail in CI, and I'm not sure why.

@relrelb relrelb force-pushed the wgpu_gradient_cxform branch from caa9399 to 10c952d Compare September 8, 2023 18:53
@relrelb
Copy link
Contributor Author

relrelb commented Sep 8, 2023

Applied some fixes, and slightly tuned some image tests to make them pass.

@Dinnerbone Can you please take a look?

@relrelb relrelb force-pushed the wgpu_gradient_cxform branch from 10c952d to 3d0649b Compare September 8, 2023 20:28
@danielhjacobs danielhjacobs added A-rendering Area: Rendering & Graphics render-wgpu Issues relating to the wgpu renderer T-fix Type: Bug fix (in something that's supposed to work already) labels Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rendering Area: Rendering & Graphics render-wgpu Issues relating to the wgpu renderer T-fix Type: Bug fix (in something that's supposed to work already)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants