Closed
Bug 1092634
Opened 10 years ago
Closed 9 years ago
SVG inset drop-shadow filter example does not look right
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
mozilla38
Tracking | Status | |
---|---|---|
firefox38 | --- | fixed |
People
(Reporter: heycam, Assigned: mstange)
References
(Blocks 1 open bug, )
Details
Attachments
(1 file, 3 obsolete files)
33.45 KB,
patch
|
bas.schouten
:
review+
|
Details | Diff | Splinter Review |
http://www.xanthir.com/b4Yv0 looks good in Chrome and wrong in Firefox.
Comment 1•10 years ago
|
||
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>.
Assignee | ||
Comment 2•10 years ago
|
||
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.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•10 years ago
|
||
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•10 years ago
|
||
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.
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8521026 -
Flags: review?(bas)
Updated•10 years ago
|
Attachment #8521026 -
Flags: review?(bas) → review+
Assignee | ||
Comment 6•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/41ecbb9a5fac
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
Flags: needinfo?(mstange)
Assignee | ||
Comment 8•9 years ago
|
||
This adds D2D filter support using a custom ExtendInput effect.
Attachment #8521026 -
Attachment is obsolete: true
Flags: needinfo?(mstange)
Attachment #8529916 -
Flags: review?(bas)
Comment 9•9 years ago
|
||
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?
Assignee | ||
Comment 10•9 years ago
|
||
Huh, must have forgotten to qrefresh.
Assignee | ||
Comment 11•9 years ago
|
||
Attachment #8529916 -
Attachment is obsolete: true
Attachment #8529916 -
Flags: review?(bas)
Attachment #8530435 -
Flags: review?(bas)
Assignee | ||
Comment 12•9 years ago
|
||
Looks like this completely breaks filter drawing in reftests... :-( https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=6f2f1170e82d
Assignee | ||
Comment 13•9 years ago
|
||
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
Attachment #8530435 -
Attachment is obsolete: true
Attachment #8530435 -
Flags: review?(bas)
Attachment #8533446 -
Flags: review?(bas)
Comment 14•9 years ago
|
||
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.
Attachment #8533446 -
Flags: review?(bas) → review+
Assignee | ||
Comment 15•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c0d3d80eb41d
Comment 16•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c0d3d80eb41d
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox38:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in
before you can comment on or make changes to this bug.
Description
•