From 423740a942b7c6ce5313bfd4d09fe02c4feabbe7 Mon Sep 17 00:00:00 2001 From: jaspervdg Date: Sun, 5 Apr 2009 19:45:12 +0000 Subject: [PATCH] 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. --- src/display/nr-filter-colormatrix.cpp | 2 +- src/display/nr-filter-component-transfer.cpp | 19 +++++++++++-------- src/display/nr-filter-composite.cpp | 8 ++++---- src/display/nr-filter-convolve-matrix.cpp | 2 +- src/display/nr-filter-diffuselighting.cpp | 2 +- src/display/nr-filter-specularlighting.cpp | 2 +- src/display/nr-filter-turbulence.cpp | 2 +- src/display/nr-filter-utils.h | 5 +++++ 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 _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(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 _tableValues(_vsize); // Convert to unsigned char for(i=0;i<_vsize;i++) { - _tableValues[i] = static_cast(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((_vsize-1) * in_data[i]); @@ -144,35 +145,37 @@ int FilterComponentTransfer::render(FilterSlot &slot, FilterUnits const &/*units for(i=color;i(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(*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(*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 */ -- 2.30.2