1499113 - The order in which blurs and scales are applied in WR seems different to Gecko and other browsers.

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P2)

Description

[Emilio Cobos Álvarez (:emilio)]

Consider the following test-case from bug 1224207:

https://bug1224207.bmoattachments.org/attachment.cgi?id=8829369

In WR the text looks way less blurry, presumably because we scale, then blur, instead of blurring, then scaling.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

I would've expected WR to generate the same output as other browsers if I wrap it in a scaling transform, but no such luck:

<div style="transform: scale(5); transform-origin: 0 0;">
  <p style="filter: blur(1px);">hello
</div>

Comment 2

[Emilio Cobos Álvarez (:emilio)]

https://drafts.fxtf.org/filter-effects/#FilterPrimitivesOverviewIntro says:

> To provide high quality rendering, all filter primitives should operate in a device dependent coordinate space, the operating coordinate space, taking device pixel density, user space transformations and zooming into account. To provide a platform independent alignment, attribute and property values are often relative to a coordinate system described by the primitiveUnits attribute. User agents must scale these relative attributes and properties to the operating coordinate space.

I think that means that the blur of a filter needs to account for the inherited transform scale... But I'm not sure what that means when the scale is not 1d...

Comment 3

[Aria Beingessner [:Gankra]]

What does gecko do under a mixed-dimension scale?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

(In reply to Alexis Beingessner [:Gankro] from comment #3)
> What does gecko do under a mixed-dimension scale?

Gecko first applies filters then transforms, I think that matches the spec and all browsers except WR.

Comment 5

[Jeff Muizelaar [:jrmuizel]]

Kats, are you interested in looking at this?

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Sure, as long as it doesn't need to be done in a hurry. It will take me some time to find and understand the relevant code paths.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Attachment: testcase

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Attachment: testcase with mixed-dimension scale

From looking at the source I suspect the blur code just isn't taking into account any scale at all between the blur and the nearest rasterization root. And the mixed-dimension scale is relevant because we'll need to store the blur for each dimension separately to handle that scenario.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

A patch like this one solves the issue here, but I'm having a hard time writing reftests for this. Also my reftest attempts thus far seem to expose other bugs. sigh

Comment 10

[Kartikaya Gupta (email:[e-mail])]

There was just one other actual bug, for which I filed bug 1529992. And I managed to write yaml reftests for this bug, so that's all good now.

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=403ce8ec555c8b711a1befa0a0448ce48ff9005c

Comment 11

[Kartikaya Gupta (email:[e-mail])]

My png was too big.

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=ad1ab0b4bfb3eed4894c954f3589bbd777e1735e

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Finally got it right: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=6533d018a8694005fe4886efbfd7190e41c6f80d

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1499113 - Turn the blur stddev value into a tuple with horizontal and vertical components. r?kvark

This patch is just plumbing; no functional changes intended.

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1499113 - Multiply transform scales into the blur radius for blur filters. r?kvark

Without this patch any enclosing scale transform between a blurred
picture and the nearest raster root was being ignored entirely for the
purposes of blur.

Also includes a couple of reftests to exercise this code.

Depends on D20907

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/da356c59e50d
Turn the blur stddev value into a tuple with horizontal and vertical components. r=kvark
https://hg.mozilla.org/integration/autoland/rev/ffe37ad57ad7
Multiply transform scales into the blur radius for blur filters. r=kvark

Comment 16

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/da356c59e50d
https://hg.mozilla.org/mozilla-central/rev/ffe37ad57ad7