Closed
Bug 964200
Opened 10 years ago
Closed 9 years ago
Implement Filter Effects Module feDropShadow filter
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: longsonr, Assigned: longsonr)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-needed)
Attachments
(4 files, 2 obsolete files)
13.14 KB,
patch
|
hsivonen
:
review-
|
Details | Diff | Splinter Review |
1.17 KB,
patch
|
hsivonen
:
review+
longsonr
:
checkin+
|
Details | Diff | Splinter Review |
35.68 KB,
patch
|
longsonr
:
checkin+
|
Details | Diff | Splinter Review |
24.92 KB,
patch
|
wchen
:
review+
longsonr
:
checkin+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•10 years ago
|
||
See https://dvcs.w3.org/hg/FXTF/raw-file/tip/filters/index.html#elementdef-fecustom Chromium implements this filter.
Attachment #8365851 -
Flags: review?(hsivonen)
Assignee | ||
Comment 2•10 years ago
|
||
Assignee: nobody → longsonr
Attachment #8365856 -
Flags: review?(hsivonen)
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8365859 -
Flags: review?(mstange)
Comment 4•10 years ago
|
||
Comment on attachment 8365859 [details] [diff] [review] shadow3.txt Very nice! >diff --git a/content/svg/content/src/SVGFEGaussianBlurElement.cpp b/content/svg/content/src/SVGFEDropShadowElement.cpp >copy from content/svg/content/src/SVGFEGaussianBlurElement.cpp >copy to content/svg/content/src/SVGFEDropShadowElement.cpp >--- a/content/svg/content/src/SVGFEGaussianBlurElement.cpp >+++ b/content/svg/content/src/SVGFEDropShadowElement.cpp >@@ -1,125 +1,179 @@ > /* a*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ > /* This Source Code Form is subject to the terms of the Mozilla Public > * License, v. 2.0. If a copy of the MPL was not distributed with this > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > >-#include "mozilla/dom/SVGFEGaussianBlurElement.h" >-#include "mozilla/dom/SVGFEGaussianBlurElementBinding.h" >+#include "mozilla/dom/SVGFEDropShadowElement.h" >+#include "mozilla/dom/SVGFEDropShadowElementBinding.h" > #include "nsSVGFilterInstance.h" >-#include "nsSVGUtils.h" > >-NS_IMPL_NS_NEW_NAMESPACED_SVG_ELEMENT(FEGaussianBlur) >- >-using namespace mozilla::gfx; I don't think you can remove this line. If it compiles, then probably because the file is getting unified with other files that have this declaration. (You can confirm whether this is the case by moving this file from the UNIFIED_SOURCES list to the SOURCES list in moz.build.) >diff --git a/gfx/src/FilterSupport.cpp b/gfx/src/FilterSupport.cpp >+ if (aDescription.InputColorSpace(0) == LINEAR_RGB) { >+ flood = FilterWrappers::SRGBToLinearRGB(aDT, flood); > } Do we need to do something similar for the lighting filters? Also, it would be much better for performance if this transformation were only applied to the single Color value that we set on the filter, and not on the whole filter output. Can you add helper methods for that?
Attachment #8365859 -
Flags: review?(mstange) → review+
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8365915 -
Flags: review?(weinjared+bmo)
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #4) > > Do we need to do something similar for the lighting filters? Lighting filters were originally fixed in bug 828526 although there's no test for that. Hopefully that carried over into the Moz2D implementation. It's only because we need a colour in the middle of filter processing that we run into an issue. Normally the colour space would be adjusted pre/post filter processing.
Assignee | ||
Comment 7•10 years ago
|
||
This compiles even if UNIFIED_SOURCES is changed to SOURCES in moz.build
Attachment #8365859 -
Attachment is obsolete: true
Comment on attachment 8365851 [details] [diff] [review] shadow1.txt >diff --git a/content/base/src/nsGkAtomList.h b/content/base/src/nsGkAtomList.h >diff --git a/content/base/src/nsTreeSanitizer.cpp b/content/base/src/nsTreeSanitizer.cpp r+ on these. >diff --git a/parser/html/javasrc/ElementName.java b/parser/html/javasrc/ElementName.java >diff --git a/parser/html/nsHtml5ElementName.cpp b/parser/html/nsHtml5ElementName.cpp r- on these for the failure to update the precomputed hashes and to put the new entry in the place where the self-generator for ElementName.java would put it. To save you the trouble of dealing with two layers of code generation, I've attached the right parser changes as attachment 8366538 [details] [diff] [review]. Also, changing the parser like this should be accompanied with a corresponding spec change. I've filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=24424 and sent http://lists.w3.org/Archives/Public/public-fx/2014JanMar/0046.html .
Attachment #8365851 -
Flags: review?(hsivonen) → review-
Attachment #8365856 -
Flags: review?(hsivonen) → review+
Attachment #8366538 -
Flags: review?(wchen)
Updated•10 years ago
|
Attachment #8366538 -
Flags: review?(wchen) → review+
Assignee | ||
Comment 10•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=d13bb1334f86
Updated•10 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Updated•10 years ago
|
Whiteboard: [leave open]
Assignee | ||
Comment 11•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/87e3729c065a
Assignee | ||
Comment 12•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/af184f7760b9
Assignee | ||
Comment 13•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/19993e76f4a0 https://hg.mozilla.org/integration/mozilla-inbound/rev/7dbe53176a38
Comment 14•10 years ago
|
||
Backed out both for build bustage - since they were on build bustage, hard to see on their landings, should be easier to see what's yours in https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=e1fb8eeca62e where your parent bustage was backed out.
Assignee | ||
Comment 15•10 years ago
|
||
It seemed to be OK on try (comment 10). I wonder if it needs a clobber?
Comment 16•10 years ago
|
||
Also, https://tbpl.mozilla.org/php/getParsedLog.php?id=34147081&tree=Mozilla-Inbound
Comment 17•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/87e3729c065a https://hg.mozilla.org/mozilla-central/rev/af184f7760b9
Assignee | ||
Updated•10 years ago
|
Attachment #8366538 -
Flags: checkin+
Assignee | ||
Comment 18•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=2d0ccb3fd5d0 https://hg.mozilla.org/integration/mozilla-inbound/rev/01617899e713
Assignee | ||
Comment 19•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/85b2df0dfa13
Assignee | ||
Comment 20•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2d3d6b83f46b
Assignee | ||
Comment 21•9 years ago
|
||
This time with a clobber, should stick I hope. https://hg.mozilla.org/integration/mozilla-inbound/rev/b7e7180113da https://hg.mozilla.org/integration/mozilla-inbound/rev/542ea91fc525
Comment 22•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b7e7180113da https://hg.mozilla.org/mozilla-central/rev/542ea91fc525
Assignee | ||
Updated•9 years ago
|
Attachment #8365856 -
Flags: checkin+
Assignee | ||
Updated•9 years ago
|
Attachment #8365998 -
Flags: checkin+
Assignee | ||
Comment 23•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=4f6163423a45 https://tbpl.mozilla.org/?tree=Try&rev=9cc6844e381c
Assignee | ||
Comment 24•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bbf7117f577
Flags: in-testsuite+
Whiteboard: [leave open]
Comment 25•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5bbf7117f577
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Comment 26•9 years ago
|
||
Comment on attachment 8365915 [details] [diff] [review] use the new feature Robert, this review was attached to an account that I don't log in to often and such I didn't see this until now. If this still needs review, please re-request review but from :jaws. Thanks.
Flags: needinfo?(longsonr)
Attachment #8365915 -
Flags: review?(weinjared+bmo)
Assignee | ||
Updated•9 years ago
|
Attachment #8365915 -
Attachment is obsolete: true
Updated•5 years ago
|
Blocks: css-filter-effects-2
You need to log in
before you can comment on or make changes to this bug.
Description
•