Fixed more issues

Fixed a displacement issue

The displacement filter was assuming that both inputs were of the same size, which is true in blink, but not necessarily in a compromised stream.

BUG=327372
Committed: http://code.google.com/p/skia/source/detail?r=12655

[Stephen White, 2013-12-09 21:16:12]

https://codereview.chromium.org/101623007/diff/1/src/core/SkXfermode.cpp
File src/core/SkXfermode.cpp (right):

https://codereview.chromium.org/101623007/diff/1/src/core/SkXfermode.cpp#newc...
src/core/SkXfermode.cpp:471: if((n < 0) && (n = L - n)) { // Store denom in 'n'
and make sure it's non zero
Mike (or Florin? not sure who wrote this) should probably take a look.

https://codereview.chromium.org/101623007/diff/1/src/effects/SkDisplacementMa...
File src/effects/SkDisplacementMapEffect.cpp (right):

https://codereview.chromium.org/101623007/diff/1/src/effects/SkDisplacementMa...
src/effects/SkDisplacementMapEffect.cpp:210: if
(!this->applyCropRect(&displBounds, ctm)) {
Please add a GM test case for this.

[Stephen White, 2013-12-09 21:17:22]

https://codereview.chromium.org/101623007/diff/1/src/effects/SkDisplacementMa...
File src/effects/SkDisplacementMapEffect.cpp (right):

https://codereview.chromium.org/101623007/diff/1/src/effects/SkDisplacementMa...
src/effects/SkDisplacementMapEffect.cpp:345: src.getBounds(&bounds);
Shouldn't we also be applying the crop rect to the displacement image here?

[reed1, 2013-12-09 21:20:48]

https://codereview.chromium.org/101623007/diff/1/src/core/SkXfermode.cpp
File src/core/SkXfermode.cpp (right):

https://codereview.chromium.org/101623007/diff/1/src/core/SkXfermode.cpp#newc...
src/core/SkXfermode.cpp:471: if((n < 0) && (n = L - n)) { // Store denom in 'n'
and make sure it's non zero
On 2013/12/09 21:16:12, Stephen White wrote:
> Mike (or Florin? not sure who wrote this) should probably take a look.

I don't know who wrote this.
For readability, I'd vote for a local var (int denom;) rather than reusing
either x or n.

[reed1, 2013-12-09 21:21:06]

btw -- do we have a gm that exercises clipColor?

[fmalita_google_do_not_use, 2013-12-09 21:29:11]

https://codereview.chromium.org/101623007/diff/1/src/core/SkXfermode.cpp
File src/core/SkXfermode.cpp (right):

https://codereview.chromium.org/101623007/diff/1/src/core/SkXfermode.cpp#newc...
src/core/SkXfermode.cpp:471: if((n < 0) && (n = L - n)) { // Store denom in 'n'
and make sure it's non zero
On 2013/12/09 21:16:12, Stephen White wrote:
> Mike (or Florin? not sure who wrote this) should probably take a look.

FWIW, this was added by Rik (https://chromiumcodereview.appspot.com/12393049).

[sugoi, 2013-12-11 19:01:39]

On 2013/12/09 21:21:06, reed1 wrote:
> btw -- do we have a gm that exercises clipColor?

Yes, there should be, through xfermodeimagefilter, which tests modes like Hue
(SkXfermode::kHue_Mode), for example, which uses hue_modeproc(), which calls
SetLum(), which calls clipColor(). (Sorry for the delay before this response, my
linux desktop had network issues yesterday which I only managed to fix this
morning after re-imaging the computer).

[sugoi, 2013-12-11 19:40:06]

Added new gm cases to test changes to displacement

[Stephen White, 2013-12-12 02:35:59]

https://codereview.chromium.org/101623007/diff/20001/src/core/SkXfermode.cpp
File src/core/SkXfermode.cpp (right):

https://codereview.chromium.org/101623007/diff/20001/src/core/SkXfermode.cpp#...
src/core/SkXfermode.cpp:472: if((n < 0) && (denom = L - n)) { // Compute denom
and make sure it's non zero
Hmm.. I'd just unconditionally compute the denom, rather than doing it in the
if. If you're worried about performance, add a bench to convince me it matters.
:)

[sugoi, 2013-12-12 15:38:26]

I will do further investigation (bench tests) on the SkXfermode fix, but, for
now, I don't want to delay the displacement fix any longer, so I'll do the
SkXfermode fix in a different cl.

[Stephen White, 2013-12-12 17:15:31]

LGTM

[commit-bot: I haz the power, 2013-12-12 18:32:13]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/sugoi@chromium.org/101623007/40001

[commit-bot: I haz the power, 2013-12-12 18:38:11]

Retried try job too often on Build-Mac10.7-Clang-x86-Release-Trybot for step(s)
BuildBench, BuildEverything, BuildGm, BuildSkiaLib, BuildTests, BuildTools
http://skiabot-master.pogerlabs.com:10117/buildstatus?builder=Build-Mac10.7-C...

[commit-bot: I haz the power, 2013-12-12 19:11:28]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/sugoi@chromium.org/101623007/40001

[commit-bot: I haz the power, 2013-12-12 19:17:52]

Retried try job too often on Build-Mac10.7-Clang-x86-Release-Trybot for step(s)
BuildBench, BuildEverything, BuildGm, BuildSkiaLib, BuildTests, BuildTools
http://skiabot-master.pogerlabs.com:10117/buildstatus?builder=Build-Mac10.7-C...

[commit-bot: I haz the power, 2013-12-12 19:44:12]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/sugoi@chromium.org/101623007/60001

[commit-bot: I haz the power, 2013-12-12 21:05:49]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/sugoi@chromium.org/101623007/100001

[commit-bot: I haz the power, 2013-12-12 21:06:01]

Failed to apply patch for expectations/gm/ignored-tests.txt:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file expectations/gm/ignored-tests.txt
  Hunk #1 FAILED at 75.
  1 out of 1 hunk FAILED -- saving rejects to file
expectations/gm/ignored-tests.txt.rej

Patch:       expectations/gm/ignored-tests.txt
Index: expectations/gm/ignored-tests.txt
diff --git a/expectations/gm/ignored-tests.txt
b/expectations/gm/ignored-tests.txt
index
f3edab2ecb1fad4d0758bda87a895528d0f1101e..7438614a9c34b817f266abcf2eb72f266e008181
100644
--- a/expectations/gm/ignored-tests.txt
+++ b/expectations/gm/ignored-tests.txt
@@ -75,3 +75,6 @@ clipped-bitmap-shaders-clamp-hq_angle
 clipped-bitmap-shaders-mirror-hq_angle
 clipped-bitmap-shaders-tile-hq_angle
 
+# Added by sugoi in https://codereview.chromium.org/101623007
+displacement
+

[commit-bot: I haz the power, 2013-12-12 21:12:40]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/sugoi@chromium.org/101623007/120001

[commit-bot: I haz the power, 2013-12-12 21:48:35]

Change committed as 12655