documentation error or confusing for -contrast-stretch

[fmw42]

The documentation for -contrast-stretch is either in error or at least confusing with regard to the white point value to use (when not expressed as percent). It says:

"While doing so black-out at most black-point pixels and white-out at most white-point pixels"

This is misleading as if one uses -contrast-stretch 10x10, one would think that it would black out 10 pixels on the low end and 10 pixels on the high end. But what it really does is black out total pixels minus 10 pixels on the hight end. To white out only 10 pixels, one must use white-point = total pixels minus 10. Thus the statement above should be changed to:

"While doing so black-out at most black-point pixels and white-out at most total pixels minus white-point pixels"



I did some tests:

I made a 1x1000 pixel gradient:

convert -size 1000x1 gradient: -rotate 90 grad1000.png
identify -verbose grad1000.png
shows 1 pixel at each of 1000 graylevels between 0 (black) and 65535 (white)

Then I issued the following to clip by percent count of 10% on each end:

convert grad1000.png -contrast-stretch 10,90% grad1000_cs10x90pct.png
identify -verbose grad1000_cs10x90pct.png
...
Histogram:
101: ( 0, 0, 0) #000000000000 black
101: (65535,65535,65535) #FFFFFFFFFFFF white
Thus 10% or 100 pixels were clipped to black and 10% or 100 pixels were clipped to white in addition to the 1 already at each end, so a value of 101 counts.

Then I issued the following to clip by count of 10 on each end:

convert grad1000.png -contrast-stretch 10,990 grad1000_cs10x990t.png
identify -verbose grad1000_cs10x990.png
...
Histogram:
11: ( 0, 0, 0) #000000000000 black
11: (65535,65535,65535) #FFFFFFFFFFFF white
Thus 10 pixels were clipped to black and 10 pixels were clipped to white in addition to the 1 already there, so a value of 11 counts on each end.

Thus the correct use of -contrast-stretch is by count from 0 at the black/low end and (totalpixels - count) from white/high end for non-percent. And percentcount from the black/low end and 100-percentcount) from the white/high end.

Thus the documentation is confusing if not wrong.

An alternate and correct way to express it is as follows:
black-point value = either number of pixels to black out or percent pixels to black out at the low end of the histogram.
white-point value = either (total pixels minus number of pixels to white out) or (100 - percent pixels to white out) at the high end of the histogram

[magick]

We believe you have write privileges to the ImageMagick documentation on magick.imagemagick.org. Feel free to correct or enhance the documentation as needed. Recall it takes 24-48 hours for any changes you make to mirror world-wide.

[fmw42]

We believe you have write privileges to the ImageMagick documentation on magick.imagemagick.org. Feel free to correct or enhance the documentation as needed. Recall it takes 24-48 hours for any changes you make to mirror world-wide.

Thanks. I will do that. Just wanted to be sure I was correct about this and had your permission.

P.S. I have now made the change.

[agss]

Certainly correcting the documentation to describe the actual semantics is an improvement but I'd argue that having -contrast-stretch black-pointxwhite-point white-out at most white-point pixels is far more useful than having it white-out at most (total pixels - white-point) pixels, even if the latter is more consistent with the percentage form. Unless there's some way of conveniently expressing (total pixels - white-point) that I don't know about, which is certainly possible, having to know how many pixels there are in an image to be able to express that I'm willing to white-out at most N pixels isn't terribly convenient.

[fmw42]

Certainly correcting the documentation to describe the actual semantics is an improvement but I'd argue that having -contrast-stretch black-pointxwhite-point white-out at most white-point pixels is far more useful than having it white-out at most (total pixels - white-point) pixels, even if the latter is more consistent with the percentage form. Unless there's some way of conveniently expressing (total pixels - white-point) that I don't know about, which is certainly possible, having to know how many pixels there are in an image to be able to express that I'm willing to white-out at most N pixels isn't terribly convenient.

