p5.js icon indicating copy to clipboard operation
p5.js copied to clipboard

saturation() issues

Open joemckay5 opened this issue 5 years ago • 12 comments

Hi Folks. I'm experiencing a problem with saturation that was first outlined in the now-closed ticket #1620. The saturation value of a color is not being returned correctly. Here's a sketch demonstrating the bug. https://editor.p5js.org/Joemckay/sketches/2NMSFqN7u I tested it with a number of versions of p5, including the 1.0.0 build. (I also tested my code in Processing and it's working there).

joemckay5 avatar Jul 01 '20 18:07 joemckay5

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, be sure to follow the issue template if you haven't already.

welcome[bot] avatar Jul 01 '20 18:07 welcome[bot]

I don't quite understand what I'm looking for in the sketch, can you give a brief description of what is the expected behaviour and the actual behaviour? It would be helpful to have the browser and OS versions as well.

limzykenneth avatar Jul 15 '20 15:07 limzykenneth

Hi limzykenneth,
The green text under the mouse is returning the hue, saturation, and brightness values of the color at mouseX, mouseY. Start with the small rect in the sketch that was made with the color hsb(50, 50, 50). If you mouseover that box the value of the returned saturation value is 33, not 50 as it should be. The brightness and Hue are correct (or really close).
I created the gradient so you can "track" how the returned saturation value deviates across the color picker (it begins at 0 and ends at 100 but is not evenly distributed between the two, and that deviation is different depending on the brightness). This issue occurs in both chrome and safari on my MacBook pro. Below is the same code in processing, where the saturation() returns the correct value. void setup() { size(400, 400); noStroke(); }

void draw() { background(220); colorMode(HSB, 300, 100, 100); for(int i = 0; i < 100 * 100; i ++){ fill(50, i % 100, i / 100); rect(i % 100 * 4, int(i/100) * 4, 4, 4); } colorMode(HSB); fill(50, 50, 50); rect (20, 80, 100, 40);

colorMode(RGB); fill(1); text("hsb(50, 50, 50)", 22, 90); color colorTemp = get(mouseX, mouseY);
float h = hue(colorTemp); float s = saturation(colorTemp); float b = brightness(colorTemp); fill(colorTemp); rect (20, 120, 100, 40); fill(2, 200, 80); text("h " +int(h) +"\ns " +int(s) + "\nb " + +int(b), mouseX, mouseY + 40); }

joemckay5 avatar Jul 15 '20 15:07 joemckay5

Hi @joemckay5! This example might be a bit complicated for tracking the problem. Is the below example similar to your problem or is the switch back to RGB important?

function setup() {
  createCanvas(400, 400);
  colorMode(HSB, 255);
  let c = color(50, 50, 50);
  fill(c);
  rect(0, 0, width);
  
  const detectedColor = get(200, 200);
  
  const h = hue(detectedColor);
  const s = saturation(detectedColor);
  const b = brightness(detectedColor);
  fill(255);
  text("write color", 10, 30);
  text("h 50\ns 50\nb 50", 30, 50);
  text("read color", 320, 30);
  text("h " +int(h) +"\ns " +int(s) + "\nb " + +int(b), 350, 50);
}

Either way I think behavior is unexpected but I also have no experience with the color modes.

stalgiag avatar Jul 16 '20 15:07 stalgiag

I think the bug I'm experiencing happens in the mode switch between hsb and rgb. Here's your code updated to replicate my problem. (This all started because I wanted to make a better color picker than the one that comes with Processing.)

function setup() {
  createCanvas(400, 400);
  colorMode(HSB, 255);
  let c = color(50, 50, 50);
  fill(c);
  rect(0, 0, width, height);
  
  colorMode(RGB);
  const detectedColor = get(200, 200);
  
  const h = hue(detectedColor);
  const s = saturation(detectedColor);
  const b = brightness(detectedColor);
  fill(255);
  text("write color", 10, 30);
  text("h 50\ns 50\nb 50", 30, 50);
  text("read color", 320, 30);
  text("h " +int(h) +"\ns " +int(s) + "\nb " + +int(b), 350, 50);
}

joemckay5 avatar Jul 16 '20 20:07 joemckay5

@joemckay5 The problem is the line colorMode(RGB);. When you call saturation(detectedColor), internally a check is made to see if the current color mode is hsb or hsl. The way that code is written causes the value of saturation to be calculated according to hsl when the current mode is rgb, hence you are getting different saturation value than expected.

This issue is not faced while calculating hue since its same for both hsl and hsb.

