Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

brightness-contrast unintuitive #6079

Closed
Harry79 opened this issue Feb 18, 2023 · 21 comments
Closed

brightness-contrast unintuitive #6079

Harry79 opened this issue Feb 18, 2023 · 21 comments

Comments

@Harry79
Copy link

Harry79 commented Feb 18, 2023

ImageMagick version

7.0.10-24

Operating system

Windows

Operating system, version and so on

11

Description

ImageMagick has the brightness-contrast command line option which is quite unintuitive for non-obvious reasons.

Had to look up the implementation to find how it actually is producing the results it outputs.

MagickCore/enhance.c in BrightnessContrastImage:

slope=tan((double) (MagickPI*(alpha/100.0+1.0)/4.0));

I would propose the following more intuitive definition by attaching it to the histogram:

  • +50 would have slope 2 so all input values would be mapped to half of the output value range.
  • -50 would do the opposite, half the input values would be mapped to the full output value range.

more formally:

slope = contrast/100+1 if contrast < 0
slope = 100/(100-contrast) else

Steps to Reproduce

magick convert test.ppm -brightness-contrast 6x-20 out.ppm

Images

No response

@urban-warrior
Copy link
Member

Both algorithms return similar values 6x-20 returns 0.8 for your algorithm, for the existing, 0.727. For 20x-80, we get 0.2 vs. 0.16. For 30x-90, 0.1 vs. 0.08. Let's get @fmw42 to comment, our resident image processing expert.

@fmw42
Copy link

fmw42 commented Feb 19, 2023

The current algorithm is designed to map contrast=-100 (actually -99.9) to slope=0, contrast=0 to slope=1 and contact=100 (actually 99.5) to slope 100 (or 99.9) to slope 500. The slope was designed to pivot about mid-gray when the brightness=0 and contrast adjusted. So the intercept changes rather than having the intercept at 0,0.

pi=`echo "scale=10; 4*a(1)" | bc -l`
arg=`echo "scale=5; $pi * ((($con * $con) / 20000) + (3 * $con / 200)) / 4" | bc`
slope=`echo "scale=3; 1 + (s($arg) / c($arg))" | bc -l`
echo "con=$con; slope=$slope"

con=-99.9; slope=0
con=0; slope=1.000
con=99.5; slope=100.900
con=99.9; slope=501.000

Please show your expression for computing the intercept. One needs to define both the slope and intercept and how they relate to brightness and contrast.

@Harry79
Copy link
Author

Harry79 commented Feb 19, 2023

I did some calculations, found some non-working solutions, and after some math ended up where you already have been.

intercept=brightness/100.0+((100-brightness)/200.0)*(1.0-slope);

So, your current intercept calculation is the best I could find. In essence it is pivoting about the gray value of the defined brightness. This maps the input brightness range values to meaningful and distinct transformations.

@fmw42
Copy link

fmw42 commented Feb 19, 2023

Perhaps you should write a script and show some results or show the transformation (output vs input) as you change brightness and contrast. See http://www.fmwconcepts.com/imagemagick/bcimage/index.php which was my prototype for -brightness-contrast.

@fmw42
Copy link

fmw42 commented Feb 20, 2023

When I get a chance, I will try to script them both to see the difference in the effects. Sorry, but I am busy with a paying contract right now and cannot spend any time on this at the moment.

@fmw42
Copy link

fmw42 commented Mar 11, 2023

I have programmed this alternate (as bcimagex) and find the following comparison to my original script (bcimage)

Brightness Comparison:

bcimage -g 50,0 lena.png x.png
slope=1.000
intcp=.500
Break Points = 0,50 50,100 100,100

bcimagex -g 50,0 lena.png x.png
slope=1
intcp=0.5
Break Points = 0,50 50,100 100,100

bcimage -g 100,0 lena.png x.png
slope=1.000
intcp=1.000
Break Points = 0,100 0,100 100,100

bcimagex -g 100,0 lena.png x.png
slope=1
intcp=1
Break Points = 0,100 0,100 100,100

bcimage -g 25,0 lena.png x.png
slope=1.000
intcp=.250
Break Points = 0,25 75,100 100,100

bcimagex -g 25,0 lena.png x.png
slope=1
intcp=0.25
Break Points = 0,25 75,100 100,100

Contrast Comparison:

bcimagex -g 0,50 lena.png x.png
slope=2
intcp=-0.5
Break Points = 0,0 25,0 75,100 100,100