I would agree with you as most of the other major image processing systems clip (usually by percent) specified from each end, e.g. Photoshop, GIMP etc. And the change is not likely hard to do. But I am afraid that making such a change could cause lots of backward compatibility issues. (It would also affect -normalize, too). Unfortunately, this is not under my control. All I was able to do was change the documentation to reflect the current situation.

But you can easily get the number of pixels in an image by:

totpix=`convert <image> -format "%[fx:w*h]" info:`

or

loval=10
hival=10
cliplow=$loval
cliphigh=`convert <image> -format "%[fx:w*h-$hival]" info:`
convert <image> -contrast-stretch ${cliplow}x${cliphigh} <result>

You probably could also add -ping to make it faster.

Just curious though -- I cannot think of a situation where I would want to do clip by count and not by percent. Can you suggest why you want to do it by count -- what is the situation/example that you have where that would be preferable?

[magick]

Making changes to the way the command behaves is up to Anthony. Anthony what is your opinion? Update the docs or modify the option behavior?

[agss]

I would agree with you as most of the other major image processing systems clip (usually by percent) specified from each end, e.g. Photoshop, GIMP etc. And the change is not likely hard to do.

Indeed, that was the patch I provided in my "-context-stretch problem?" thread. (I think at least. :^)

But I am afraid that making such a change could cause lots of backward compatibility issues.

Yes, I realize that the change isn't backwards compatible so it's a judgement call that someone else other than a relatively new user (ie. me) will have to make. However, given the fact that the documentation had it wrong, a change isn't going to break documented behaviour. (Well, until you fixed the docs that is, but that's too recent to count. :^)

Ideally (IMO) the percentage form would also work as you say, clipping the specified percentage of pixels at the top end rather than the percentage subtracted from 100, but changing that would break documented behaviour.

(It would also affect -normalize, too).

Not sure why since -normalize is documented as working with percentages. The patch I provided only affected non-percentage arguments to -contrast-stretch.

But you can easily get the number of pixels in an image by:

Code: Select all

totpix=`convert <image> -format "%[fx:w*h]" info:`

Sure, but having to compute the number of pixels left at the top end is a good deal more verbose than just being able to specify the number.

Just curious though -- I cannot think of a situation where I would want to do clip by count and not by percent. Can you suggest why you want to do it by count -- what is the situation/example that you have where that would be preferable?

I'm usually not aware of the exact number of pixels in my image when I'm adjusting it. When I found out about -contrast-stretch for the first time and read the documentation I thought "nice, I'll give it a go and clip a small number of pixels at each end" and then proceeded to be surprised by the results of -contrast-stretch 1000x1000. Turned out that to use the non-percent form in the way I'd intended then I did have to know the exact number of pixels, and then the percent form does indeed becomes preferable. If this was changed so that the non-percent form clipped at most white-point pixels rather than total pixels - white-point then I wouldn't see anything inherently more useful with the percent form.

[agss]

However, given the fact that the documentation had it wrong, a change isn't going to break documented behaviour.

More to the point, the change would make a non-percent -contrast-stretch behave as was documented.

[fmw42]

However, given the fact that the documentation had it wrong, a change isn't going to break documented behaviour.

More to the point, the change would make a non-percent -contrast-stretch behave as was documented.

The real issue is that the function needs to behave the same whether using count or percent count. IM has implemented it so that both need to use total pixels (or 100%) minus the white-point value for the higher end to get something reasonable. But this is not what most other applications do. They do not require you to have to subtract what you want to clip from the total pixels (or 100%) . That is they clip 10 pixels on each end of the histogram or clip 10% on each end of the histogram. This is the way it ought to work --- using the direct amounts to be clipped from each end. However, IM has not implemented it that way. Thus to change it, both count and percent count white-points need to change. This can be done keeping the algorithm as it is, but just internally changing white-point to (total pixels minus white point) or (100% minus white-point) as the code can be made to find the total pixels easily. Thus the user could specify clip values as small numbers for both black-point and white-point. Then internally the white point would be converted to its "complement" and used in the existing algorithm.

