Bug 380894 - FigureUtilities lighter produce in certain cases not the correct color
Summary: FigureUtilities lighter produce in certain cases not the correct color
Status: NEW
Alias: None
Product: GEF
Classification: Tools
Component: GEF-Legacy Draw2d (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: gef-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-29 09:04 EDT by Alois Zoitl CLA
Modified: 2012-08-04 13:08 EDT (History)
1 user (show)

See Also:


Attachments
patch as outlined in the previouse comment (4.87 KB, patch)
2012-06-03 15:51 EDT, Alois Zoitl CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alois Zoitl CLA 2012-05-29 09:04:52 EDT
Build Identifier: Version: Indigo Service Release 2 Build id: 20120216-1857

The helper functions lighter operate in the rgb color space and there the each element is transformed independent of the others. If you have pure colors (e.g., red with rgb<255,0,0>) than the transformation results in the same color. The correct way would be to use the HSL color space. I did some tests with it and than the problem is gone. 
 
Furthermore this affects also other helper functions in the FigureUtilities like paintEtchedBorder. 

Reproducible: Always
Comment 1 Alexander Nyßen CLA 2012-05-30 09:30:50 EDT
Alois, would you be willing to provide a patch?
Comment 2 Alois Zoitl CLA 2012-05-30 11:05:00 EDT
The changes are rather small so not sure if they are worth the effort. Here are the code changes required for the FigureUtilities class. I tested them in my workaround for this bug. 

private static final float VALUE_MULTIPLIER = 0.6f;

	public static Color lighter(Color original) {
		double hsl[] = rgbToHSL(original.getRGB());
		hsl[2] = Math.min(1.0, hsl[2] / VALUE_MULTIPLIER);
		return new Color(null, hslToRGB(hsl));
	}

	public static Color darker(Color original) {
		double hsl[] = rgbToHSL(original.getRGB());
		hsl[2] = hsl[2] * VALUE_MULTIPLIER;
		return new Color(null,  hslToRGB(hsl));
	}

	/** Transforms the RGB color into the according HSL color space.
	 * Algorithm based on the Book:
	 * Computer Graphics: Principles and Practice in C (2nd Edition) (Systems Programming Series) (Hardcover)
	 * by James D. Foley (Author), Andries van Dam (Author), Steven K. Feiner (Author), John F. Hughes (Author)  
	 * 
	 * @param rgb the rgb values of the color to convert
	 * @return the color in hsl space h: 0..360, s: 0..1, l: 0..1
	 */
	public static double[] rgbToHSL(RGB rgb) {

		double r = rgb.red / 255.f;
		double g = rgb.green / 255.f;
		double b = rgb.blue / 255.f;
		double max = Math.max(Math.max(r, g), b);
		double min = Math.min(Math.min(r, g), b);
		
		double hsl[] = new double[]{0, 0, (max + min)/2 };
		
		if(max != min){
			//we are not just grey
			double delta = max - min;
			
			if(hsl[2] <= 0.5){
				hsl[1] = (delta / (max + min));
			}else {
				hsl[1] = (delta /( 2.0 - (max + min)));
			}
			
			if(r == max){
				hsl[0] =  (g - b) / delta;
			}else if (g == max){
				hsl[0] = 2.0 + (b - r) / delta;				
			}else if (b == max){
				hsl[0] = 4.0 + (r - g) / delta;
			}
			hsl[0] *= 60.0;
			
			if(hsl[0] < 0.0){
				hsl[0] += 360.0;
			}
		}
		return hsl;
	}

	/**Transforms the HSL color into the according RGB color space.
	 * Algorithm based on the Book:
	 * Computer Graphics: Principles and Practice in C (2nd Edition) (Systems Programming Series) (Hardcover)
	 * by James D. Foley (Author), Andries van Dam (Author), Steven K. Feiner (Author), John F. Hughes (Author)  
	 * 
	 * @param hsl the color in hsl space h: 0..360, s: 0..1, l: 0..1
	 * @return the color in rgb space
	 */
	public static RGB hslToRGB(double[] hsl) {
		RGB retVal = new RGB(0,0,0);
		
		
		
		if(hsl[1] == 0.0){
			if(hsl[0] == 0.0){
				retVal.red = retVal.green = retVal.blue = (int)(hsl[2] * 255.0);
			}
			else{
				// in the achromatic (grey) case h must not have a value
				SWT.error(SWT.ERROR_INVALID_ARGUMENT);				
			}			
		}
		else{
			double m2= ((hsl[2] <= 0.5) ? hsl[2] * (1.0 + hsl[1]) : hsl[2] + hsl[1] - (hsl[2] * hsl[1]));
			double m1 = 2.0 * hsl[2] - m2;
			
			retVal.red = hslValue(m1, m2, hsl[0] + 120.0);
			retVal.green = hslValue(m1, m2, hsl[0]); 
			retVal.blue = hslValue(m1, m2, hsl[0]- 120.0);
		}
		return retVal;
	}

	private static int hslValue(double m1, double m2, double hue) {
		double retVal = m1;
		
		if(hue > 360.0){
			hue -= 360.0;
		} else if (hue < 0.0){
			hue += 360.0;
		}
		
		if( hue < 60.0){
			retVal = m1 + (m2 - m1) * hue / 60.0;
		}else if (hue < 180.0){
			retVal = m2;
		}else if (hue < 240.0){
			retVal = m1 + (m2 - m1) * (240.0 - hue) / 60.0;
		}
				
		return (int)(255.0 * retVal);
	}
Comment 3 Alexander Nyßen CLA 2012-06-01 05:43:55 EDT
Alois, it would be nice if you could produce a Git patch as outlined in our contribution guide (http://wiki.eclipse.org/GEF_Contributor_Guide), because this will be needed to create a CQ for your contribution.
Comment 4 Alois Zoitl CLA 2012-06-03 15:51:28 EDT
Created attachment 216739 [details]
patch as outlined in the previouse comment
Comment 5 Alexander Nyßen CLA 2012-06-04 02:57:42 EDT
(In reply to comment #4)
> Created attachment 216739 [details]
> patch as outlined in the previouse comment

Thanks! Created CQ 6559 to keep track of this contribution.
Comment 6 Alexander Nyßen CLA 2012-08-04 13:08:38 EDT
(In reply to comment #5)
> (In reply to comment #4)
> > Created attachment 216739 [details]
> > patch as outlined in the previouse comment
> 
> Thanks! Created CQ 6559 to keep track of this contribution.

CQ 6559 has been approved, so this is good to go now...