Use single pass blur if std deviation is small.

[mephisto41]

Performance comparison:

without optimize:

  "tests": [
    {
      "name": "benchmarks/small-blur-radius.yaml",
      "backend_time_ns": 38865,
      "composite_time_ns": 191967,
      "paint_time_ns": 862416,
      "draw_calls": 4
    }
  ]

with optimize

  "tests": [
    {
      "name": "benchmarks/small-blur-radius.yaml",
      "backend_time_ns": 34906,
      "composite_time_ns": 186965,
      "paint_time_ns": 685445,
      "draw_calls": 3
    }
  ]

Slightly improvement.

---

This change is 

[glennw]

Could we do a gecko try run for this?

[mephisto41]

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

Looks like we don't have any reftest in gecko with very small blur radius now.

[glennw]

@mephisto41 Thanks! Sorry I didn't get to review this today, but it looks good from a quick scan. Is there any way we can share some of the shader code via use of the #include mechanism we have in shaders? I'll take a detailed look at this tomorrow and do a quick run through of the Servo WPT tests.

[bors-servo]

☔ The latest upstream changes (presumably #2031) made this pull request unmergeable. Please resolve the merge conflicts.

[glennw]

This looks good to me. My only concern is that the change to not call device_length() for the blur radius may have an effect on the Gecko reftests. If the previous try run already tested that successfully, then r=me. Otherwise, can we do one more try run just to confirm there's no issues with that change?

[mephisto41]

Sure, but gecko doesn't have latest webrender. Therefore, this patch is hardly applied to current gecko. I'll wait for next webrender update in gecko in order to send a new try. Thanks.

[mephisto41]

Latest try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=87c62b28c2def158676b9627c7e787bb14707523

[bors-servo]

☔ The latest upstream changes (presumably #2052) made this pull request unmergeable. Please resolve the merge conflicts.

[glennw]

Looks like there is one failure in boxshadow-large-border-radius, which sounds like it could be related to this change?

[glennw]

A rebase is also needed.

[mephisto41]

boxshadow-large-border-radius might be a existing issue. We already have fuzzy for this test case (fuzzy-if(webrender,70-70,1320-1320)) and the color of result image is lighter than reference image.

So after removing device_length means we don't rounding the blur radius after multiply the device pixel ratio. So the result image is even more lighter because we have a smaller blur radius because we don't rounding the radius.

[glennw]

Is there a specific reason to change that device_length line though, if that makes one of the tests have a fuzzier result? Is that change needed for the rest of this patch?

[mephisto41]

This is because the maximum kernel size for blur 2d is 5 pixel. If we rounding the radius to int, then our minimum radius is 1, thus the kernel size is 1 * 3 * 2 + 1 = 7 and never hit this optimization.

One solution is keeping the device_length but set the maximum kernel size to 7. How do you think?

[glennw]

That might be OK for now (the rounding is a deeper issue we need to fix properly at some point).

Could you check the performance difference when a kernel size of 7 between the two methods? If they are the same or the single pass is faster, then that sounds good to me :)

[mephisto41]

Looks like the set the maximum kernel size to 7 will reduced the performance.

without blur 2d.

{
  "tests": [
    {
      "name": "benchmarks/small-blur-radius.yaml",
      "backend_time_ns": 24135,
      "composite_time_ns": 162599,
      "paint_time_ns": 886416,
      "draw_calls": 4
    }
  ]
}

with blur 2d

{
  "tests": [
    {
      "name": "benchmarks/small-blur-radius.yaml",
      "backend_time_ns": 25760,
      "composite_time_ns": 165018,
      "paint_time_ns": 1362666,
      "draw_calls": 3
    }
  ]
}

[glennw]

@mephisto41 OK - do you think it's worth continuing with this optimization for now? Should we perhaps wait until we sort out that device length rounding code and then revisit this?

[mephisto41]

Sure, I think we should wait for the device_length get fixed here. Close this for now.