Skip to content

Commit

Permalink
[fix bug 27607] Changed Pixel_from_hsla/Pixel_to_hsla to use the new …
Browse files Browse the repository at this point in the history
…range for saturation and lightness: 0-255. Added ability to use old ranges (0-100) by allowing percentage arguments. Used hacky way to check IM version to make sure code works for IM 6.4.9-6.5.8. Uncommented from_hsla test case and added new tests for percentages.
  • Loading branch information
baror committed Dec 21, 2009
1 parent 4dbfe84 commit fdea5b4
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 44 deletions.
4 changes: 2 additions & 2 deletions ext/RMagick/rmagick.h
Expand Up @@ -6,7 +6,7 @@
* Changes since Nov. 2009 copyright © by Benjamin Thomas and Omer Bar-or
*
* @file rmagick.h
* @version $Id: rmagick.h,v 1.280 2009/12/20 02:33:33 baror Exp $
* @version $Id: rmagick.h,v 1.281 2009/12/21 10:34:56 baror Exp $
* @author Tim Hunter
******************************************************************************/

Expand Down Expand Up @@ -1262,7 +1262,7 @@ extern char *rm_str2cstr(VALUE, long *);
extern int rm_check_num2dbl(VALUE);
extern double rm_fuzz_to_dbl(VALUE);
extern Quantum rm_app2quantum(VALUE);
extern double rm_percentage(VALUE);
extern double rm_percentage(VALUE,double);
extern double rm_str_to_pct(VALUE);
extern VALUE rm_define_enum_type(const char *);
extern void rm_write_temp_image(Image *, char *);
Expand Down
30 changes: 15 additions & 15 deletions ext/RMagick/rmimage.c
Expand Up @@ -6,7 +6,7 @@
* Changes since Nov. 2009 copyright © by Benjamin Thomas and Omer Bar-or
*
* @file rmimage.c
* @version $Id: rmimage.c,v 1.359 2009/12/20 02:33:33 baror Exp $
* @version $Id: rmimage.c,v 1.360 2009/12/21 10:34:57 baror Exp $
* @author Tim Hunter
******************************************************************************/

Expand Down Expand Up @@ -1192,7 +1192,7 @@ Image_bias_eq(VALUE self, VALUE pct)
double bias;

image = rm_check_frozen(self);
bias = rm_percentage(pct);
bias = rm_percentage(pct,1.0);
image->bias = bias * QuantumRange;