The issue is thus really about backward compatibility. Everyone out there who is currently using it in scripts would have to modify their script. The others would have to learn the new behaviour, but this is less an issue. So the folks at IM need to decide if such a change and backward compatibility is worth the effort.

I personally have some scripts and those would need to have built in traps for the IM version where such a change in behaviour would be made so that it could handle it both ways for newer as well as older IM versions. But if I am the only person who has scripts using this, I could live with the changes, if that was decided.

[agss]

The real issue is that the function needs to behave the same whether using count or percent count. IM has implemented it so that both need to use total pixels (or 100%) minus the white-point value for the higher end to get something reasonable.

I'm all for consistency but I don't really see why the behaviour has to be the same for the percentage and non-percentage form. It's just a matter of interpreting white-point differently depending on whether or not it's a percentage. (Which is what my patch, which only changed the non-percentage behaviour, did.)

Given that the documentation in fact did attach a different interpretation to white-point in the percentage and non-percentage forms, it's just a matter of deciding whether the documention of the implemented behaviour is "right". IMO the documented behaviour is the more useful since I'm unlikely to want to write, for example, -contrast-stretch 1000x465987 to clip at most 1000 pixels of an image containing 466987 pixels. Writing 1000x1000 (or just 1000) for this seems more usable IMO.

However, IM has not implemented it that way. Thus to change it, both count and percent count white-points need to change. This can be done keeping the algorithm as it is, but just internally changing white-point to (total pixels minus white point) or (100% minus white-point) as the code can be made to find the total pixels easily.

Maybe I missed something when I looked at the code (which is entirely possible) but what I saw was that the it was simply the argument parsing that interpreted white-point. A percentage white-point is converted to a total number of pixels and then a function is called to do the heavy lifting. That is, once you got past the argument parsing it's all pixel counts.

I personally have some scripts and those would need to have built in traps for the IM version where such a change in behaviour would be made so that it could handle it both ways for newer as well as older IM versions. But if I am the only person who has scripts using this, I could live with the changes, if that was decided.

That's assuming the percentage form of -contrast-stretch is changed (I assume, since I got the impression that you didn't use the non-percentage form :^), which I'm not advocating. I'm just advocating that the interpretation of a non-percentage white-point argument to -contrast-stretch be fixed to match the documentation rather than the documentation being changed to match the implementation.

[fmw42]

Maybe I missed something when I looked at the code (which is entirely possible) but what I saw was that the it was simply the argument parsing that interpreted white-point. A percentage white-point is converted to a total number of pixels and then a function is called to do the heavy lifting. That is, once you got past the argument parsing it's all pixel counts.

That was my point. The changes have only to do with argument parsing. The code can be made to find the total pixels and subtract white-point or use 100%-white-point and core algorithm does not have to be changed.

I'm all for consistency but I don't really see why the behaviour has to be the same for the percentage and non-percentage form. It's just a matter of interpreting white-point differently depending on whether or not it's a percentage. (Which is what my patch, which only changed the non-percentage behaviour, did.)

I believe strongly in consistency!!!!

I believe in making the documentation agree with the code or changing both, not changing the code in one part to agree with inconsistent documentation

As a user of the percent form, if the count form is changed, I think the percent form should be changed as well; otherwise, I think it is too confusing for the users. That way you don't need to subtract from 100, you just use the value provided. Furthermore it then conforms to all the other applications that do clipping, such as Photoshop, GIMP, etc

But again this is a matter of backward compatibility and a decision for the IM folks to make.

