1980565 - Add fuzzy to web-platform test during enabling WebRender dithering

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, task)

Description

[Sotaro Ikeda [:sotaro]]

From the following reasons, it seems better to disable dithering in web-platform tests.

• A lot of test failures by enabling dithering in WebRender. It could be addressed by adding fuzzy. But the tests may be updated at unexpected timing.
• Some linear gradient tests already failed without dithering. If fuzzy is added, it is impossible to distinguish whether the fuzziness is caused by dithering or other factors.

Comment 1

[Sotaro Ikeda [:sotaro]]

Attachment: Bug 1980565 - Disable dithering in web-platform tests

Comment 2

[Sotaro Ikeda [:sotaro]]

Confirmed that the current D259517 disabled dithering in web-platform test.
https://treeherder.mozilla.org/jobs?repo=try&revision=fab1c16f54935ecb566dc6159281410df7fa1a71

Comment 3

[Sotaro Ikeda [:sotaro]]

:tnikkel advised to use __dir__.ini In this case, all tests in the directory are affected by adding fuzzy. I am going to try it.

Comment 4

[Timothy Nikkel (:tnikkel)]

I'm not sure if that will work, I've only seen it used to apply prefs to a whole directory.

Comment 5

[Sotaro Ikeda [:sotaro]]

I checked the test failures. More than half of test failures seemed to happen by comparing linear gradient with dithering and simple green color.
https://docs.google.com/spreadsheets/d/1KybHUCARWqRum6XyofejRiVFsG8564BNMTgXJkgZlqE/edit?gid=0#gid=0

Comment 6

[Jeff Muizelaar [:jrmuizel]]