return self;
Expand Down Expand Up @@ -1648,7 +1648,7 @@ special_composite(Image *image, Image *overlay, double image_pct, double overlay
* - Default x_offset is 0
* - Default y_offset is 0
* - Percent can be a number or a string in the form "NN%"
* - The default value for dst_percent is 100.0-src_percent
* - The default value for dst_percent is 100%-src_percent
*
* @param argc number of input arguments
* @param argv array of input arguments
Expand Down Expand Up @@ -1683,11 +1683,11 @@ Image_blend(int argc, VALUE *argv, VALUE self)
switch (argc)
{
case 3:
dst_percent = rm_percentage(argv[2]) * 100.0;
src_percent = rm_percentage(argv[1]) * 100.0;
dst_percent = rm_percentage(argv[2],1.0) * 100.0;
src_percent = rm_percentage(argv[1],1.0) * 100.0;
break;
case 2:
src_percent = rm_percentage(argv[1]) * 100.0;
src_percent = rm_percentage(argv[1],1.0) * 100.0;
dst_percent = FMAX(100.0 - src_percent, 0);
break;
default:
Expand Down Expand Up @@ -4622,7 +4622,7 @@ Image_deskew(int argc, VALUE *argv, VALUE self)
sprintf(auto_crop_width, "%ld", width);
SetImageArtifact(image, "deskew:auto-crop", auto_crop_width);
case 1:
threshold = rm_percentage(argv[0]) * QuantumRange;
threshold = rm_percentage(argv[0],1.0) * QuantumRange;
case 0:
break;
default:
Expand Down Expand Up @@ -5053,9 +5053,9 @@ Image_dissolve(int argc, VALUE *argv, VALUE self)
switch (argc)
{
case 3:
dst_percent = rm_percentage(argv[2]) * 100.0;
dst_percent = rm_percentage(argv[2],1.0) * 100.0;
case 2:
src_percent = rm_percentage(argv[1]) * 100.0;
src_percent = rm_percentage(argv[1],1.0) * 100.0;
break;
default:
rb_raise(rb_eArgError, "wrong number of arguments (%d for 2 to 6)", argc);
Expand Down Expand Up @@ -11461,7 +11461,7 @@ Image_selective_blur_channel(int argc, VALUE *argv, VALUE self)

// threshold is either a floating-point number or a string in the form "NN%".
// Either way it's supposed to represent a percentage of the QuantumRange.
threshold = rm_percentage(argv[2]) * QuantumRange;
threshold = rm_percentage(argv[2],1.0) * QuantumRange;

GetExceptionInfo(&exception);
new_image = SelectiveBlurImageChannel(image, channels, radius, sigma, threshold, &exception);
Expand Down Expand Up @@ -11841,7 +11841,7 @@ Image_shadow(int argc, VALUE *argv, VALUE self)
switch (argc)
{
case 4:
opacity = rm_percentage(argv[3]); // Clamp to 1.0 < x <= 100.0
opacity = rm_percentage(argv[3],1.0); // Clamp to 1.0 < x <= 100.0
if (fabs(opacity) < 0.01)
{
rb_warning("shadow will be transparent - opacity %g very small", opacity);
Expand Down Expand Up @@ -14268,8 +14268,8 @@ Image_virtual_pixel_method_eq(VALUE self, VALUE method)
* - @verbatim Image#watermark(mark, brightness, saturation, x_off, y_off) @endverbatim
*
* Notes:
* - Default brightness is 100.0
* - Default saturation is 100.0
* - Default brightness is 100%
* - Default saturation is 100%
* - Default x_off is 0
* - Default y_off is 0
* - x_off and y_off can be negative, which means measure from the
Expand Down Expand Up @@ -14305,9 +14305,9 @@ Image_watermark(int argc, VALUE *argv, VALUE self)
switch (argc)
{
case 3:
dst_percent = rm_percentage(argv[2]) * 100.0;
dst_percent = rm_percentage(argv[2],1.0) * 100.0;
case 2:
src_percent = rm_percentage(argv[1]) * 100.0;
src_percent = rm_percentage(argv[1],1.0) * 100.0;
case 1:
break;
default:
Expand Down
42 changes: 27 additions & 15 deletions ext/RMagick/rmpixel.c
Expand Up @@ -6,7 +6,7 @@
* Changes since Nov. 2009 copyright &copy; by Benjamin Thomas and Omer Bar-or
*
* @file rmpixel.c
* @version $Id: rmpixel.c,v 1.6 2009/12/20 02:33:34 baror Exp $
* @version $Id: rmpixel.c,v 1.7 2009/12/21 10:34:58 baror Exp $
* @author Tim Hunter
******************************************************************************/

Expand Down Expand Up @@ -467,14 +467,15 @@ Pixel_from_color(VALUE class, VALUE name)
* Construct an RGB pixel.
*
* Ruby usage:
* - @verbatim Pixel#from_hsla(hue, saturation, lightness) @endverbatim
* - @verbatim Pixel#from_hsla(hue, saturation, lightness, alpha) @endverbatim
*
* Notes:
* - Default alpha is 1.0
* - 0 <= hue < 360
* - 0 <= saturation <= 1
* - 0 <= lightness <= 1
* - 0 <= alpha <= 1 (0 is transparent, 1 is opaque)
* - 0 <= hue < 360 OR "0%" <= hue < "100%"
* - 0 <= saturation <= 255 OR "0%" <= saturation <= "100%"
* - 0 <= lightness <= 255 OR "0%" <= lightness <= "100%"
* - 0 <= alpha <= 1 (0 is transparent, 1 is opaque) OR "0%" <= alpha <= "100%"
* - Replaces brain-dead Pixel_from_HSL.
*
* @param argc number of input arguments
Expand All @@ -496,12 +497,14 @@ Pixel_from_hsla(int argc, VALUE *argv, VALUE class)
switch (argc)
{
case 4:
a = NUM2DBL(argv[3]);
a = rm_percentage(argv[3],1.0);
alpha = MagickTrue;
case 3:
l = NUM2DBL(argv[2]);
s = NUM2DBL(argv[1]);
h = NUM2DBL(argv[0]);
// saturation and lightness are out of 255 in new ImageMagicks and
// out of 100 in old ImageMagicks. Compromise: always use %.
l = rm_percentage(argv[2],255.0);
s = rm_percentage(argv[1],255.0);
h = rm_percentage(argv[0],360.0);
break;
default:
rb_raise(rb_eArgError, "wrong number of arguments (%d for 3 or 4)", argc);
Expand All @@ -512,19 +515,28 @@ Pixel_from_hsla(int argc, VALUE *argv, VALUE class)
{
rb_raise(rb_eRangeError, "alpha %g out of range [0.0, 1.0]", a);
}
if (l < 0.0 || l > 100.0)
if (l < 0.0 || l > 255.0)
{
rb_raise(rb_eRangeError, "lightness %g out of range [0.0, 100.0]", l);
rb_raise(rb_eRangeError, "lightness %g out of range [0.0, 255.0]", l);
}
if (s < 0.0 || s > 100.0)
if (s < 0.0 || s > 255.0)
{
rb_raise(rb_eRangeError, "saturation %g out of range [0.0, 100.0]", s);
rb_raise(rb_eRangeError, "saturation %g out of range [0.0, 255.0]", s);
}
if (h < 0.0 || h >= 360.0)
{
rb_raise(rb_eRangeError, "hue %g out of range [0.0, 360.0)", h);
}

// Ugly way of checking for change in ImageMagick 6.5.6-5 to see whether
// saturation/lightness should be out of 255 or out of 100.
if(MagickLibVersion < 0x656 ||
(MagickLibVersion == 0x656 && strcmp(MagickLibSubversion,"-5") <= 0) )
{
s = s/2.55;
l = l/2.55;
}

memset(name, 0, sizeof(name));
if (alpha)
{
Expand Down Expand Up @@ -888,8 +900,8 @@ Pixel_to_hsla(VALUE self)

ConvertRGBToHSL(pixel->red, pixel->green, pixel->blue, &hue, &sat, &lum);
hue *= 360.0;
sat *= 100.0;
lum *= 100.0;
sat *= 255.0;
lum *= 255.0;

if (pixel->opacity == OpaqueOpacity)
{
Expand Down
13 changes: 7 additions & 6 deletions ext/RMagick/rmutil.c
Expand Up @@ -6,7 +6,7 @@
* Changes since Nov. 2009 copyright &copy; by Benjamin Thomas and Omer Bar-or
*
* @file rmutil.c
* @version $Id: rmutil.c,v 1.181 2009/12/20 02:33:34 baror Exp $
* @version $Id: rmutil.c,v 1.182 2009/12/21 10:34:58 baror Exp $
* @author Tim Hunter
******************************************************************************/

Expand Down Expand Up @@ -364,17 +364,18 @@ rescue_not_str(VALUE arg)


/**
* Return a double between 0.0 and 1.0, inclusive. If the argument is a number
* convert to a Float object, otherwise it's supposed to be a string in the form
* "NN%". Convert to a number and then to a Float.
* Return a double between 0.0 and max (the second argument), inclusive. If the
* argument is a number convert to a Float object, otherwise it's supposed to be
* a string in the form * "NN%". Convert to a number and then to a Float.
*
* No Ruby usage (internal function)
*
* @param arg the argument
* @param max the maximum allowed value
* @return a double
*/
double
rm_percentage(VALUE arg)
rm_percentage(VALUE arg, double max)
{
double pct;
long pct_long;
Expand All @@ -401,7 +402,7 @@ rm_percentage(VALUE arg)

if (*end == '%' && pct_long != 0)
{
pct = ((double)pct_long) / 100.0;
pct = (((double)pct_long) / 100.0) * max;
}
else
{
Expand Down
26 changes: 20 additions & 6 deletions test/Pixel.rb
Expand Up @@ -50,11 +50,12 @@ def test_fcmp
assert_raises(TypeError) { red.fcmp(blue, 'x') }
assert_raises(TypeError) { red.fcmp(blue, 10, 'x') }
end
=begin
changed in 6.5.6-6

def test_from_hsla
assert_nothing_raised { Magick::Pixel.from_hsla(127, 50, 50) }
assert_nothing_raised { Magick::Pixel.from_hsla(127, 50, 50, 0) }
assert_nothing_raised { Magick::Pixel.from_hsla(127, "50%", 50, 0) }
assert_nothing_raised { Magick::Pixel.from_hsla("10%", "50%", 50, 0) }
assert_raise(TypeError) { Magick::Pixel.from_hsla([], 50, 50, 0) }
assert_raise(TypeError) { Magick::Pixel.from_hsla(127, [], 50, 0) }
assert_raise(TypeError) { Magick::Pixel.from_hsla(127, 50, [], 0) }
Expand All @@ -73,16 +74,29 @@ def test_from_hsla
#hsla[0] = ((hsla[0] + 0.005) % 360.0) - 0.005
#hsla[1] = ((hsla[1] + 0.005) % 360.0) - 0.005
#hsla[2] = ((hsla[2] + 0.005) % 360.0) - 0.005
assert_in_delta(args[0], hsla[0], 0.01, "expected #{args.inspect} got #{hsla.inspect}")
assert_in_delta(args[1], hsla[1], 0.01, "expected #{args.inspect} got #{hsla.inspect}")
assert_in_delta(args[2], hsla[2], 0.01, "expected #{args.inspect} got #{hsla.inspect}")
assert_in_delta(args[0], hsla[0], 0.25, "expected #{args.inspect} got #{hsla.inspect}")
assert_in_delta(args[1], hsla[1], 0.25, "expected #{args.inspect} got #{hsla.inspect}")
assert_in_delta(args[2], hsla[2], 0.25, "expected #{args.inspect} got #{hsla.inspect}")
assert_in_delta(args[3], hsla[3], 0.005, "expected #{args.inspect} got #{hsla.inspect}")
end
end
end
end

# test percentages
args = ["20%","20%","20%","20%"]
args2 = [360.0/5,255.0/5,255.0/5,1.0/5]
px = Magick::Pixel.from_hsla(*args)
hsla = px.to_hsla
px2 = Magick::Pixel.from_hsla(*args2)
hsla2 = px2.to_hsla

assert_in_delta(hsla[0], hsla2[0], 0.25, "#{hsla.inspect} != #{hsla2.inspect} with args: #{args.inspect} and #{args2.inspect}")
assert_in_delta(hsla[1], hsla2[1], 0.25, "#{hsla.inspect} != #{hsla2.inspect} with args: #{args.inspect} and #{args2.inspect}")
assert_in_delta(hsla[2], hsla2[2], 0.25, "#{hsla.inspect} != #{hsla2.inspect} with args: #{args.inspect} and #{args2.inspect}")
assert_in_delta(hsla[3], hsla2[3], 0.005, "#{hsla.inspect} != #{hsla2.inspect} with args: #{args.inspect} and #{args2.inspect}")
end
=end

def test_to_color
assert_nothing_raised { @pixel.to_color(Magick::AllCompliance) }
assert_nothing_raised { @pixel.to_color(Magick::SVGCompliance) }
Expand Down

0 comments on commit fdea5b4

Please sign in to comment.