bcimage -g 0,50 lena.png x.png
slope=1.820
intcp=-.410
Break Points = 0,0 22,0 77,100 100,100

So at bri=0 and con=+50, the two produce very similar break points

bcimagex -g 0,-50 lena.png x.png
slope=0.5
intcp=0.25
Break Points = 0,25 100,75

bcimage -g 0,-50 lena.png x.png
slope=.466
intcp=.267
Break Points = 0,26 100,73

So at bri=0 and con=-50, the two produce very similar break points

bcimagex -g 0,100 lena.png x.png
slope=1e+14
intcp=-5e+13
(standard_in) 1: parse error
expr: syntax error

bcimage -g 0,100 lena.png x.png
slope=501.000
intcp=-250.000
Break Points = 0,0 49,0 50,100 100,100

At bri=0 and con=100, the new algorithm blows up with huge values for slope and intercept and cannot compute the break points, whereas my original algorithm was designed to stay finite up to +-100

However, if back off by 1 to con=99, both algorithms produce finite results.

bcimagex -g 0,99 lena.png x.png
slope=100
intcp=-49.5
Break Points = 0,0 49,0 50,100 100,100

bcimage -g 0,99 lena.png x.png
slope=50.950
intcp=-24.975
Break Points = 0,0 49,0 50,100 100,100

At bri=0 and con=99, the new algorithm slope is about twice my original algorithm. So the new algorithm slope rises faster above con=50. But a slope of 100 and slope of 50 are both very vertical. And the break points are identical.

So since brightness is the same in both algorithms and for contrast at con=+-50 produces very similar results (which was your main requirement), I am not inclined to recommend any change to the current -brightness-contrast.

@fmw42
Copy link

fmw42 commented Apr 6, 2023

The new proposed algorithm does have some nice features. Brightness values produce the same results and Contrast values produce similar results. So I am not opposed to this change. However, I leave it up to the IM developers if they have time and interest in changing the code and if it is not going to cause issues with backward compatibity.

@dlemstra
Copy link
Member

dlemstra commented Apr 7, 2023

Maybe we could introduce this under a flag?

@fmw42
Copy link

fmw42 commented Apr 7, 2023

Dirk, do you mean a -define?

@dlemstra
Copy link
Member

dlemstra commented Apr 7, 2023

Yeah that is what I meant. Not sure if that is a good idea or how we should call that define.

@fmw42
Copy link

fmw42 commented Apr 7, 2023

Both algorithms produce very similar results. I am not sure it is worth having options for both. I think we should just decide to either switch or keep the old one.

@dlemstra
Copy link
Member

dlemstra commented Apr 7, 2023

Maybe you and @urban-warrior can talk about this and decide what to do?

@urban-warrior
Copy link
Member

Give us a few days, we'll export a patch soon.

@fmw42
Copy link

fmw42 commented Apr 7, 2023

Do you need me to send you some equations or my script for the new method?

@urban-warrior
Copy link
Member

Per the usual @fmw42, forward the equations and use cases, we'll implement, and you will review and approve before we release the change.

@fmw42
Copy link

fmw42 commented Apr 8, 2023

@urban-warrior

I am getting some differences that are small, but noticeable, between my script and the IM beta -brightness-contrast function. For example at brightness=0 and contrast=50. (Alternate viewing these with animate to see the differences). They might be precision between my script and the compiled code. But check my formulae below to be sure.

Commands for the profiles below:

bcimagex-Harald -g 0,50 grad.png bcimagex-Harald_0x50.png
profile -r 1 -w 506 -l on -f 1 bcimagex-Harald_0x50.png bcimagex-Harald_0x50_profile.gif 
im7beta magick grad.png -brightness-contrast 0,50 bc_0x50.png
profile -r 1 -w 506 -l on -f 1 bc_0x50.png bc_0x50_profile.gif 

IM beta -brightness-contrast:

bc_0x50_profile

My script:

bcimagex-Harald_0x50_profile

See all profiles of a gradient processed for values of 25, 50, 75 for either brightness or contrast (bri and con in the script). bc is the IM beta function and bcimagex-Harald is my script version

bc.zip

Here is my script, bcimagex-Harald and profile

bcimagex-Harald.zip

profile.zip

Here are the slope and intercept code (from input bri and con values) from my script where bri and con vary between -100 and +100. Code converted from Harald formulae above

ctest=`convert xc: -format "%[fx:$con>=0?1:0]" info:`
if [ $ctest -eq 1 ]; then
	slope=`convert xc: -format "%[fx:100/(100-$con)]" info:`
