Bug 141376

Summary: [SVG] Add support for 'lighter' operator in feComposite
Product: WebKit Reporter: Nikos Andronikos <nikos.andronikos>
Component: SVGAssignee: Nikos Andronikos <nikos.andronikos>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, krit, rniwa, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 152814    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews103 for mac-yosemite
none
Archive of layout-test-results from ews107 for mac-yosemite-wk2
none
Archive of layout-test-results from ews115 for mac-yosemite
none
Patch none

Comment 1 Dirk Schulze 2015-02-08 14:58:32 PST
You can take a look at http://trac.webkit.org/changeset/170433 to see how new compositing modes (that are not part of SVGOM) can be implemented.
Comment 2 Nikos Andronikos 2015-12-16 21:51:44 PST
Created attachment 267534 [details]
Patch
Comment 3 WebKit Commit Bot 2015-12-16 21:54:24 PST
Attachment 267534 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/filters/FEComposite.h:39:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/graphics/filters/FEComposite.h:40:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Nikos Andronikos 2015-12-16 21:56:51 PST
(In reply to comment #3)
> Attachment 267534 [details] did not pass style-queue:

Note: style failures are false positives in this case. SVG code uses all caps for enums everywhere.
Comment 5 Build Bot 2015-12-16 22:41:39 PST
Comment on attachment 267534 [details]
Patch

Attachment 267534 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/570037

New failing tests:
svg/dom/SVGAnimatedEnumeration-SVGFECompositeElement.html
Comment 6 Build Bot 2015-12-16 22:41:43 PST
Created attachment 267540 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 7 Build Bot 2015-12-16 22:44:10 PST
Comment on attachment 267534 [details]
Patch

Attachment 267534 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/570034

New failing tests:
svg/dom/SVGAnimatedEnumeration-SVGFECompositeElement.html
Comment 8 Build Bot 2015-12-16 22:44:13 PST
Created attachment 267541 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 9 Build Bot 2015-12-16 22:45:47 PST
Comment on attachment 267534 [details]
Patch

Attachment 267534 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/570020

New failing tests:
svg/dom/SVGAnimatedEnumeration-SVGFECompositeElement.html
Comment 10 Build Bot 2015-12-16 22:45:51 PST
Created attachment 267542 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 11 Nikos Andronikos 2016-01-27 21:04:53 PST
Created attachment 270087 [details]
Patch
Comment 12 WebKit Commit Bot 2016-01-27 21:06:35 PST
Attachment 270087 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/filters/FEComposite.h:39:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/graphics/filters/FEComposite.h:40:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Darin Adler 2016-01-28 08:45:46 PST
Comment on attachment 270087 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=270087&action=review

> Source/WebCore/svg/SVGFECompositeElement.h:56
> +            return "lighter";

All the values in this function should be returning ASCIILiteral("lighter") instead.
Comment 14 WebKit Commit Bot 2016-01-28 09:33:53 PST
Comment on attachment 270087 [details]
Patch

Clearing flags on attachment: 270087

Committed r195745: <http://trac.webkit.org/changeset/195745>
Comment 15 WebKit Commit Bot 2016-01-28 09:33:58 PST
All reviewed patches have been landed.  Closing bug.
Comment 16 Nikos Andronikos 2016-01-28 17:35:15 PST
(In reply to comment #13)
> Comment on attachment 270087 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=270087&action=review
> 
> > Source/WebCore/svg/SVGFECompositeElement.h:56
> > +            return "lighter";
> 
> All the values in this function should be returning ASCIILiteral("lighter")
> instead.

Darin, what's the process for making this change now that the patch has landed? Or is this just something to keep in mind for next time?
Thanks for reviewing btw.
Comment 17 Said Abou-Hallawa 2016-02-01 09:32:40 PST
Comment on attachment 270087 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=270087&action=review

>>> Source/WebCore/svg/SVGFECompositeElement.h:56
>>> +            return "lighter";
>> 
>> All the values in this function should be returning ASCIILiteral("lighter") instead.
> 
> Darin, what's the process for making this change now that the patch has landed? Or is this just something to keep in mind for next time?
> Thanks for reviewing btw.

You can file a new webkit bug and mention that it is a follow up for this bug. Or you can reopen this bug and attach a new patch and get it reviewed and landed as the current patch.