Use the texture filtering hardware to reduce the number of sample operations in the blur shader. #3122

pcwalton · 2018-09-25T22:01:26Z

Linear filtering allows us to evaluate the formula

k₀c₀ + k₁c₁                 (Equation 1)

where c₀ and c₁ are the colors of adjacent texels and k₀ and k₁ are arbitrary
factors (in this case, the results of evaluating the Gaussian function) with a
single lookup. Linear filtering evaluates the following expression for some t
between 0 and 1:

lerp(c₀, c₁, t)

It can be shown algebraically that Equation 1 is equivalent to:

             ⎛           k₁  ⎞
(k₀ + k₁)lerp⎜c₀, c₁, ───────⎟
             ⎝        k₀ + k₁⎠

Which can be readily evaluated by letting t = k₁/(k₀ + k₁) and performing a
texture lookup.

This results in some minor visual differences because we always compute an even
number of pixels on each side (i.e. the support is now always rounded up to
the nearest even number of pixels), whereas before we might have evaluated an
odd number of pixels. All differences should therefore be minor quality
improvements.

Don't merge this yet; I want to run tests and a try build.

This change is

pcwalton · 2018-09-25T22:12:49Z

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6920b63af9336557d1937429b59706ee4f6cf5d1

pcwalton · 2018-09-26T02:25:09Z

Yeah, this looks bad. I'll try to match the existing rendering more closely.

mstange · 2018-09-26T02:38:40Z

If then new blur is just as "correct" as the software reference blur, it's probably not worth making them match. It would be better to spend the time on making sure that WebRender is used for both the test and the reference (by making SVG filters go through WebRender.)

I'm pretty confident this is correct--it's either just as accurate or more accurate than the current implementation--so it'll let me sort out which test failures, if any, from servo#3028 and servo#3122 are true regressions.

Round blur support up to the nearest even number of pixels. I'm pretty confident this is correct--it's either just as accurate or more accurate than the current implementation--so it'll let me sort out which test failures, if any, from #3028 and #3122 are true regressions.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/3126)

bors-servo · 2018-09-27T00:13:02Z

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

pcwalton · 2018-09-27T02:47:03Z

Much better now. Just one test has imperceptible differences, probably due to minor rounding differences introduced by algebraic manipulation.

pcwalton · 2018-09-27T03:28:10Z

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6f72ba3298e9ca75a6cd4bc86e5be4946fcda3e6

pcwalton · 2018-09-27T13:20:08Z

Just a few minor fuzziness differences on try. This looks good.

r? @gw3583

kvark

Reviewed 2 of 2 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @gw3583)

kvark · 2018-09-28T17:00:43Z

Windows reftests are failing though.

gw3583 · 2018-10-29T22:59:00Z

@pcwalton It looks like this was basically ready to go but then got overlooked. If that's the case, I guess rebasing and doing a try would be good to make sure it's still fine, and then we can get this merged?

pcwalton · 2018-10-30T02:29:54Z

It was ready to go but I wasn't sure about try results. Let me try again.

gw3583 · 2018-11-20T01:13:08Z

@pcwalton Did you get a chance to kick off another try for this?

pcwalton · 2018-12-04T20:38:34Z

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

bors-servo · 2018-12-05T04:18:57Z

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

gw3583 · 2018-12-05T08:41:18Z

From a quick look at the try run:

1 and 2 failures look like unrelated intermittents.
R1 and R3 on OSX are new passes (less fuzzy than the current annotations).
R7 on Linux is a new fail - the max pixel difference is the same as the current reftest annotations, but there are more pixels that need to be marked fuzzy.

I think these changes are fine, but we should get @staktrace or @jrmuizel to double check. Also looks like it needs a rebase to fix the conflicting reftest image.

staktrace · 2018-12-05T12:32:40Z

From a quick look at the try run:

1 and 2 failures look like unrelated intermittents.

Yes although I don't see these failures anywhere else so I'd like to see another try push rebased on central to confirm. And this try push was actually based on mozilla-beta rather than mozilla-central, so it may not even be accurate in terms of what it shows.

R1 and R3 on OSX are new passes (less fuzzy than the current annotations).
R7 on Linux is a new fail - the max pixel difference is the same as the current reftest annotations, but there are more pixels that need to be marked fuzzy.

Agreed, with the caveat that these results are from mozilla-beta. If we get the same results on mozilla-central that's fine.

gw3583 · 2018-12-05T16:01:40Z

@staktrace Thanks for double checking those.

@pcwalton Looks like we need a try run against m-c, and to fix the rebase conflict above, then this should hopefully be good to go.

pcwalton · 2018-12-05T22:25:06Z

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

gw3583 · 2018-12-06T13:35:02Z

Try looks good (although will get @staktrace to double check again). Just need to resolve that conflicting png above then this is ready, I think.

staktrace · 2018-12-06T14:03:31Z

Yup, try push looks good to me (I can adjust the reftest numbers on those two tests when I merge it to m-c).

pcwalton · 2018-12-06T18:36:17Z

Patch updated.

gw3583 · 2018-12-06T18:53:07Z

@bors-servo r+

bors-servo · 2018-12-06T18:53:08Z

📌 Commit 3b42369 has been approved by gw3583

bors-servo · 2018-12-06T19:54:53Z

⌛ Testing commit 3b42369 with merge 013e808...

Use the texture filtering hardware to reduce the number of sample operations in the blur shader. Linear filtering allows us to evaluate the formula k₀c₀ + k₁c₁ (Equation 1) where c₀ and c₁ are the colors of adjacent texels and k₀ and k₁ are arbitrary factors (in this case, the results of evaluating the Gaussian function) with a single lookup. Linear filtering evaluates the following expression for some t between 0 and 1: lerp(c₀, c₁, t) It can be shown algebraically that Equation 1 is equivalent to: ⎛ k₁ ⎞ (k₀ + k₁)lerp⎜c₀, c₁, ───────⎟ ⎝ k₀ + k₁⎠ Which can be readily evaluated by letting `t = k₁/(k₀ + k₁)` and performing a texture lookup. This results in some minor visual differences because we always compute an even number of pixels on each side (i.e. the *support* is now always rounded up to the nearest even number of pixels), whereas before we might have evaluated an odd number of pixels. All differences should therefore be minor quality improvements. Don't merge this yet; I want to run tests and a try build.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/3122)

bors-servo · 2018-12-06T20:40:00Z

💔 Test failed - status-taskcluster

gw3583 · 2018-12-06T20:45:40Z

One of the reftest fails on Linux CI and will need fuzziness of ..... 1 pixel .... max difference 1 🤦‍♂️

REFTEST TEST-UNEXPECTED-FAIL | reftests/text/two-shadows.yaml == reftests/text/two-shadows.png | image comparison, max difference: 1, number of differing pixels: 1

…rations in the blur shader. Linear filtering allows us to evaluate the formula k₀c₀ + k₁c₁ (Formula 1) where c₀ and c₁ are the colors of adjacent texels and k₀ and k₁ are arbitrary factors (in this case, the results of evaluating the Gaussian function) with a single lookup. Linear filtering evaluates the following expression for some t between 0 and 1: lerp(c₀, c₁, t) It can be shown algebraically that Formula 1 is equivalent to: ⎛ k₁ ⎞ (k₀ + k₁)lerp⎜c₀, c₁, ───────⎟ ⎝ k₀ + k₁⎠ Which can be readily evaluated by letting `t = k₁/(k₀ + k₁)` and performing a texture lookup.

pcwalton · 2018-12-06T21:29:20Z

Updated.

staktrace · 2018-12-06T22:08:32Z

@bors-servo r=gw3583

bors-servo · 2018-12-06T22:08:32Z

📌 Commit 38ec7db has been approved by gw3583

Use the texture filtering hardware to reduce the number of sample operations in the blur shader. Linear filtering allows us to evaluate the formula k₀c₀ + k₁c₁ (Equation 1) where c₀ and c₁ are the colors of adjacent texels and k₀ and k₁ are arbitrary factors (in this case, the results of evaluating the Gaussian function) with a single lookup. Linear filtering evaluates the following expression for some t between 0 and 1: lerp(c₀, c₁, t) It can be shown algebraically that Equation 1 is equivalent to: ⎛ k₁ ⎞ (k₀ + k₁)lerp⎜c₀, c₁, ───────⎟ ⎝ k₀ + k₁⎠ Which can be readily evaluated by letting `t = k₁/(k₀ + k₁)` and performing a texture lookup. This results in some minor visual differences because we always compute an even number of pixels on each side (i.e. the *support* is now always rounded up to the nearest even number of pixels), whereas before we might have evaluated an odd number of pixels. All differences should therefore be minor quality improvements. Don't merge this yet; I want to run tests and a try build.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/3122)

bors-servo · 2018-12-06T22:08:36Z

⌛ Testing commit 38ec7db with merge 19abc69...

bors-servo · 2018-12-06T23:02:22Z

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: gw3583
Pushing 19abc69 to master...

…2b5ac5aeefe9 (WR PR #3122). r=kats servo/webrender#3122 Differential Revision: https://phabricator.services.mozilla.com/D13975 --HG-- extra : moz-landing-system : lando

…2b5ac5aeefe9 (WR PR #3122). r=kats servo/webrender#3122 Differential Revision: https://phabricator.services.mozilla.com/D13975

…2b5ac5aeefe9 (WR PR #3122). r=kats servo/webrender#3122 Differential Revision: https://phabricator.services.mozilla.com/D13975 UltraBlame original commit: 2a526099b1f4405dd3df6852d38d0666580f4542

pcwalton mentioned this pull request Sep 26, 2018

Round blur support up to the nearest even number of pixels. #3126

Merged

pcwalton force-pushed the blur-filtering branch 2 times, most recently from 4759586 to 58814ff Compare September 27, 2018 02:46

pcwalton requested a review from gw3583 September 27, 2018 13:20

kvark approved these changes Sep 28, 2018

View reviewed changes

pcwalton force-pushed the blur-filtering branch from 58814ff to 6f52612 Compare October 5, 2018 18:19

pcwalton mentioned this pull request Oct 5, 2018

Rewrite the blur shader to remove the variable-length loop and to use the texture filtering hardware more effectively. #3028

Closed

pcwalton force-pushed the blur-filtering branch from 6f52612 to 5283e7d Compare October 30, 2018 02:39

pcwalton force-pushed the blur-filtering branch from 5283e7d to 02477fd Compare December 4, 2018 20:19

pcwalton force-pushed the blur-filtering branch from 02477fd to 3b42369 Compare December 6, 2018 18:36

pcwalton force-pushed the blur-filtering branch from 3b42369 to 38ec7db Compare December 6, 2018 21:29

bors-servo merged commit 38ec7db into servo:master Dec 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use the texture filtering hardware to reduce the number of sample operations in the blur shader. #3122

Use the texture filtering hardware to reduce the number of sample operations in the blur shader. #3122

pcwalton commented Sep 25, 2018 •

edited by larsbergstrom

pcwalton commented Sep 25, 2018

pcwalton commented Sep 26, 2018

mstange commented Sep 26, 2018

bors-servo commented Sep 27, 2018

pcwalton commented Sep 27, 2018 •

edited

pcwalton commented Sep 27, 2018

pcwalton commented Sep 27, 2018

kvark left a comment

kvark commented Sep 28, 2018

gw3583 commented Oct 29, 2018

pcwalton commented Oct 30, 2018

gw3583 commented Nov 20, 2018

pcwalton commented Dec 4, 2018

bors-servo commented Dec 5, 2018

gw3583 commented Dec 5, 2018

staktrace commented Dec 5, 2018

gw3583 commented Dec 5, 2018

pcwalton commented Dec 5, 2018

gw3583 commented Dec 6, 2018

staktrace commented Dec 6, 2018

pcwalton commented Dec 6, 2018

gw3583 commented Dec 6, 2018

bors-servo commented Dec 6, 2018

bors-servo commented Dec 6, 2018

bors-servo commented Dec 6, 2018

gw3583 commented Dec 6, 2018

pcwalton commented Dec 6, 2018

staktrace commented Dec 6, 2018

bors-servo commented Dec 6, 2018

bors-servo commented Dec 6, 2018

bors-servo commented Dec 6, 2018

Use the texture filtering hardware to reduce the number of sample operations in the blur shader. #3122

Use the texture filtering hardware to reduce the number of sample operations in the blur shader. #3122

Conversation

pcwalton commented Sep 25, 2018 • edited by larsbergstrom

pcwalton commented Sep 25, 2018

pcwalton commented Sep 26, 2018

mstange commented Sep 26, 2018

bors-servo commented Sep 27, 2018

pcwalton commented Sep 27, 2018 • edited

pcwalton commented Sep 27, 2018

pcwalton commented Sep 27, 2018

kvark left a comment

Choose a reason for hiding this comment

kvark commented Sep 28, 2018

gw3583 commented Oct 29, 2018

pcwalton commented Oct 30, 2018

gw3583 commented Nov 20, 2018

pcwalton commented Dec 4, 2018

bors-servo commented Dec 5, 2018

gw3583 commented Dec 5, 2018

staktrace commented Dec 5, 2018

gw3583 commented Dec 5, 2018

pcwalton commented Dec 5, 2018

gw3583 commented Dec 6, 2018

staktrace commented Dec 6, 2018

pcwalton commented Dec 6, 2018

gw3583 commented Dec 6, 2018

bors-servo commented Dec 6, 2018

bors-servo commented Dec 6, 2018

bors-servo commented Dec 6, 2018

gw3583 commented Dec 6, 2018

pcwalton commented Dec 6, 2018

staktrace commented Dec 6, 2018

bors-servo commented Dec 6, 2018

bors-servo commented Dec 6, 2018

bors-servo commented Dec 6, 2018

pcwalton commented Sep 25, 2018 •

edited by larsbergstrom

pcwalton commented Sep 27, 2018 •

edited