else
	slope=`convert xc: -format "%[fx:$con/100+1]" info:`
fi

# v = in + brightness/100  ---  Note from Harald: apply the brightness first, it is just an offset.
# out = (v-0.5)*slope+0.5  --- Note from Harald: then apply the contrast, using the mid-gray pivot to get the output pixel value
# out = a * in + b  (a=slope, b=intercept)
intcp=`convert xc: -format "%[fx:($bri/100-0.5)*$slope+0.5]" info:`
echo "slope=$slope"
echo "intcp=$intcp"

@fmw42
Copy link

fmw42 commented Apr 8, 2023

Can you add a define to list to the terminal the slope and intercept values.

-define brightness-contrast:verbose=on

or

-define brightness-contrast:slope-intercept=on

whichever makes most sense to you.

@fmw42
Copy link

fmw42 commented Apr 8, 2023

I am still seeing differences in the profiles between my script and the beta -brightness-contrast. Here are my tests:

Brightness 50, Contrast 0

bcimagex-Harald 50,0 grad.png bcimagex-Harald_50x0.png
profile -r 1 -w 256 -l on -f 1 bcimagex-Harald_50x0.png bcimagex-Harald_50x0_profile.gif 
im7beta magick grad.png -brightness-contrast 50,0 bc_50x0.png
profile -r 1 -w 256 -l on -f 1 bc_50x0.png bc_50x0_profile.gif 

Brightness 0, Contrast 50

bcimagex-Harald 0,50 grad.png bcimagex-Harald_0x50.png
profile -r 1 -w 506 -l on -f 1 bcimagex-Harald_0x50.png bcimagex-Harald_0x50_profile.gif 
im7beta magick grad.png -brightness-contrast 0,50 bc_0x50.png
profile -r 1 -w 506 -l on -f 1 bc_0x50.png bc_0x50_profile.gif 

Animate the pairs of profiles in the followinging:

bc2.zip

@fmw42
Copy link

fmw42 commented Apr 8, 2023

If you add the -define, then I can just compare slope and intercepts. The new formula should have slope exactly equal to 2.0 when contrast=50 and brightness=0.

@urban-warrior
Copy link
Member

We ran a bunch of combinations of brightness and contrast between magick and bcimagex and both produced the same slope and intercept to the 6th decimal place confirming the slope and intercept are computed properly. We pass the slope and intercept to FunctionImage() with a PolynomialFunction method. What do you use to convert the input image to the output image for a given slope and intercept?

@fmw42
Copy link

fmw42 commented Apr 8, 2023

In my code I convert slope-intercept to break points and then to a clut. I suppose there could be differences dues to extra processing in my script for the break points and then to the clut.

In the profiles above, I plot values for each pixel (along the first row) for the output. So the same code for the both IM beta and bcimagex-Harald script. It is my profile script.

As long as the slope and intercepts are the same between the -brightness-contrast and the script and for brightness=0 and contrast=50, the slope is exactly 2.0. Then your code is fine. That was why I wanted to have the define to use to compare slope and intercepts between the two codes.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Apr 23, 2023
7.1.1-8 -  2023-04-22

Fixed

    Added checks to make sure all bytes were read to resolve #6267. #6267

Commits

    beta release 35ec38f
    Treat warnings as errors in the MacOS build and enabled more warnings for that build. 02b2aa6
    Corrected compare. 35505ca
    Not longer export methods that are not used in other parts of the library. 01251e5
    No longer call ParseMetaGeometry twice when we don't add a thumbnail. 6a94dd8
    Fix typo that caused a division by zero in #6263. 78347b5
    don't reduct 3 to 1 channel gray if meta channels are present a8f6186
    The depth must be specified when reading a map image. 2d6e2e9
    validate pixel offset 90e067c
    validate pixel offset d92cb0e
    release 920f792

7.1.1-7 - 2023-04-16

Merged

    PixelChannel: Add invalid channel field #6230