(In reply to Sotaro Ikeda [:sotaro] from comment #5)

I checked the test failures. More than half of test failures seemed to happen by comparing linear gradient with dithering and simple green color.
https://docs.google.com/spreadsheets/d/1KybHUCARWqRum6XyofejRiVFsG8564BNMTgXJkgZlqE/edit?gid=0#gid=0

In https://searchfox.org/mozilla-central/source/testing/web-platform/tests/css/css-break/background-image-003.html for example, shouldn't the linear gradient with dithering be solid green?

Is the dithering producing noise in this case? I wouldn't expect it to. Do you understand why it is?

Comment 7

[Sotaro Ikeda [:sotaro]]

I am working for it in Bug 1981304.

Comment 8

[Sotaro Ikeda [:sotaro]]

Bug 1981304 seemed to reduce test failures.
https://treeherder.mozilla.org/jobs?repo=try&revision=bd454f0d12742e95578853daa0f4cc33756e351d

Comment 9

[Jeff Muizelaar [:jrmuizel]]

(In reply to Sotaro Ikeda [:sotaro] from comment #8)

Bug 1981304 seemed to reduce test failures.
https://treeherder.mozilla.org/jobs?repo=try&revision=bd454f0d12742e95578853daa0f4cc33756e351d

Even without that shouldn't there be no noise in the gradient unless we're in an area of transition between colors?

Comment 10

[Sotaro Ikeda [:sotaro]]

With current Webrender dithering implementation, when dithering is enabled, brush_linear_gradient and cs_linear_gradient always does dithering.

• https://searchfox.org/mozilla-central/rev/00d2cc8ebe323e0cde5619004c588d5e08ad1f46/gfx/wr/webrender/src/renderer/shade.rs#1024
• https://searchfox.org/mozilla-central/rev/00d2cc8ebe323e0cde5619004c588d5e08ad1f46/gfx/wr/webrender/src/renderer/shade.rs#1024
• https://searchfox.org/mozilla-central/rev/00d2cc8ebe323e0cde5619004c588d5e08ad1f46/gfx/wr/webrender/res/gradient.glsl#15

For now, dithering is disabled only when fast path of linear gradient is used.
Then the disabling dithering is depends on the following code. It disables dithering when linear gradient has 2 stops and same color.

• https://searchfox.org/mozilla-central/rev/00d2cc8ebe323e0cde5619004c588d5e08ad1f46/gfx/wr/webrender/src/prim_store/gradient/linear.rs#400

The triggering it is affected by optimize_linear_gradient().

optimize_linear_gradient() and LinearGradientTemplate::from() fail to detect/handle all cases where there are no color transitions.

Comment 11

[Sotaro Ikeda [:sotaro]]

deleted.

Comment 12

[Sotaro Ikeda [:sotaro]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #9)

(In reply to Sotaro Ikeda [:sotaro] from comment #8)

Bug 1981304 seemed to reduce test failures.
https://treeherder.mozilla.org/jobs?repo=try&revision=bd454f0d12742e95578853daa0f4cc33756e351d

Even without that shouldn't there be no noise in the gradient unless we're in an area of transition between colors?

Yea, unless it is a transition area between colors, noise should not exist.

Comment 13

[Sotaro Ikeda [:sotaro]]

:gw, what is a good way to ensure it? With Bug 1981304 fix, majority of use cases seem to be handled.

Comment 14

[Glenn Watson [:gw] | PTO until 6th Jan]

This might be something Nical has a better understanding of than me.

Comment 15

[Nicolas Silva [:nical]]

The problem comes from the way our dithering implementation simply adds noise after evaluating the gradient. That has a couple of drawbacks like the noise on constant colors here and also the fact that on average it makes the the gradient look a tiny bit more grey.

Another approach

Another way to do dithering is to jitter the interpolation factor between the colors instead of jittering the output color. That way if we interpolate between red and blue, it won't introduce small amounts of green, and if we interpolate between the same color, it will basically be the same color.

Here is an example of the approach: https://www.shadertoy.com/view/sd2yDd (it computes the gradient 3 different ways, the interesting one is grad2_Dithered_sRGB).

So that's one way. I like it because it has the nice properties I just mentioned, but it's a bit more complicated to get right because small variations in the interpolation factor t can cause large variations in the output color depending on how close the gradient stop offsets are, unlike our current approach for which the applied jitter has a constant average "strength" regardless of the contents of the gradient (I suppose it's both the cause of the problem and a simplifying property).
So if we were to jitter the interpolation factor instead of the output color, I would suggest trying to scale the jitter strength based on the difference between the stop offsets (you need to jitter less if the stops are close). And possibly also dampen the strength of the jitter based on the distance between the endpoints of the gradient.

With the current approach

We can also in the fragment shader measure the difference between the two colors and simply not apply dithering if it is zero.

Comment 16

[Nicolas Silva [:nical]]

Jrmuizel pointed out on matrix that we can also jitter the final output as we currently do but making sure that the the jitter amount is small enough for each channel that constant colors always round to the same value.

I also forgot to mention that from a look at the generated code, skia appears to jitter the final color after evaluating the gradient (similar at least in broad strokes to what we have now).

Comment 17

[Jeff Muizelaar [:jrmuizel]]

Skia's code:

  // Dithering happens here:
  float _33_value = texture(
    uTextureSampler_0_S1,
    mat3x2(umatrix_S1_c1) * vec3(sk_FragCoord.xy, 1.0),
    -0.475
  ).x - 0.5;

  vec4 output_S1 = vec4(clamp(_32_color.xyz + _33_value * urange_S1, 0.0, _32_color.w), _32_color.w);
  {
    sk_FragColor = output_S1;
  }

Comment 18

[Nicolas Silva [:nical]]

For reference, here is the generated code for a radial gradient shader with two stops in skia. I manually edited the code to make it more readable:

#version 420

uniform vec2 u_skRTFlip;
out vec4 sk_FragColor;
uniform vec4 ustart_S1_c0_c0_c0;
uniform vec4 uend_S1_c0_c0_c0;
uniform mat3 umatrix_S1_c0_c0_c1;
uniform vec4 uleftBorderColor_S1_c0_c0;
uniform vec4 urightBorderColor_S1_c0_c0;
uniform mat3 umatrix_S1_c1;
uniform float urange_S1;
uniform sampler2D uTextureSampler_0_S1;
flat in vec4 vcolor_S0;
noperspective in vec2 vTransformedCoords_6_S0;

void main() {
  vec4 sk_FragCoord = vec4(gl_FragCoord.x, u_skRTFlip.x + u_skRTFlip.y * gl_FragCoord.y, gl_FragCoord.z, gl_FragCoord.w);
  vec4 inputColor = vcolinputColoror_S0; // This is then unused/overwritten.
  vec4 t = vec4(length(vTransformedCoords_6_S0), 1.0, 0.0, 0.0);
  vec4 gradientColor;
  if (t.x < 0.0) {
    gradientColor = uleftBorderColor_S1_c0_c0;
  } else if (t.x > 1.0) {
    gradientColor = urightBorderColor_S1_c0_c0;
  } else {
    gradientColor = mix(ustart_S1_c0_c0_c0, uend_S1_c0_c0_c0, t.x);
  }

  // Dithering happens here:
  float ditherNoise = texture(
    uTextureSampler_0_S1,
    mat3x2(umatrix_S1_c1) * vec3(sk_FragCoord.xy, 1.0),
    -0.475
  ).x - 0.5;

  sk_FragColor = vec4(
    clamp(
      gradientColor.xyz + ditherNoise * urange_S1,
      0.0,
      gradientColor.w
    ),
    gradientColor.w
  );
}

// Uniforms:
// ---------
// umatrix_S1_c1: float3x3 (column_major):
//  {0.125, 0.00, 0.00}
//  {0.00, 0.125, 0.00}
//  {0.00, 0.00, 1.00}    
// u_skRTFlip: float2                 960.00, -1.00
// uend_S1_c0_c0_c0: float4           0.00, 0.00, 0.00, 1.00
// urange_S1: float                   0.00392
// urightBorderColor_S1_c0_c0: float4 0.00, 0.00, 0.00, 1.00
// ustart_S1_c0_c0_c0: float4         1.00, 1.00, 1.00, 1.00

As noted by Jeff on matrix, urange_S1 = 1/255 and by the look of it the ditherNoise variable is in the [-0.5, 0.5] range so it looks like they are doing exactly what Jeff suggested in comment 16.

Comment 19

[Nicolas Silva [:nical]]

So to sum up the conversation, I think that someone should check in renderdoc whether our dithering noise is too strong (if it goes outside of the [-0.5/255, 0.5/255] range. We should make sure that it is in the that range and hopefully it should solve the noisy constant color issue.

I'm talking about the noise amount in our implementation:

vec4 dither(vec4 color) {
    const int matrix_mask = 7;

    ivec2 pos = ivec2(gl_FragCoord.xy) & ivec2(matrix_mask);
    float noise_normalized = (texelFetch(sDither, pos, 0).r * 255.0 + 0.5) / 64.0;
    float noise = (noise_normalized - 0.5) / 256.0; // scale down to the unit length

    return color + vec4(noise, noise, noise, 0);
}

By the way the this looks quite a bit more convoluted than it needs to be. Skia's version is pretty straightfoward in comparison, it boils down to (texelFetch(noise_uv, ).r - 0.5) / 255.0.

Comment 20

[Sotaro Ikeda [:sotaro]]

Thank you very much for the advice!

Comment 21

[Sotaro Ikeda [:sotaro]]

Attachment: WIP: Bug 1980565 - Disable fast path of linear gradient and enable dithering

Comment 22

[Sotaro Ikeda [:sotaro]]

D261345 is created to test linear gradient dithering. Since when fast path is used, dithering is not used.

Comment 23

[Sotaro Ikeda [:sotaro]]

Attachment: simple_green_linear_gradient.html

By applying D261345, with simple green linear gradient, there are occasional (0, 127, 0) instead of (0, 128, 0) by using color picker with Win11 NVIDIA GPU pc.

With chrome browser, during showing simple green linear gradient, occasional (2, 127, 0) existed occasionally by using color picker with the pc.

Comment 24

[Sotaro Ikeda [:sotaro]]

Attachment: simple_red_linear_gradient.html

Comment 25

[Sotaro Ikeda [:sotaro]]

By applying D261345, with simple red linear gradient, I always saw (255, 0, 0) with m-c Firefox and chrome browser with Win11 NVIDIA GPU pc.

Comment 26

[Sotaro Ikeda [:sotaro]]

When dither(vec4 color) returned vec4(0, 0.5, 0, 1) with Attachment 9507494, the linear gradient green value was 127. I thought that it should become 128.

Comment 27

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #26)

When dither(vec4 color) returned vec4(0, 0.5, 0, 1) with Attachment 9507494, the linear gradient green value was 127. I thought that it should become 128.

I got 128 on another Win11 PC. Then it might be a GPU specific problem.

Comment 28

[Sotaro Ikeda [:sotaro]]

:nical asked to try something like the following.

---

vec4 dither(vec4 color) {
// We want the output noise to be in the -0.5..0.5 range with a slight
// margin to prevent precision/rounding errors from making the noise
// change the look of constant sections of the gradient.
const int matrix_mask = 7;
ivec2 pos = ivec2(gl_FragCoord.xy) & ivec2(matrix_mask);
// We are fetching values in the 0..(63/255) range.
float noise_normalized = texelFetch(sDither, pos, 0).r * 255.0 / 63.0;
// Center the range around zero and scale it down by a tiny amount to
// avoid hardware-specific rounding differences when the fractional
// part of the resulting color is at exactly 0.5.
float noise = (noise_normalized - 0.5) * 0.995;

return color + vec4(noise, noise, noise, 0);

}

Comment 29

[Sotaro Ikeda [:sotaro]]

:nical is working for long term solution in Bug 1978773.

Comment 30

[Sotaro Ikeda [:sotaro]]

Attachment: Bug 1980565 - Add fuzzy to web-platform test for enabling WebRender dithering

Comment 31

[Sotaro Ikeda [:sotaro]]

By Bug 1984016 fix, amount of fuzzy has been reduced.

Comment 32

[Sotaro Ikeda [:sotaro]]

D262038 addressed web-platform test failure during enabling WebRender dithering.

https://treeherder.mozilla.org/jobs?repo=try&revision=a893247efde8d1a66769fe11c8b46b4e473f3fa0

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c8d69d305c96
https://hg.mozilla.org/integration/autoland/rev/35117e199b23
Add fuzzy to web-platform test for enabling WebRender dithering r=gfx-reviewers,ahale

Comment 34

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/35117e199b23