Implement edgeMode="duplicate" for feGaussianBlur

NEW
Unassigned

Status

()

Core
Graphics
4 years ago
3 years ago

People

(Reporter: Max Vujovic, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
http://dev.w3.org/fxtf/filters/#element-attrdef-fegaussianblur-edgemode

edgeMode="duplicate" gives blurred elements a hard edge instead of a feathered edge. Hard edges on blurs are popular in designs today.

Dirk has implemented edgeMode="duplicate" in WebKit nightly, and Chrome is receptive to it. I'd like to see it in all browsers.

I'm working on some patches.
(Reporter)

Updated

4 years ago
Assignee: nobody → mvujovic
(Reporter)

Comment 1

4 years ago
Created attachment 8517793 [details] [diff] [review]
Part 1: Store edgeMode in SVGFEGaussianBlurElement.

This next set of patches implements edgeMode="duplicate" for feGaussianBlur in the software pipeline.

I'm thinking I'll also have to implement edgeMode="duplicate" for D2D before landing these, otherwise the feature won't work on all platforms. Thus, no rush for reviewing these :)
Attachment #8517793 - Flags: review?(mstange)
(Reporter)

Comment 2

4 years ago
Created attachment 8517797 [details] [diff] [review]
Part 2: Refactor ConvolveMatrixEdgeMode to EdgeMode for use by both feConvolveMatrix and feGaussianBlur.
Attachment #8517797 - Flags: review?(mstange)
(Reporter)

Comment 3

4 years ago
Created attachment 8517800 [details] [diff] [review]
Part 3: Store edgeMode in blur-related FilterNodeSoftware classes.
Attachment #8517800 - Flags: review?(mstange)
(Reporter)

Comment 4

4 years ago
Created attachment 8517801 [details] [diff] [review]
Part 4: Render feGaussianBlur edgeMode="duplicate" in software.
Attachment #8517801 - Flags: review?(mstange)
(Reporter)

Comment 5

4 years ago
Created attachment 8517804 [details] [diff] [review]
Part 5: Add a test for feGaussianBlur edgeMode="duplicate".

This test is highly inspired by Dirk's test for edgeMode="duplicate" in WebKit. Thanks for the donation, Dirk :)
Attachment #8517804 - Flags: review?(mstange)
(Reporter)

Updated

4 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 6

4 years ago
Created attachment 8518547 [details] [diff] [review]
Part 2 [v2]: Refactor ConvolveMatrixEdgeMode to EdgeMode for use by both feConvolveMatrix and feGaussianBlur.

Fix a bug in part 2, where I was falling back from EDGE_MODE_WRAP to EDGE_MODE_DUPLICATE for feConvolveMatrix unintentionally.
Attachment #8517797 - Attachment is obsolete: true
Attachment #8517797 - Flags: review?(mstange)
Attachment #8518547 - Flags: review?(mstange)
Comment on attachment 8517793 [details] [diff] [review]
Part 1: Store edgeMode in SVGFEGaussianBlurElement.

Review of attachment 8517793 [details] [diff] [review]:
-----------------------------------------------------------------

webidl changes also need review from a DOM peer.
Attachment #8517793 - Flags: review?(mstange) → review+
Does the spec say what should happen if the source graphic is smaller than the primitive subregion?
I remember that for the convolve matrix filter I had to make the edge mode apply to whole subregion, which means padding the source graphic with transparency and repeating those transparent pixels. This was needed to get the feConvolveMatrix-2.svg reftest to pass. See ATT_CONVOLVE_MATRIX_SOURCE_RECT, aTransparencyPaddedSourceRect and the comment in the FilterNodeConvolveD2D1 constructor for how that is handled.

Comment 9

4 years ago
(In reply to Markus Stange [:mstange] from comment #8)
> Does the spec say what should happen if the source graphic is smaller than
> the primitive subregion?

Why does that matter? Of course a SourceGraphic can be smaller than the primitive subregion. In fact, most of the time it is, otherwise you wouldn't see a blur correctly :)

> I remember that for the convolve matrix filter I had to make the edge mode
> apply to whole subregion, which means padding the source graphic with
> transparency and repeating those transparent pixels. This was needed to get
> the feConvolveMatrix-2.svg reftest to pass. See
> ATT_CONVOLVE_MATRIX_SOURCE_RECT, aTransparencyPaddedSourceRect and the
> comment in the FilterNodeConvolveD2D1 constructor for how that is handled.
If the location of the "edge" depends on the size of the source graphic, then the results will depend on how much work browser do to find tight bounds of the source graphic. Say you have a <div> with a green background and a box-shadow: 0 0 0 10px transparent, and now you apply a blur filter with edgeMode="duplicate" to that. Is the "edge" around the invisible box shadow or around the green rectangle? In Firefox we make the SourceGraphic as small as possible around what we're actually painting, but I don't know if we actually have an optimization to throw out invisible box-shadows. And in any case, that's an optimization that shouldn't be observable, so we don't want it to affect the blur edge.
What are the next steps here?
(Reporter)

Comment 12

3 years ago
(In reply to Markus Stange [:mstange] from comment #11)
> What are the next steps here?

I think the next steps are to get some clarification from the spec around transparency-padding the SourceGraphic, and then add that functionality to these patches if necessary. Then, we need the D2D implementation, which should probably share edgeMode-related code with feConvolveMatrix.

I got pulled onto another project, so I don't think I'll be able to get back to this bug anytime soon unfortunately. I'll unassign myself and remove the r?'s in case someone wants to pick up where I left off.
Assignee: mvujovic → nobody
Status: ASSIGNED → NEW
(Reporter)

Updated

3 years ago
Attachment #8517800 - Flags: review?(mstange)
(Reporter)

Updated

3 years ago
Attachment #8517801 - Flags: review?(mstange)
(Reporter)

Updated

3 years ago
Attachment #8517804 - Flags: review?(mstange)
(Reporter)

Updated

3 years ago
Attachment #8518547 - Flags: review?(mstange)
You need to log in before you can comment on or make changes to this bug.