@stalgiag @limzykenneth I am not sure if it is a good way of solving the issue but for handling cases like these where color mode is set to rgb, get method could be extended to return color object based on an optional parameter specifying the color mode, with default value as rgb. This way before returning the color object, suitable conversion from rgb to other modes can be made. Then there would not be need for additional call to saturation().

Prateek93a avatar Feb 06 '21 12:02 Prateek93a

I think that the behavior you're seeing here aligns with the documented functionality: https://p5js.org/reference/#/p5/hue

Am I misunderstanding?

lmccart avatar Mar 10 '21 01:03 lmccart

I have a simpler sketch that shows a problem with color saturation calculation.

function setup() {
  createCanvas(400, 400);
}

function draw() {
  
  colorMode(RGB)
  noStroke()
  background("white")
  
  let c = color(155,  34, 38) // 358,78,61 HSB values

  fill(c)
  rect(width/4, height/4, 100, 100)
  rect(width/4+100, height/4+100, 100, 100
      )
  let adjustedColor = adjustBrightness(c)
  
  fill(adjustedColor)
  rect(width/4, height/4+100, 100, 100)
  rect(width/4+100, height/4, 100, 100)
  
  // note the top right and lower left squares are less saturated even
  // though the color was not changed
}

// this function is essentially a no-op. 
// the brightness is not adjusted on line 41
function adjustBrightness(col) {

  colorMode(HSB)
  let b = brightness(col)
  let h = hue(col)
  let s = saturation(col)

  // here I would adjust brightness up or down, but even if I don't
  // change anything--just return a new color with the same components, 
  // the result is wrong
  
  let fb = b // no-op

  let newColor = color(h, s, fb)
 
  colorMode(RGB)
  return(color(red(newColor), green(newColor), blue(newColor)))

}

What I had wanted to do is to adjust the brightness of the color in the adjustBrightness() function. However, no matter what I did, the resulting colour would be desaturated. I made this sample sketch which literally does a no-op on the change of brightness. It should return the same color it was given. However, some console.log() statements will prove that the value of s = saturation(col) is incorrect. It should be 78, but it is 64. Since the color mode is HSB at the time, I do not think this accords with the documented behaviour.

Here is a link to the sketch: https://editor.p5js.org/fridgebuzz/sketches/k5KMGe3Rh

Is there any workaround for this issue? Thanks!

fridgebuzz avatar Jan 25 '23 02:01 fridgebuzz

While I still think it's a bug, I did find a workaround (which the linked sketch now shows). If you set the colorMode to HSB before you create an RGB color (using the string form: color('rgb(123,45,67)'), then when you set the colorMode to HSB and retrieve the saturation it will be correct. Hope that saves someone the two days it took me to figure it all out! Is this really how it's meant to work? Thanks.

fridgebuzz avatar Jan 25 '23 02:01 fridgebuzz

fridgebuzz is right. The function saturation() always returns the HSL value and not the HSB value (even when colour mode is set to HSB), unless the colour is created with the RGB shorthand, while in HSB mode. It's a bug.

daylmer avatar Sep 15 '23 15:09 daylmer

The main problem we have here is that if the colorMode is in RGB there is no way for us to know when someone calls saturation() whether they mean it is saturation for HSB or HSL which are two different values. The documentation states that "By default, this function returns the HSL saturation. If the colorMode() is set to HSB, it returns the HSB saturation.".

For me this is less of a bug but more of a workaround undefined behaviour that's documented.

@daylmer The problem you are seeing is related to when a p5.Color is created, it keeps the colorMode of the sketch when it is created and not change when the sketch's colorMode change. In short the colorMode stays with the color once defined. This is documented here https://p5js.org/reference/#/p5.Color. We can add a clearer note to the color() documentation if this is easy to miss but it is intended behaviour.

limzykenneth avatar Sep 26 '23 16:09 limzykenneth

Relates to issue #7018

Qianqianye avatar Jun 05 '24 19:06 Qianqianye

I just noticed some color differences while working with the HSB color mode. After checking with a color picker, I confirmed that the hue and brightness remain the same, the variation occurs only in the saturation.

The first line uses the default color system. col is a color object.

image

Vamoss avatar Nov 20 '24 14:11 Vamoss

A small fix, paste anywhere in your sketch:

p5.prototype.saturation = function(c) {
	c = this.color(c);
	const r = red(c);
	const g = green(c);
	const b = blue(c);
	const max = Math.max(r, g, b);
	const min = Math.min(r, g, b);
	const delta = max - min;
	const saturation = max === 0 ? 0 : delta / max;
	return saturation * 100;
}

Vamoss avatar Nov 20 '24 21:11 Vamoss

Closing as there is a new system in 2.0. Thank you!

ksen0 avatar Apr 17 '25 11:04 ksen0