By the way, I am currently working on a script to do just that --- namely, allow the user to specify the amounts (counts or percents) to clip from either end without having to subtract from total pixels or 100%. It will also allow one to do the contrast stretch on either 1) the intensity, 2) each of the red,green,blue channels separately, 3) each of saturation,brightness channels separately or 4) each of the saturation/lightness channels separately. Furthermore, if the count or percent is exactly zero on both ends, it finds the true min and max graylevel rather than min and max histogram bins and stretches using those values via -level. This script should be available in a day or two.

[agss]

That was my point. The changes have only to do with argument parsing. The code can be made to find the total pixels and subtract white-point or use 100%-white-point and core algorithm does not have to be changed.

Ok, then we agree. Sorry, I misunderstood you.

I believe strongly in consistency!!!!

So do I, but not for it's own sake. I accept that it's probably impossible to change the percentage form since that will break correctly documented behaviour but if the choice is between consistency and a more useful non-percentage for (IMO) then I can live with a bit of inconsistency. (That said, I'm not going to fall over if the behaviour remains as is. I'll just write a script or use yours. :^)

As a user of the percent form, if the count form is changed, I think the percent form should be changed as well; otherwise, I think it is too confusing for the users. That way you don't need to subtract from 100, you just use the value provided. Furthermore it then conforms to all the other applications that do clipping, such as Photoshop, GIMP, etc

But again this is a matter of backward compatibility and a decision for the IM folks to make.

I agree. It's certainly their call.

[fmw42]

That was my point. The changes have only to do with argument parsing. The code can be made to find the total pixels and subtract white-point or use 100%-white-point and core algorithm does not have to be changed.

Ok, then we agree. Sorry, I misunderstood you.

I believe strongly in consistency!!!!

So do I, but not for it's own sake. I accept that it's probably impossible to change the percentage form since that will break correctly documented behaviour but if the choice is between consistency and a more useful non-percentage for (IMO) then I can live with a bit of inconsistency. (That said, I'm not going to fall over if the behaviour remains as is. I'll just write a script or use yours. :^)

As a user of the percent form, if the count form is changed, I think the percent form should be changed as well; otherwise, I think it is too confusing for the users. That way you don't need to subtract from 100, you just use the value provided. Furthermore it then conforms to all the other applications that do clipping, such as Photoshop, GIMP, etc

But again this is a matter of backward compatibility and a decision for the IM folks to make.

I agree. It's certainly their call.

Well, for the most part we both agree. So I guess we have to leave the decision up to the IM folks. However, correct me if I am wrong, but did you write a code update for the count part that you were offering to IM? If so, and to be consistent about the change, would you be willing to update it for the percent format as well. That way, both count and percent formats are consistent in their use, both use the actual user supplied clip values, the user does not have to do any calculations and it is consistent with the way the other apps such as Photoshop, GIMP etc use it. If you offer to do that (and if I have not misunderstood your statement about your own code change), then that might sway the IM folks to change the behavior despite the backward compatibility issue. Perhaps I am the only one who has scripts that would require changing and I am willing to put a compatibility trap into my scripts. I have certainly done so for other scripts when behavior was changed (like perspective control point formats) or when improvements were made such as new string formats.

[agss]

Well, for the most part we both agree. So I guess we have to leave the decision up to the IM folks. However, correct me if I am wrong, but did you write a code update for the count part that you were offering to IM?

Yes, at the beginning of my thread entitled "-contrast-stretch problem?".

If so, and to be consistent about the change, would you be willing to update it for the percent format as well. That way, both count and percent formats are consistent in their use, both use the actual user supplied clip values, the user does not have to do any calculations and it is consistent with the way the other apps such as Photoshop, GIMP etc use it.

Sure, here's a patch on 6.4.4-8.

Code: Select all

*** wand/mogrify.c.orig	Fri Oct 10 03:26:57 2008
--- wand/mogrify.c	Fri Oct 17 01:28:33 2008
***************
*** 915,931 ****
              (void) SyncImageSettings(image_info,*image);
              flags=ParseGeometry(argv[i+1],&geometry_info);
              black_point=geometry_info.rho;
!             white_point=(MagickRealType) (*image)->columns*(*image)->rows;
!             if ((flags & SigmaValue) != 0)
!               white_point=geometry_info.sigma;
              if ((flags & PercentValue) != 0)
                {
                  black_point*=(double) (*image)->columns*(*image)->rows/100.0;
                  white_point*=(double) (*image)->columns*(*image)->rows/100.0;
                }
!             if ((flags & SigmaValue) == 0)
!               white_point=(MagickRealType) (*image)->columns*(*image)->rows-
!                 black_point;
              (void) ContrastStretchImageChannel(*image,channel,black_point,
                white_point);
              InheritException(exception,&(*image)->exception);
--- 915,929 ----
              (void) SyncImageSettings(image_info,*image);
              flags=ParseGeometry(argv[i+1],&geometry_info);
              black_point=geometry_info.rho;
!             white_point=(flags & SigmaValue) != 0 ?
!               geometry_info.sigma : black_point;
              if ((flags & PercentValue) != 0)
                {
                  black_point*=(double) (*image)->columns*(*image)->rows/100.0;
                  white_point*=(double) (*image)->columns*(*image)->rows/100.0;
                }
!             white_point=(MagickRealType) (*image)->columns*(*image)->rows-
!               white_point;
              (void) ContrastStretchImageChannel(*image,channel,black_point,
                white_point);
              InheritException(exception,&(*image)->exception);

[fmw42]

Well, for the most part we both agree. So I guess we have to leave the decision up to the IM folks. However, correct me if I am wrong, but did you write a code update for the count part that you were offering to IM?

Yes, at the beginning of my thread entitled "-contrast-stretch problem?".

If so, and to be consistent about the change, would you be willing to update it for the percent format as well. That way, both count and percent formats are consistent in their use, both use the actual user supplied clip values, the user does not have to do any calculations and it is consistent with the way the other apps such as Photoshop, GIMP etc use it.

Sure, here's a patch on 6.4.4-8.

Code: Select all

*** wand/mogrify.c.orig	Fri Oct 10 03:26:57 2008
--- wand/mogrify.c	Fri Oct 17 01:28:33 2008
***************
*** 915,931 ****
              (void) SyncImageSettings(image_info,*image);
              flags=ParseGeometry(argv[i+1],&geometry_info);
              black_point=geometry_info.rho;
!             white_point=(MagickRealType) (*image)->columns*(*image)->rows;
!             if ((flags & SigmaValue) != 0)
!               white_point=geometry_info.sigma;
              if ((flags & PercentValue) != 0)
                {
                  black_point*=(double) (*image)->columns*(*image)->rows/100.0;
                  white_point*=(double) (*image)->columns*(*image)->rows/100.0;
                }
!             if ((flags & SigmaValue) == 0)
!               white_point=(MagickRealType) (*image)->columns*(*image)->rows-
!                 black_point;
              (void) ContrastStretchImageChannel(*image,channel,black_point,
                white_point);
              InheritException(exception,&(*image)->exception);
--- 915,929 ----
              (void) SyncImageSettings(image_info,*image);
              flags=ParseGeometry(argv[i+1],&geometry_info);
              black_point=geometry_info.rho;
!             white_point=(flags & SigmaValue) != 0 ?
!               geometry_info.sigma : black_point;
              if ((flags & PercentValue) != 0)
                {
                  black_point*=(double) (*image)->columns*(*image)->rows/100.0;
                  white_point*=(double) (*image)->columns*(*image)->rows/100.0;
                }
!             white_point=(MagickRealType) (*image)->columns*(*image)->rows-
!               white_point;
              (void) ContrastStretchImageChannel(*image,channel,black_point,
                white_point);
              InheritException(exception,&(*image)->exception);

Great. I will send a private message to Anthony to get him in on this as he has either not seen or not yet offered his opinion to this thread. Lets get his opinion about the change and backward compatibility.