Commits

    beta release 3bd5ce1
    account for extra samples 9a9896f
    The quantum extent should also including the pad. b019133
    Another improvement of calculating the size of the extent. 3151fd8
    The padding is per pixel. dc0556c
    Cosmetic. ad36935
    Use the new API when available. 8b12fc1
    don't cut off letters (ImageMagick/ImageMagick#6221) fcf2674
    Moved static declaration. 8eaadcf
    Removed unused lt_dlexit define. d454d11
    Removed unused NTSetSearchPath method. 43ea02a
    Windows doesn't need a call to lt_dlinit. 48e9207
    There is no need to call WSAStartup because we will always have version 2.2 on the supported platforms. 24990f7
    Moved linking of lib inside other check. b13fabd
    More cleanup of header files. d048972
    Silence warnings for when the distribute-cache is not supported. 13841a1
    support --enable-dpc configure option fc7bb1d
    Fixed build error. 178f677
    theoretically a more intuitive brighness contrast algorithm (ImageMagick/ImageMagick#6079) cdc73fb
    revert format hint (ImageMagick/ImageMagick#6242) 9e1492e
    Use the new MAGICKCORE_DPC_SUPPORT flag. 7c8a486
    Removed define that always had the same value. 8e5fbea
    We only need to link ws2_32.lib on Windows when we have enabled DPC support. 1c4f052
    identify correct format ef12f38
    Restored the call to WSAStartup because we do need to do this when we use winsock2. 1bc9391
    Correct define check. 610a8a5
    Corrected checks that determine if we actually are able to support dpc. 70cd76b
    revert 723b63a
    correct slope/intercept 405c61a
    correct intercept b29bcd9
    eliminate compiler warnings (ImageMagick/ImageMagick#6230) fba0e38
    cosmetic 1fe2162
    eliminate compiler warning 3b84c79
    release e4b3733
    beta release 3b5b4a1
    if the image type is explicit, use the file extension if possible (ImageMagick/ImageMagick#6242) 67aa0ed
    Removed unused return value. 0d5e3a0
    Use version number instead. 1bc3bdc
    Silence warning by casting to size_t instead where negative values will overflow in a large value. 8def9df
    Silenced warnings. 7285688
    Silenced warnings. 864e6db
    Added missing break. 58b6568
    Moved devcontainer into security folder. 386f3b8
    Reverted patch and silenced warning. 4602557
    Added name to the container. f9b5142
    Added missing break. 9754705
    Silenced MagickCore warnings. 427e443
    Silenced coders warnings. 8f78bf4
    Corrected devcontainer name. ecc72e5
    Unreference in else statement instead. 69da709
    Removed unused argument. 890b4c7
    Removed unused return values. 1309276
    Only define GetICCProperty when build with LCMS delegate. bacfcad
    Silenced warning. a489b2a
    Silenced warnings in MagickWand. bf1e065
    Added devcontainer for local development. 18fd137
    Use define because the fallthrough attribute is only supported by gcc. 312aaf9
    Added Dockerfile to the editorconfig. 38ecac1
    Use different notation to fix the macos build. 35152a1
    Added devcontainer that uses the clang compiler. fc6f751
    Changed build to treat all warnings as errors and check for more warnings. 2a003f8
    Silenced warning. f85c832
    Change code to make it compile with -Wall -Wextra -Werror b164646
    Silenced warning. cdba683
    Ignore unused-function and builtin-declaration-mismatch warnings to work around autoconf issues. f311596
    Clang needs different flags to fix the build. 3e49a05
    Also build with clang in the daily build. 717d8d7
    Include compiler in the title. 50b4062
    Silence warnings that occur when freetype is enabled. 3f9cfbd
    Silenced more warnings. 7c809d7
    Silence the warning differently. feaa675
    elimiate compiler warnings f49eeca
    eliminate compiler warnings c40db4e
    eliminate compiler warnings 9d85754
    eliminate compiler warning 7157e1a
    expand channel map by 1 cc97c3a
    initialize max channels + 1 bdd4599
    add additional checks for casting double to size_t f7b5682
    cosmetic 77f6713
    identify z component of chromaticity fecfed4
    Refactor code to make it more readable. 9783994
    Corrected compare. c13cada
    Also skip writing the exif/tiff resolution properties when the pHYs chunk is written. d4f233b
    improved range checking 4daec2d
    cosmetic. 8066117
    Removed unused return value. d3cf508
    consistent method to check for alpha channel 242e940
    Correct comment. 43aa790
    Added method to update the density and orientation in the xmp profile. fc4f67b
    Corrected value for tiff:ResolutionUnit. c9f17dc
    Cosmetic. 589856f
    Removed debug printf statement... fecd253
    round crop width properly adda986
    Install more dependencies for the macOS build. 4a52f67
    Silence warning. 30f3676
    release 21d049b
@dlemstra dlemstra closed this as completed Jul 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants