Bug 1523495 - Adjust shadow blur target sizes to avoid down-scaling artifacts. r=gw
ClosedPublic
Actions

Authored by nical on Feb 6 2019, 5:26 PM.

Details

Reviewers

Commits

Bugzilla Bug ID

1523495

Summary

Repository

rMOZILLACENTRAL mozilla-central

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

phab-bot requested review of this revision.Feb 6 2019, 5:26 PM

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".

phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".

phab-bot removed a project: Restricted Project.

Is there only one reftest image affected by this change? That seems quite surprising to me, but maybe it's expected?

Could we add a wrench reftest + reference image that is fixed by this change? How does the try run look?

gfx/wr/webrender/src/render_task.rs
632	I think the blur shader has some clamping logic to ensure that sampling outside the bounds of the source results in sampling the right texels. Do we need to do anything to that shader with this change? i.e. Can we end up in a case after the downsampling where the blur is operating on a source with padding and if so, does that cause correctness issues?

This revision is now accepted and ready to land.Feb 6 2019, 8:47 PM

gfx/wr/webrender/src/render_task.rs
632	The clamping prevents us from reading out of the source render task which is good, I don't think that we need to change anything in the shader about that. For shadows at least, inflating the render task should not cause any issue, for blurs I'm not exactly sure about the implication on the blurred content's clipping. Worst case, if the clip on the content is smaller than the inflated task size, then we would read from a line of transparent pixels I suppose, but I would expect this to show up in reftests since we have 50% chance of inflating the source of the downscale so it's quite common. The reftest run is https://treeherder.mozilla.org/#/jobs?repo=try&revision=5c68311dbd7946321d1286711bb5d47a6a309fd3 There's the wrench failure which is the reference image change I added to this patch series, and the R6 failure which I think is due to how this patch always ceils the size instead of rounding. This results in a slightly bigger blur if there are a lot of down-scale passes that were ceiled but I think that the difference is small enough to fuzz.