Fixes some wrong uses of CLAMP_D_TO_U8 in filtering code (mostly based on my misunderstanding of this clamp function that actually does round+clamp). Plus some extra comments to warn of this behaviour.

(bzr r7627)

8 files changed, 25 insertions, 17 deletions

diff --git a/src/display/nr-filter-colormatrix.cpp b/src/display/nr-filter-colormatrix.cpp
index 21e659a6a..66fb196cb 100644
--- a/src/display/nr-filter-colormatrix.cpp
+++ b/src/display/nr-filter-colormatrix.cpp
@@ -91,7 +91,7 @@ int FilterColorMatrix::render(FilterSlot &slot, FilterUnits const &/*units*/) {
                         g = in_data[i+1];
                         b = in_data[i+2];
                         a = in_data[i+3];
-                        out_data[i] = CLAMP_D_TO_U8( r*values[0] + g*values[1] + b*values[2] + a*values[3] + a04 );
+                        out_data[i] = CLAMP_D_TO_U8( r*values[0] + g*values[1] + b*values[2] + a*values[3] + a04 ); // CLAMP includes rounding!
                         out_data[i+1] = CLAMP_D_TO_U8( r*values[5] + g*values[6] + b*values[7] + a*values[8] + a14 );
                         out_data[i+2] = CLAMP_D_TO_U8( r*values[10] + g*values[11] + b*values[12] + a*values[13] + a24 );
                         out_data[i+3] = CLAMP_D_TO_U8( r*values[15] + g*values[16] + b*values[17] + a*values[18] + a34 );
diff --git a/src/display/nr-filter-component-transfer.cpp b/src/display/nr-filter-component-transfer.cpp
index bd52c071e..87f87c95a 100644
--- a/src/display/nr-filter-component-transfer.cpp
+++ b/src/display/nr-filter-component-transfer.cpp
@@ -98,6 +98,7 @@ int FilterComponentTransfer::render(FilterSlot &slot, FilterUnits const &/*units
                     if (!premultiplied || color==3) {
                         std::vector<gdouble> _tableValues(tableValues[color]);
                         // Scale by 255 and add .5 to avoid having to add it later for rounding purposes
+                        //   Note that this means that CLAMP_D_TO_U8 cannot be used here (as it includes rounding!)
                         for(i=0;i<_vsize;i++) {
                             _tableValues[i] = std::max(0.,std::min(255.,255*_tableValues[i])) + .5;
                         }
@@ -115,7 +116,7 @@ int FilterComponentTransfer::render(FilterSlot &slot, FilterUnits const &/*units
                             if (in_data[i+3-color]==0) continue;
                             int k = ((_vsize-1) * in_data[i]) / in_data[i+3-color];
                             double dx = ((_vsize-1) * in_data[i]) / (double)in_data[i+3-color] - k;
-                            out_data[i] = static_cast<unsigned char>(out_data[i+3-color] * (_tableValues[k] + dx * (_tableValues[k+1] - _tableValues[k])) + .5);
+                            out_data[i] = CLAMP_D_TO_U8_ALPHA(out_data[i+3-color] * (_tableValues[k] + dx * (_tableValues[k+1] - _tableValues[k])), out_data[i+3-color]); // CLAMP includes rounding!
                         }
                     }
                 }
@@ -130,7 +131,7 @@ int FilterComponentTransfer::render(FilterSlot &slot, FilterUnits const &/*units
                         std::vector<unsigned char> _tableValues(_vsize);
                         // Convert to unsigned char
                         for(i=0;i<_vsize;i++) {
-                            _tableValues[i] = static_cast<unsigned char>(std::max(0.,std::min(255.,255*tableValues[color][i])) + .5);
+                            _tableValues[i] = CLAMP_D_TO_U8(255*tableValues[color][i]);
                         }
                         for(i=color;i<size;i+=4){
                             int k = FAST_DIVIDE<255>((_vsize-1) * in_data[i]);
@@ -144,35 +145,37 @@ int FilterComponentTransfer::render(FilterSlot &slot, FilterUnits const &/*units
                         for(i=color;i<size;i+=4){
                             if (in_data[i+3-color]==0) continue;
                             int k = ((_vsize-1) * in_data[i]) / in_data[i+3-color];
-                            out_data[i] =  static_cast<unsigned char>(out_data[i+3-color] * _tableValues[k] + .5);
+                            out_data[i] = CLAMP_D_TO_U8_ALPHA(out_data[i+3-color] * _tableValues[k], out_data[i+3-color]);
                         }
                     }
                 }
                 break;
             case COMPONENTTRANSFER_TYPE_LINEAR:
                 if (!premultiplied || color==3) {
-                    _intercept = 255*_intercept + .5;
+                    _intercept = 255*_intercept;
                     for(i=color;i<size;i+=4){
-                        out_data[i] = CLAMP_D_TO_U8(_slope * in_data[i] + _intercept);
+                        out_data[i] = CLAMP_D_TO_U8(_slope * in_data[i] + _intercept); // CLAMP includes rounding!
                     }
                 } else {
                     for(i=color;i<size;i+=4){
                         if (in_data[i+3-color]==0) continue;
-                        out_data[i] = CLAMP_D_TO_U8(out_data[i+3-color] * (_slope * in_data[i] / in_data[i+3-color] + _intercept) + .5);
+                        double out = _slope * in_data[i] / in_data[i+3-color] + _intercept;
+                        out_data[i] = CLAMP_D_TO_U8_ALPHA(out_data[i+3-color] * out, out_data[i+3-color]);
                     }
                 }
                 break;
             case COMPONENTTRANSFER_TYPE_GAMMA:
                 if (!premultiplied || color==3) {
                     _amplitude *= pow(255.0, -_exponent+1); // The input should be divided by 255, then exponentiated and then multiplied by 255 again, instead the amplitude is modified accordingly.
-                    _offset = 255*_offset + .5;
+                    _offset = 255*_offset;
                     for(i=color;i<size;i+=4){
                         out_data[i] = CLAMP_D_TO_U8(_amplitude * pow((double)in_data[i], _exponent) + _offset);
                     }
                 } else {
                     for(i=color;i<size;i+=4){
                         if (in_data[i+3-color]==0) continue;
-                        out_data[i] = CLAMP_D_TO_U8(out_data[i+3-color] * (_amplitude * pow((double)in_data[i] / in_data[i+3-color], _exponent) + _offset) + .5);
+                        double out = _amplitude * pow((double)in_data[i] / in_data[i+3-color], _exponent) + _offset;
+                        out_data[i] = CLAMP_D_TO_U8_ALPHA(out_data[i+3-color] * out, out_data[i+3-color]);
                     }
                 }
                 break;
diff --git a/src/display/nr-filter-composite.cpp b/src/display/nr-filter-composite.cpp
index e68c0bd90..51652d743 100644
--- a/src/display/nr-filter-composite.cpp
+++ b/src/display/nr-filter-composite.cpp
@@ -154,10 +154,10 @@ int FilterComposite::render(FilterSlot &slot, FilterUnits const &/*units*/) {
             pixops_mix<composite_xor>(*out, *in1, *in2);
             break;
         case COMPOSITE_ARITHMETIC:
-            arith_k1 = (int)(k1 * 255);
-            arith_k2 = (int)(k2 * 255 * 255);
-            arith_k3 = (int)(k3 * 255 * 255);
-            arith_k4 = (int)(k4 * 255 * 255 * 255);
+            arith_k1 = (int)round(k1 * 255);
+            arith_k2 = (int)round(k2 * 255 * 255);
+            arith_k3 = (int)round(k3 * 255 * 255);
+            arith_k4 = (int)round(k4 * 255 * 255 * 255);
             pixops_mix<composite_arithmetic>(*out, *in1, *in2);
             break;
         case COMPOSITE_DEFAULT:
diff --git a/src/display/nr-filter-convolve-matrix.cpp b/src/display/nr-filter-convolve-matrix.cpp
index 3fca952da..fc279df4c 100644
--- a/src/display/nr-filter-convolve-matrix.cpp
+++ b/src/display/nr-filter-convolve-matrix.cpp
@@ -78,7 +78,7 @@ int FilterConvolveMatrix::render(FilterSlot &slot, FilterUnits const &/*units*/)
                 }
             }
             unsigned int out_index = 4*( x + width*y );
