Differential D1082 Diff 3412 source/blender/imbuf/intern/colormanagement.c

Changeset View

Standalone View

source/blender/imbuf/intern/colormanagement.c

Context not available.
	static int global_tot_view = 0;	static int global_tot_view = 0;
	static int global_tot_looks = 0;	static int global_tot_looks = 0;

		static float luma_coefficients[3] = { 0.0, 0.0, 0.0 };
		sergeyUnsubmitted Not Done Inline Actions First of all guess it should be default coefficients here. So things keeps working if there's no coefficients in the config.ocio. Also, make it float values and avoid double promotion. sergey: First of all guess it should be default coefficients here. So things keeps working if there's…
		sergeyUnsubmitted Not Done Inline Actions Just standard name is enough, we can totally see values in the actual code. sergey: Just standard name is enough, we can totally see values in the actual code.
		sobotkaUnsubmitted Not Done Inline Actions Changed. sobotka: Changed.

	/* lock used by pre-cached processors getters, so processor wouldn't	/* lock used by pre-cached processors getters, so processor wouldn't
	* be created several times	* be created several times
	* LOCK_COLORMANAGE can not be used since this mutex could be needed to	* LOCK_COLORMANAGE can not be used since this mutex could be needed to
Context not available.

	colormanage_look_add(name, process_space, false);	colormanage_look_add(name, process_space, false);
	}	}

		/* load luma coefficients */
		sergeyUnsubmitted Not Done Inline Actions Sentence starts with capital letter and ends with fullstop. sergey: Sentence starts with capital letter and ends with fullstop.
		sobotkaUnsubmitted Not Done Inline Actions Tried to match existing comment style by file's author. Fixed. sobotka: Tried to match existing comment style by file's author. Fixed.
		OCIO_configGetDefaultLumaCoefs(config, luma_coefficients);
	}	}

	static void colormanage_free_config(void)	static void colormanage_free_config(void)
Context not available.
	return ibuf->rect_colorspace->name;	return ibuf->rect_colorspace->name;
	}	}

		/* Convert a float RGB triplet to the correct luminance weighted average.
		sergeyUnsubmitted Not Done Inline Actions I'm still not sure why it's helpful to see this knowledge dump in here. sergey: I'm still not sure why it's helpful to see this knowledge dump in here.
		sobotkaUnsubmitted Not Done Inline Actions There were several comments in the previous implementations that showed a good deal of confusion regarding luminance values. I thought it would be future friendly to help get a developer up to speed on the correct values, and how to obtain them without merely referencing Poynton, and perhaps prevent the multiple / incorrect implementations from appearing in the future. Happy to remove it if desired. sobotka: There were several comments in the previous implementations that showed a good deal of…
		sergeyUnsubmitted Not Done Inline Actions IMO this belongs to wiki, not the code. You can link the wiki page from the comment tho. sergey: IMO this belongs to wiki, not the code. You can link the wiki page from the comment tho.
		sobotkaUnsubmitted Not Done Inline Actions Changed. sobotka: Changed.
		*
		* Addendum February 7, 2015:
		sergeyUnsubmitted Not Done Inline Actions Why do you need to dump it here? sergey: Why do you need to dump it here?
		*
		* Grayscale, or Luma is a distillation of RGB data values down to a weighted average
		* based on the luminance positions of the red, green, and blue primaries.
		* Given that the internal reference space may be arbitrarily set, any
		* effort to glean the luminance coefficients must be aware of the reference
		* space primaries. Currently, via OCIO, the only method provided is via
		*
		* void Config::getDefaultLumaCoefs(float* rgb) const
		*
		* The luminance coefficients are stored globally for performance.
		*
		* Terminology:
		*
		* Luminance refers to a weighted average for radiometrically linear values.
		* Referred to as Y, from the XYZ primary position.
		*
		* Luma refers to a weighted average luminance post nonlinear transfer curve.
		* Referred to as Y', indicating that the value has been nonlinearly transformed.
		*
		* Both forms Y and Y' use precisely the same luminance coefficients, regardless
		* as to whether the source image was nonlinear or linearized. The term difference
		* results from the fact that some hacks concatenate the weights down
		* to a single weight that combines both tone / transfer curve inversion and
		* luminance coefficients into a single value. Given the complexities of color
		* management in a color managed system however, hard coded values will break
		* color management. Further, it is impossible to arrive at a single weight for
		* linearizations that require two part formulas such as sRGB or ITU-REC.709.
		*
		* Developers:
		*
		* If a developer needs to linearize a value prior to determining the luminance
		* weighting, they must use the appropriate OCIO implementation to linearize
		* it to the reference space. This is because every RGB colorspace has a unique
		* transfer curve, and as such, there is no method to use hard coded values.
		*
		* Technique:
		*
		* The weights are always gleaned from the XYZ positions, in particular, the Y
		* position, of the primaries for a given RGB colorspace.
		*
		* For example, the matrix transformation for D65 sRGB / 709 (they share the same
		* primaries) is:
		*
		* R G B
		* X 0.4124564 0.3575761 0.1804375
		* Y 0.2126729 0.7151522 0.0721750
		* Z 0.0193339 0.1191920 0.9503041
		*
		* The luma / luminance weights therefore are 0.2126729 R, 0.7151522 G, and
		* 0.0721750 B. The coefficients sum to 1.0.
		*
		* These weights are kept within the OpenColorIO configuration file:
		* "luma: [Y.RED, Y.GREEN, Y.BLUE]"
		*/

		float IMB_colormanagement_get_luminance(const float rgb[3])
		sergeyUnsubmitted Not Done Inline Actions Indentation is weird. Plus it seems you can use dot-product from BLI_math sergey: Indentation is weird. Plus it seems you can use dot-product from BLI_math
		sobotkaUnsubmitted Not Done Inline Actions Noted. Will adjust. Indentation is on fours, which looks to misalign the second and third lines. I could cheat the second and third back a space, but that too seems odd? sobotka: Noted. Will adjust. Indentation is on fours, which looks to misalign the second and third…
		sergeyUnsubmitted Not Done Inline Actions I'd make next lines indented with single tab and space to keep words aligned. But really consider using a dot-product. sergey: I'd make next lines indented with single tab and space to keep words aligned. But really…
		sobotkaUnsubmitted Not Done Inline Actions Changed. sobotka: Changed.
		{
		return luma_coefficients[0] * rgb[0] +
		luma_coefficients[1] * rgb[1] +
		sergeyUnsubmitted Not Done Inline Actions Indentation seems broken. sergey: Indentation seems broken.
		luma_coefficients[2] * rgb[2];
		}

		/* Byte equivalent of IMB_colormanagement_get_luminance()
		sergeyUnsubmitted Not Done Inline Actions Fullstop. Plus `/` in this case belongs to the same line. sergey:* Fullstop. Plus `*/` in this case belongs to the same line.
		*/
		sergeyUnsubmitted Not Done Inline Actions You can't just cast because of machine epsilon. Use FTOCHAR instead. sergey: You can't just cast because of machine epsilon. Use FTOCHAR instead.
		sobotkaUnsubmitted Not Done Inline Actions Noted. Will adjust. sobotka: Noted. Will adjust.
		sobotkaUnsubmitted Not Done Inline Actions Changed according to existing standards in colortools.c. sobotka: Changed according to existing standards in colortools.c.
		unsigned char IMB_colormanagement_get_luminance_byte(const unsigned char rgb[3])
		{
		return (unsigned char)(luma_coefficients[0] * (float)rgb[0] +
		(float)luma_coefficients[1] * rgb[1] +
		sergeyUnsubmitted Not Done Inline Actions Indentation. Plus casts seems weird? sergey: Indentation. Plus casts seems weird?
		sobotkaUnsubmitted Not Done Inline Actions Good spot on my mistake. sobotka: Good spot on my mistake.
		(float)luma_coefficients[2] * rgb[2]);
		}

	/********************* Threaded display buffer transform routines ***********************/	/********************* Threaded display buffer transform routines ***********************/

	typedef struct DisplayBufferThread {	typedef struct DisplayBufferThread {
Context not available.