Add the 'lighter' composite operation to feComposite.

Add the 'lighter' composite operator to feComposite.

Spec: http://www.w3.org/TR/filter-effects-1/#valdef-operator-lighter

BUG=439037
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187029

[Erik Dahlström (inactive), 2014-12-05 13:46:49]

ed@opera.com changed reviewers:
+ dschulze@chromium.org, pdr@chromium.org

[Erik Dahlström (inactive), 2014-12-05 13:46:50]

PTAL

[krit, 2014-12-05 13:53:52]

lgtm

lgtm

I like the idea of the Canvas reference test.

[Erik Dahlström (inactive), 2014-12-05 14:06:42]

ed@opera.com changed reviewers:
+ schenney@google.com

[Erik Dahlström (inactive), 2014-12-10 12:44:22]

Looking for a Source/platform green light too, pdr or schenney?

[pdr., 2014-12-10 20:58:08]

https://codereview.chromium.org/779963002/diff/20001/Source/core/svg/SVGFECom...
File Source/core/svg/SVGFECompositeElement.cpp (right):

https://codereview.chromium.org/779963002/diff/20001/Source/core/svg/SVGFECom...
Source/core/svg/SVGFECompositeElement.cpp:46: template<> unsigned short
getMaxExposedEnumValue<CompositeOperationType>()
Did you mean to leave this in the patch? This is deadcode.

[pdr., 2014-12-10 21:04:38]

LGTM with deadcode removed.

[fs, 2014-12-11 09:30:19]

fs@opera.com changed reviewers:
+ fs@opera.com

[fs, 2014-12-11 09:30:20]

https://codereview.chromium.org/779963002/diff/20001/Source/core/svg/SVGFECom...
File Source/core/svg/SVGFECompositeElement.cpp (right):

https://codereview.chromium.org/779963002/diff/20001/Source/core/svg/SVGFECom...
Source/core/svg/SVGFECompositeElement.cpp:46: template<> unsigned short
getMaxExposedEnumValue<CompositeOperationType>()
On 2014/12/10 20:58:08, pdr wrote:
> Did you mean to leave this in the patch? This is deadcode.

I don't think it's dead code - it's just really non-obvious code [1]. Might be
worth adding a test just to keep the assumption visible.

[1] SVGAnimatedEnumeration<CompositeOperationType>::create (m_svgOperator below)
which calls SVGEnumeration<Enum>::create, which in turn calls the constructor,
which calls this method (and the one above it.) Don't you just love templates?
=)
I have some ideas for how to make this more obvious, but still need to put those
to the test...

[fs, 2014-12-11 13:21:02]

https://codereview.chromium.org/779963002/diff/40001/LayoutTests/svg/dom/scri...
File
LayoutTests/svg/dom/script-tests/SVGAnimatedEnumeration-SVGFECompositeElement.js
(right):

https://codereview.chromium.org/779963002/diff/40001/LayoutTests/svg/dom/scri...
LayoutTests/svg/dom/script-tests/SVGAnimatedEnumeration-SVGFECompositeElement.js:63:
debug("Switch to 'lighter'");
Maybe extend this with something making it clear that
SVG_FECOMPOSITE_OPERATOR_LIGHTER should not (and is not) exposed on
SVGFECompositeElement (and hence is just 'undefined').

[Erik Dahlström (inactive), 2014-12-11 13:36:21]

On 2014/12/11 13:21:02, fs wrote:
>
https://codereview.chromium.org/779963002/diff/40001/LayoutTests/svg/dom/scri...
> File
>
LayoutTests/svg/dom/script-tests/SVGAnimatedEnumeration-SVGFECompositeElement.js
> (right):
> 
>
https://codereview.chromium.org/779963002/diff/40001/LayoutTests/svg/dom/scri...
>
LayoutTests/svg/dom/script-tests/SVGAnimatedEnumeration-SVGFECompositeElement.js:63:
> debug("Switch to 'lighter'");
> Maybe extend this with something making it clear that
> SVG_FECOMPOSITE_OPERATOR_LIGHTER should not (and is not) exposed on
> SVGFECompositeElement (and hence is just 'undefined').

