1092634 - SVG inset drop-shadow filter example does not look right

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Cameron McCormack (:heycam)]

http://www.xanthir.com/b4Yv0 looks good in Chrome and wrong in Firefox.

Comment 1

[Tab Atkins Jr.]

It looks like the filter region attributes aren't being used.  The FF rendering is the same as what you get in Chrome if you remove x/y/width/height from the <filter>.

Comment 2

[Markus Stange [:mstange]]

Oh. I missed that the component alpha primitive can produce output outside the bounds of its input if you have a <feFuncA> transfer function that turns alpha = 0 into alpha > 0.

Comment 3

[Markus Stange [:mstange]]

Er, I meant "component transfer primitive".

If you want to work around the Firefox bug you can replace the <feComponentTransfer> primitive with this:
<feFlood fill-color="black"/>
<feComposite in2="SourceAlpha" operator="out"/>
That also makes it work in Safari.

And I just noticed that the color matrix primitive has the same bug.
<feColorMatrix in="SourceAlpha" type="matrix" values="0 0 0 0 0  0 0 0 0 0  0 0 0 0 0  0 0 0 -1 1"/>

The lighting filters also have it, but that's already tracked in bug 1051185.

Comment 4

[Tab Atkins Jr.]

Ah, so you're optimizing by finding a bounding box of the non-transparent content and implicitly using that as the filter region instead.  That makes sense in this context, since the normal drop-shadow does pay attention to the filter region attributes correctly.

Comment 5

[Markus Stange [:mstange]]

Attachment: patch

Comment 6

[Markus Stange [:mstange]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/41ecbb9a5fac

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/690632848ada for reftest failures:
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4018294&repo=mozilla-inbound

Comment 8

[Markus Stange [:mstange]]

Attachment: v2

This adds D2D filter support using a custom ExtendInput effect.

Comment 9

[Bas Schouten (:bas.schouten)]

Comment on attachment 8529916 [details] [diff] [review]
v2

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

As a first quick look.

::: gfx/2d/DrawTargetD2D1.cpp
@@ +846,5 @@
>      return nullptr;
>    }
>  
>    RadialGradientEffectD2D1::Register(mFactory);
> +  ExtendInputEffectD2D1::Register(mFactory);

Can you add the unregister too?

Comment 10

[Markus Stange [:mstange]]

Huh, must have forgotten to qrefresh.

Comment 11

[Markus Stange [:mstange]]

Attachment: v3

Comment 12

[Markus Stange [:mstange]]

Looks like this completely breaks filter drawing in reftests... :-(
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=6f2f1170e82d

Comment 13

[Markus Stange [:mstange]]

Attachment: v4

I had to fix up the D2D1_VECTOR_4F -> D2D1_RECT_L conversion which was overflowing. I also moved the Register/Unregister calls to DrawTargetD2D so that they're also called when D2D1 filters are used with DrawTargetD2D.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=734b1842cddd

Comment 14

[Bas Schouten (:bas.schouten)]

Comment on attachment 8533446 [details] [diff] [review]
v4

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

This looks great.

::: gfx/2d/ExtendInputEffectD2D1.cpp
@@ +21,5 @@
> +        <Effect>
> +            <!-- System Properties -->
> +            <Property name='DisplayName' type='string' value='ExtendInputEffect'/>
> +            <Property name='Author' type='string' value='Mozilla'/>
> +            <Property name='Category' type='string' value='Pattern effects'/>

nit: This was copy-pasted from the RadialGradientEffect, you may want to rename this :).

::: gfx/2d/ExtendInputEffectD2D1.h
@@ +13,5 @@
> +#include "2D.h"
> +#include "mozilla/Attributes.h"
> +
> +// {97143DC6-CBC4-4DD4-A8BA-13342B0BA46D}
> +DEFINE_GUID(CLSID_ExtendInputEffect, 

nit: some whitespace in this file.

Comment 15

[Markus Stange [:mstange]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c0d3d80eb41d

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c0d3d80eb41d