Fix two bugs with component transfer filter primitive:

1. Discrete type was ignoring last value in list.
2. Change in alpha was not propogated to pre-multiplied color
values leading to "ghosting".

(bzr r14532)

1 files changed, 47 insertions, 155 deletions

diff --git a/src/display/nr-filter-component-transfer.cpp b/src/display/nr-filter-component-transfer.cpp
index dd90193fe..dd37ab873 100644
--- a/src/display/nr-filter-component-transfer.cpp
+++ b/src/display/nr-filter-component-transfer.cpp
@@ -30,6 +30,32 @@ FilterPrimitive * FilterComponentTransfer::create() {
 FilterComponentTransfer::~FilterComponentTransfer()
 {}
 
+struct UnmultiplyAlpha {
+    guint32 operator()(guint32 in) {
+        EXTRACT_ARGB32(in, a, r, g, b);
+        if (a == 0 )
+            return in;
+        r = unpremul_alpha(r, a);
+        g = unpremul_alpha(g, a);
+        b = unpremul_alpha(b, a);
+        ASSEMBLE_ARGB32(out, a, r, g, b);
+        return out;
+    }
+};
+
+struct MultiplyAlpha {
+    guint32 operator()(guint32 in) {
+        EXTRACT_ARGB32(in, a, r, g, b);
+        if (a == 0 )
+            return in;
+        r = premul_alpha(r, a);
+        g = premul_alpha(g, a);
+        b = premul_alpha(b, a);
+        ASSEMBLE_ARGB32(out, a, r, g, b);
+        return out;
+    }
+};
+
 struct ComponentTransfer {
     ComponentTransfer(guint32 color)
         : _shift(color * 8)
@@ -40,11 +66,7 @@ protected:
     guint32 _mask;
 };
 
-template <bool alpha>
-struct ComponentTransferTable;
-
-template <>
-struct ComponentTransferTable<false> : public ComponentTransfer {
+struct ComponentTransferTable : public ComponentTransfer {
     ComponentTransferTable(guint32 color, std::vector<double> const &values)
         : ComponentTransfer(color)
         , _v(values.size())
@@ -55,49 +77,17 @@ struct ComponentTransferTable<false> : public ComponentTransfer {
     }
     guint32 operator()(guint32 in) {
         guint32 component = (in & _mask) >> _shift;
-        guint32 alpha = (in & 0xff000000) >> 24;
-        if (alpha == 0) return in;
-
-        component = (255 * component + alpha/2) / alpha;
         guint32 k = (_v.size() - 1) * component;
         guint32 dx = k % 255;  k /= 255;
         component = _v[k]*255 + (_v[k+1] - _v[k])*dx;
         component = (component + 127) / 255;
-        component = premul_alpha(component, alpha);
         return (in & ~_mask) | (component << _shift);
     }
 private:
     std::vector<guint32> _v;
 };
 
-template <>
-struct ComponentTransferTable<true> {
-    ComponentTransferTable(std::vector<double> const &values)
-        : _v(values.size())
-    {
-        for (unsigned i = 0; i< values.size(); ++i) {
-            _v[i] = round(CLAMP(values[i], 0.0, 1.0) * 255);
-        }
-    }
-    guint32 operator()(guint32 in) {
-        guint32 alpha = (in & 0xff000000) >> 24;
-        if (alpha == 0) return in;
-
-        guint32 k = (_v.size() - 1) * alpha;
-        guint32 dx = k % 255;  k /= 255;
-        alpha = _v[k]*255 + (_v[k+1] - _v[k])*dx;
-        alpha = (alpha + 127) / 255;
-        return (in & 0x00ffffff) | (alpha << 24);
-    }
-private:
-    std::vector<guint32> _v;
-};
-
-template <bool alpha>
-struct ComponentTransferDiscrete;
-
-template <>
-struct ComponentTransferDiscrete<false> : public ComponentTransfer {
+struct ComponentTransferDiscrete : public ComponentTransfer {
     ComponentTransferDiscrete(guint32 color, std::vector<double> const &values)
         : ComponentTransfer(color)
         , _v(values.size())
@@ -108,45 +98,15 @@ struct ComponentTransferDiscrete<false> : public ComponentTransfer {
     }
     guint32 operator()(guint32 in) {
         guint32 component = (in & _mask) >> _shift;
-        guint32 alpha = (in & 0xff000000) >> 24;
-        if (alpha == 0) return in;
-
-        component = (255 * component + alpha/2) / alpha;
-        guint32 k = (_v.size() - 1) * component / 255;
+        guint32 k = (_v.size()) * component / 255;
         component = _v[k];
-        component = premul_alpha(component, alpha);
-        return (in & ~_mask) | (component << _shift);
+        return (in & ~_mask) | ((guint32)component << _shift);
     }
 private:
     std::vector<guint32> _v;
 };
 
-template <>
-struct ComponentTransferDiscrete<true> {
-    ComponentTransferDiscrete(std::vector<double> const &values)
-        : _v(values.size())
-    {
-        for (unsigned i = 0; i< values.size(); ++i) {
-            _v[i] = round(CLAMP(values[i], 0.0, 1.0) * 255);
-        }
-    }
-    guint32 operator()(guint32 in) {
-        guint32 alpha = (in & 0xff000000) >> 24;
-        if (alpha == 0) return in;
-
-        guint32 k = (_v.size() - 1) * alpha / 255;
-        alpha = _v[k];
-        return (in & 0x00ffffff) | (alpha << 24);
-    }
-private:
-    std::vector<guint32> _v;
-};
-
-template <bool alpha>
-struct ComponentTransferLinear;
-
-template <>
-struct ComponentTransferLinear<false> : public ComponentTransfer {
+struct ComponentTransferLinear : public ComponentTransfer {
     ComponentTransferLinear(guint32 color, double intercept, double slope)
         : ComponentTransfer(color)
         , _intercept(round(intercept*255*255))
@@ -154,14 +114,10 @@ struct ComponentTransferLinear<false> : public ComponentTransfer {
     {}
     guint32 operator()(guint32 in) {
         gint32 component = (in & _mask) >> _shift;
-        guint32 alpha = (in & 0xff000000) >> 24;
-        if (alpha == 0) return 0;
 
         // TODO: this can probably be reduced to something simpler
-        component = (255 * component + alpha/2) / alpha;
         component = pxclamp(_slope * component + _intercept, 0, 255*255);
         component = (component + 127) / 255;
-        component = premul_alpha(component, alpha);
         return (in & ~_mask) | (component << _shift);
     }
 private:
@@ -169,28 +125,7 @@ private:
     gint32 _slope;
 };
 
-template <>
-struct ComponentTransferLinear<true> {
-    ComponentTransferLinear(double intercept, double slope)
-        : _intercept(round(intercept*255*255))
-        , _slope(round(slope*255))
-    {}
-    guint32 operator()(guint32 in) {
-        gint32 alpha = (in & 0xff000000) >> 24;
-        alpha = pxclamp(_slope * alpha + _intercept, 0, 255*255);
-        alpha = (alpha + 127) / 255;
-        return (in & 0x00ffffff) | (alpha << 24);
-    }
-private:
-    gint32 _intercept;
-    gint32 _slope;
-};
-
-template <bool alpha>
-struct ComponentTransferGamma;
-
-template <>
-struct ComponentTransferGamma<false> : public ComponentTransfer {
+struct ComponentTransferGamma : public ComponentTransfer {
     ComponentTransferGamma(guint32 color, double amplitude, double exponent, double offset)
         : ComponentTransfer(color)
         , _amplitude(amplitude)
@@ -199,13 +134,9 @@ struct ComponentTransferGamma<false> : public ComponentTransfer {
     {}
     guint32 operator()(guint32 in) {
         double component = (in & _mask) >> _shift;
-        guint32 alpha = (in & 0xff000000) >> 24;
-        if (alpha == 0) return 0;
-
-        double alphaf = alpha;
-        component /= alphaf;
+        component /= 255.0;
         component = _amplitude * pow(component, _exponent) + _offset;
-        guint32 cpx = pxclamp(component * alphaf, 0, 255);
+        guint32 cpx = pxclamp(component * 255.0, 0, 255);
         return (in & ~_mask) | (cpx << _shift);
     }
 private:
@@ -214,26 +145,6 @@ private:
     double _offset;
 };
 
-template <>
-struct ComponentTransferGamma<true> {
-    ComponentTransferGamma(double amplitude, double exponent, double offset)
-        : _amplitude(amplitude)
-        , _exponent(exponent)
-        , _offset(offset)
-    {}
-    guint32 operator()(guint32 in) {
-        double alpha = (in & 0xff000000) >> 24;
-        alpha /= 255.0;
-        alpha = _amplitude * pow(alpha, _exponent) + _offset;
-        guint32 cpx = pxclamp(alpha * 255.0, 0, 255);
-        return (in & 0x00ffffff) | (cpx << 24);
-    }
-private:
-    double _amplitude;
-    double _exponent;
-    double _offset;
-};
-
 void FilterComponentTransfer::render_cairo(FilterSlot &slot)
 {
     cairo_surface_t *input = slot.getcairo(_input);
@@ -252,31 +163,38 @@ void FilterComponentTransfer::render_cairo(FilterSlot &slot)
     //cairo_surface_t *outtemp = ink_cairo_surface_create_identical(out);
     ink_cairo_surface_blit(input, out);
 
+    // We need to operate on unmultipled by alpha color values otherwise a change in alpha screws
+    // up the premultiplied by alpha r, g, b values.
+    ink_cairo_surface_filter(out, out, UnmultiplyAlpha());
+
     // parameters: R = 0, G = 1, B = 2, A = 3
     // Cairo:      R = 2, G = 1, B = 0, A = 3
     // If tableValues is empty, use identity.
-    for (unsigned i = 0; i < 3; ++i) {
+    for (unsigned i = 0; i < 4; ++i) {
+
         guint32 color = 2 - i;
+        if(i==3) color = 3; // alpha
+
         switch (type[i]) {
         case COMPONENTTRANSFER_TYPE_TABLE:
             if(!tableValues[i].empty()) {
               ink_cairo_surface_filter(out, out,
-                  ComponentTransferTable<false>(color, tableValues[i]));
+                  ComponentTransferTable(color, tableValues[i]));
             }
             break;
         case COMPONENTTRANSFER_TYPE_DISCRETE:
             if(!tableValues[i].empty()) {
                 ink_cairo_surface_filter(out, out,
-                    ComponentTransferDiscrete<false>(color, tableValues[i]));
+                    ComponentTransferDiscrete(color, tableValues[i]));
             }
             break;
         case COMPONENTTRANSFER_TYPE_LINEAR:
             ink_cairo_surface_filter(out, out,
-                ComponentTransferLinear<false>(color, intercept[i], slope[i]));
+                ComponentTransferLinear(color, intercept[i], slope[i]));
             break;
         case COMPONENTTRANSFER_TYPE_GAMMA:
             ink_cairo_surface_filter(out, out,
-                ComponentTransferGamma<false>(color, amplitude[i], exponent[i], offset[i]));
+                ComponentTransferGamma(color, amplitude[i], exponent[i], offset[i]));
             break;
         case COMPONENTTRANSFER_TYPE_ERROR:
         case COMPONENTTRANSFER_TYPE_IDENTITY:
@@ -286,33 +204,7 @@ void FilterComponentTransfer::render_cairo(FilterSlot &slot)
         //ink_cairo_surface_blit(out, outtemp);
     }
 
-    // fast paths for alpha channel
-    switch (type[3]) {
-    case COMPONENTTRANSFER_TYPE_TABLE:
-        if(!tableValues[3].empty()) {
-          ink_cairo_surface_filter(out, out,
-              ComponentTransferTable<true>(tableValues[3]));
-        }
-        break;
-    case COMPONENTTRANSFER_TYPE_DISCRETE:
-        if(!tableValues[3].empty()) {
-          ink_cairo_surface_filter(out, out,
-              ComponentTransferDiscrete<true>(tableValues[3]));
-        }
-        break;
-    case COMPONENTTRANSFER_TYPE_LINEAR:
-        ink_cairo_surface_filter(out, out,
-            ComponentTransferLinear<true>(intercept[3], slope[3]));
-        break;
-    case COMPONENTTRANSFER_TYPE_GAMMA:
-        ink_cairo_surface_filter(out, out,
-            ComponentTransferGamma<true>(amplitude[3], exponent[3], offset[3]));
-        break;
-    case COMPONENTTRANSFER_TYPE_ERROR:
-    case COMPONENTTRANSFER_TYPE_IDENTITY:
-    default:
-        break;
-    }
+    ink_cairo_surface_filter(out, out, MultiplyAlpha());
 
     slot.set(_output, out);
     cairo_surface_destroy(out);