Done.

The supposedly dead code isn't really dead (as explained by fs), and without it
the original SVGAnimatedEnumeration-SVGFECompositeElement test fails.

[Erik Dahlström (inactive), 2014-12-11 13:36:39]

The CQ bit was checked by ed@opera.com

[commit-bot: I haz the power, 2014-12-11 13:37:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/779963002/60001

[commit-bot: I haz the power, 2014-12-11 13:37:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-11 13:37:28]

Failed to apply patch for LayoutTests/TestExpectations:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file LayoutTests/TestExpectations
  Hunk #1 FAILED at 1424.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/TestExpectations.rej

Patch:       LayoutTests/TestExpectations
Index: LayoutTests/TestExpectations
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index
7afeb5066e8df1495f18533227ac1ee712146a7d..329ab7c5a1b216ca32600184da6b8c3ba6236a1c
100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -1424,3 +1424,5 @@ crbug.com/440272 [ SnowLeopard Debug ]
virtual/slimmingpaint/fast/text/internati
 crbug.com/440277 crypto/abandon-crypto-operation.html [ Pass Crash ]
 crbug.com/439492 [ XP ] svg/batik/masking/maskRegions.svg [ NeedsRebaseline ]
 crbug.com/410929
virtual/gpu/fast/canvas/canvas-toDataURL-jpeg-maximum-quality.html [ Pass
ImageOnlyFailure ]
+
+crbug.com/439037 svg/filters/feComposite.svg [ NeedsRebaseline ]

[Erik Dahlström (inactive), 2014-12-11 14:14:17]

The CQ bit was checked by ed@opera.com

[commit-bot: I haz the power, 2014-12-11 14:15:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/779963002/80001

[commit-bot: I haz the power, 2014-12-11 15:30:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-11 15:30:43]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/3...)

[Erik Dahlström (inactive), 2014-12-11 15:53:29]

The CQ bit was checked by ed@opera.com

[commit-bot: I haz the power, 2014-12-11 15:54:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/779963002/80001

[commit-bot: I haz the power, 2014-12-11 18:12:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-11 18:12:03]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/40935)

[Erik Dahlström (inactive), 2014-12-12 08:41:14]

The CQ bit was checked by ed@opera.com

[commit-bot: I haz the power, 2014-12-12 08:41:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/779963002/80001

[commit-bot: I haz the power, 2014-12-12 10:56:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-12 10:56:09]

Failed to apply patch for LayoutTests/TestExpectations:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file LayoutTests/TestExpectations
  Hunk #1 FAILED at 1385.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/TestExpectations.rej

Patch:       LayoutTests/TestExpectations
Index: LayoutTests/TestExpectations
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index
d9050f0d32bcd1392d7b421f3fbc90460ce10fd2..a8904a1b16c6034c7052d276627ab87a9a2e7fcf
100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -1385,3 +1385,4 @@ crbug.com/410929
virtual/gpu/fast/canvas/canvas-toDataURL-jpeg-maximum-quality.h
 
 crbug.com/396013 inspector/console/console-error-on-call-frame.html [ Skip ]
 crbug.com/396013
inspector/sources/debugger/debugger-eval-on-call-frame-inside-iframe.html [ Skip
]
+crbug.com/439037 svg/filters/feComposite.svg [ NeedsRebaseline ]

[commit-bot: I haz the power, 2014-12-12 10:56:16]

The CQ bit was unchecked by commit-bot@chromium.org

[Erik Dahlström (inactive), 2014-12-12 11:22:23]

The CQ bit was checked by ed@opera.com

[commit-bot: I haz the power, 2014-12-12 11:23:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/779963002/100001

[commit-bot: I haz the power, 2014-12-12 13:08:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-12 13:08:44]

Try jobs failed on following builders:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/3...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/35110)

[Erik Dahlström (inactive), 2014-12-12 13:58:42]

The CQ bit was checked by ed@opera.com

[commit-bot: I haz the power, 2014-12-12 13:58:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/779963002/120001

[commit-bot: I haz the power, 2014-12-12 15:42:27]

Committed patchset #7 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187029