Fix for displacement mapping

src/effects/SkDisplacementMapEffect.cpp

Index: src/effects/SkDisplacementMapEffect.cpp
===================================================================
--- src/effects/SkDisplacementMapEffect.cpp	(revision 8169)
+++ src/effects/SkDisplacementMapEffect.cpp	(working copy)
@@ -50,26 +50,28 @@
 void computeDisplacement(SkScalar scale, SkBitmap* dst, SkBitmap* displ, SkBitmap* src)
 {
     static const SkScalar Inv8bit = SkScalarDiv(SK_Scalar1, SkFloatToScalar(255.0f));
-    static const SkScalar Half8bit = SkFloatToScalar(255.0f * 0.5f);
     const int dstW = displ->width();
     const int dstH = displ->height();
     const int srcW = src->width();
     const int srcH = src->height();
-    const SkScalar scaleX = SkScalarMul(SkScalarMul(scale, SkIntToScalar(dstW)), Inv8bit);
-    const SkScalar scaleY = SkScalarMul(SkScalarMul(scale, SkIntToScalar(dstH)), Inv8bit);
+    const SkScalar scaleForColor = SkScalarMul(scale, Inv8bit);
+    const SkScalar scaleAdj = SK_ScalarHalf - SkScalarMul(scale, SK_ScalarHalf);
     const SkUnPreMultiply::Scale* table = SkUnPreMultiply::GetScaleTable();
     for (int y = 0; y < dstH; ++y) {
         const SkPMColor* displPtr = displ->getAddr32(0, y);
         SkPMColor* dstPtr = dst->getAddr32(0, y);
         for (int x = 0; x < dstW; ++x, ++displPtr, ++dstPtr) {
-            const SkScalar displX =
-                SkScalarMul(scaleX, SkIntToScalar(getValue<typeX>(*displPtr, table))-Half8bit);
-            const SkScalar displY =
-                SkScalarMul(scaleY, SkIntToScalar(getValue<typeY>(*displPtr, table))-Half8bit);
-            const int coordX = x + SkScalarRoundToInt(displX);
-            const int coordY = y + SkScalarRoundToInt(displY);
-            *dstPtr = ((coordX < 0) || (coordX >= srcW) || (coordY < 0) || (coordY >= srcH)) ?
-                      0 : *(src->getAddr32(coordX, coordY));
+            const SkScalar displX = SkScalarMul(scaleForColor,
+                SkIntToScalar(getValue<typeX>(*displPtr, table))) + scaleAdj;
+            const SkScalar displY = SkScalarMul(scaleForColor,
+                SkIntToScalar(getValue<typeY>(*displPtr, table))) + scaleAdj;
+            // Truncate the displacement values

[sugoi, 2013/03/15 17:47:06]

I didn't see any SkScalarTruncToInt or similar function, so I implemented the
trunc here.

[bsalomon, 2013/03/15 19:09:53]

Doesn't cast do what you want? If we care about fixed point perhaps it should be
wrapped in a macro.

+            const int srcX = x + (displX < 0 ? SkScalarCeilToInt(displX) :
+                                               SkScalarFloorToInt(displX));
+            const int srcY = y + (displY < 0 ? SkScalarCeilToInt(displY) :
+                                               SkScalarFloorToInt(displY));
+            *dstPtr = ((srcX < 0) || (srcX >= srcW) || (srcY < 0) || (srcY >= srcH)) ?
+                      0 : *(src->getAddr32(srcX, srcY));
         }
     }
 }
@@ -345,7 +347,7 @@
     return GrTBackendEffectFactory<GrDisplacementMapEffect>::getInstance();
 }
 
-void GrDisplacementMapEffect::getConstantColorComponents(GrColor* color,
+void GrDisplacementMapEffect::getConstantColorComponents(GrColor*,
                                                          uint32_t* validFlags) const {
     // Any displacement offset bringing a pixel out of bounds will output a color of (0,0,0,0),
     // so the only way we'd get a constant alpha is if the input color image has a constant alpha
@@ -360,7 +362,7 @@
 GR_DEFINE_EFFECT_TEST(GrDisplacementMapEffect);
 
 GrEffectRef* GrDisplacementMapEffect::TestCreate(SkMWCRandom* random,
-                                                 GrContext* context,
+                                                 GrContext*,
                                                  GrTexture* textures[]) {
     int texIdxDispl = random->nextBool() ? GrEffectUnitTest::kSkiaPMTextureIdx :
                                            GrEffectUnitTest::kAlphaTextureIdx;
@@ -373,7 +375,7 @@
     SkDisplacementMapEffect::ChannelSelectorType yChannelSelector =
         static_cast<SkDisplacementMapEffect::ChannelSelectorType>(
         random->nextRangeU(1, kMaxComponent));
-    SkScalar scale = random->nextUScalar1();
+    SkScalar scale = random->nextRangeScalar(0, SkFloatToScalar(100.0f));

[sugoi, 2013/03/15 17:47:06]

Changed the test range so that it's more appropriate.

 
     return GrDisplacementMapEffect::Create(xChannelSelector, yChannelSelector, scale,
                                            textures[texIdxDispl], textures[texIdxColor]);
@@ -491,10 +493,11 @@
                                stage.getCoordChangeMatrix(),
                                colorTex);
 
-    uman.set2f(fScaleUni, SkScalarToFloat(displacementMap.scale()),
-                colorTex->origin() == kTopLeft_GrSurfaceOrigin ?
-                SkScalarToFloat(displacementMap.scale()) :
-                SkScalarToFloat(-displacementMap.scale()));
+    SkScalar scaleX = SkScalarDiv(displacementMap.scale(), SkIntToScalar(colorTex->width()));

[sugoi, 2013/03/15 17:47:06]

The scale now has to be divided by the size in gpu to fit the cpu, which no
longer multiplies the scale by the size.

+    SkScalar scaleY = SkScalarDiv(displacementMap.scale(), SkIntToScalar(colorTex->height()));
+    uman.set2f(fScaleUni, SkScalarToFloat(scaleX),
+               colorTex->origin() == kTopLeft_GrSurfaceOrigin ?
+               SkScalarToFloat(scaleY) : SkScalarToFloat(-scaleY));
 }
 
 GrGLEffect::EffectKey GrGLDisplacementMapEffect::GenKey(const GrEffectStage& stage,