-            out_data[out_index++] = CLAMP_D_TO_U8(result_R / divisor + bias);
+            out_data[out_index++] = CLAMP_D_TO_U8(result_R / divisor + bias); // CLAMP includes rounding!
             out_data[out_index++] = CLAMP_D_TO_U8(result_G / divisor + bias);
             out_data[out_index++] = CLAMP_D_TO_U8(result_B / divisor + bias);
 
diff --git a/src/display/nr-filter-diffuselighting.cpp b/src/display/nr-filter-diffuselighting.cpp
index bf5b97fb1..8915c88b1 100644
--- a/src/display/nr-filter-diffuselighting.cpp
+++ b/src/display/nr-filter-diffuselighting.cpp
@@ -87,7 +87,7 @@ int FilterDiffuseLighting::render(FilterSlot &slot, FilterUnits const &units) {
                 NR::compute_surface_normal(N, ss, in, i / w, i % w, dx, dy);
                 inter = kd * NR::scalar_product(N, L);
 
-                data_o[j++] = CLAMP_D_TO_U8(inter * LC[LIGHT_RED]);
+                data_o[j++] = CLAMP_D_TO_U8(inter * LC[LIGHT_RED]); // CLAMP includes rounding!
                 data_o[j++] = CLAMP_D_TO_U8(inter * LC[LIGHT_GREEN]);
                 data_o[j++] = CLAMP_D_TO_U8(inter * LC[LIGHT_BLUE]);
                 data_o[j++] = 255;
diff --git a/src/display/nr-filter-specularlighting.cpp b/src/display/nr-filter-specularlighting.cpp
index 526f49ec8..b096ee49d 100644
--- a/src/display/nr-filter-specularlighting.cpp
+++ b/src/display/nr-filter-specularlighting.cpp
@@ -105,7 +105,7 @@ int FilterSpecularLighting::render(FilterSlot &slot, FilterUnits const &units) {
                 NR::compute_surface_normal(N, ss, in, i / w, i % w, dx, dy);
                 COMPUTE_INTER(inter, N, H, ks, specularExponent);
 
-                data_o[j++] = CLAMP_D_TO_U8(inter * LC[LIGHT_RED]);
+                data_o[j++] = CLAMP_D_TO_U8(inter * LC[LIGHT_RED]); // CLAMP includes rounding!
                 data_o[j++] = CLAMP_D_TO_U8(inter * LC[LIGHT_GREEN]);
                 data_o[j++] = CLAMP_D_TO_U8(inter * LC[LIGHT_BLUE]);
                 data_o[j] = MAX(MAX(data_o[j-3], data_o[j-2]), data_o[j-1]);
diff --git a/src/display/nr-filter-turbulence.cpp b/src/display/nr-filter-turbulence.cpp
index 1336e5f79..a91db3d56 100644
--- a/src/display/nr-filter-turbulence.cpp
+++ b/src/display/nr-filter-turbulence.cpp
@@ -100,7 +100,7 @@ void FilterTurbulence::render_area(NRPixBlock *pix, NR::IRect &full_area, Filter
             for (int x = std::max(bbox_x0, pix->area.x0); x < std::min(bbox_x1, pix->area.x1); x++){
                 int out_pos = out_line + 4 * (x - pix->area.x0);
                 point[0] = x * unit_trans[0] + unit_trans[4];
-                pb[out_pos] = CLAMP_D_TO_U8( turbulence(0,point)*255 );
+                pb[out_pos] = CLAMP_D_TO_U8( turbulence(0,point)*255 ); // CLAMP includes rounding!
                 pb[out_pos + 1] = CLAMP_D_TO_U8( turbulence(1,point)*255 );
                 pb[out_pos + 2] = CLAMP_D_TO_U8( turbulence(2,point)*255 );
                 pb[out_pos + 3] = CLAMP_D_TO_U8( turbulence(3,point)*255 );
diff --git a/src/display/nr-filter-utils.h b/src/display/nr-filter-utils.h
index c825a814e..5c59a0e84 100644
--- a/src/display/nr-filter-utils.h
+++ b/src/display/nr-filter-utils.h
@@ -64,6 +64,11 @@ inline int clamp_alpha(int const val, int const alpha) {
     return val;
 }
 
+/**
+ * Macro to use the clamp function with double inputs and unsigned char output
+ */
+#define CLAMP_D_TO_U8_ALPHA(v,a) (unsigned char) clamp_alpha((int)round((v)),(a))
+
 } /* namespace Filters */
 } /* namespace Inkscape */