Commit 61e4bfab authored by Noel Gordon's avatar Noel Gordon Committed by Commit Bot

[piexwasm] Remove ColorSpace from ImageLoaderRequest

ColorSpace is an internal detail of the image loader and should not be
visible to clients: remove it from ImageLoaderRequest.

The internal use is in ImageRequestTask: add a colorSpace_ member, and
document that it used for RAW images (PIEX) only. After CL:2432150, it
was decided that we just use the raw string PIEX color space value, so
extend that idea and use the raw string in ImageLoaderUtil too (allows
us to remove ColorSpace). No change in behavior.

Spelling correction: ganna => gamma. Checked the veracity of the color
space transform adobeRgb to sRgb and it is correct. This code appeared
before the <canvas> element had color management. The <canvas> element
supports that feature today in Chrome: add a TODO.

Bug: 1132695
Change-Id: I12cad9a0aea106ce4a6c6dcbc2a9137bf0224e58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2437530Reviewed-by: default avatarTrent Apted <tapted@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811549}
parent 26fcd20c
......@@ -26,11 +26,6 @@ ImageLoaderUtil.shouldProcess = function(width, height, request) {
return true;
}
// Non-standard color space has to be converted.
if (request.colorSpace && request.colorSpace !== ColorSpace.SRGB) {
return true;
}
// No changes required.
return false;
};
......@@ -198,15 +193,13 @@ ImageLoaderUtil.MATRIX_FROM_ADOBE_TO_STANDARD = [
];
/**
* Converts the canvas of color space into sRGB.
* Converts the canvas of color space into sRGB. TODO(noel): the Chrome <canvas>
* is color managed today. Is this code still needed?
* @param {HTMLCanvasElement} target Target canvas.
* @param {ColorSpace} colorSpace Current color space.
* @param {string} colorSpace Current color space.
*/
ImageLoaderUtil.convertColorSpace = function(target, colorSpace) {
if (colorSpace === ColorSpace.SRGB) {
return;
}
if (colorSpace === ColorSpace.ADOBE_RGB) {
if (colorSpace === 'adobeRgb') {
const matrix = ImageLoaderUtil.MATRIX_FROM_ADOBE_TO_STANDARD;
const context =
assertInstanceof(target.getContext('2d'), CanvasRenderingContext2D);
......@@ -218,17 +211,17 @@ ImageLoaderUtil.convertColorSpace = function(target, colorSpace) {
let adobeG = data[i + 1] / 255;
let adobeB = data[i + 2] / 255;
// Revert gannma transformation.
// Apply adobeRgb inverse gamma to convert to linear color.
adobeR = adobeR <= 0.0556 ? adobeR / 32 : Math.pow(adobeR, 2.2);
adobeG = adobeG <= 0.0556 ? adobeG / 32 : Math.pow(adobeG, 2.2);
adobeB = adobeB <= 0.0556 ? adobeB / 32 : Math.pow(adobeB, 2.2);
// Convert color space.
// Matrix convert linear adobeRgb color to linear sRgb color.
let sR = matrix[0] * adobeR + matrix[1] * adobeG + matrix[2] * adobeB;
let sG = matrix[3] * adobeR + matrix[4] * adobeG + matrix[5] * adobeB;
let sB = matrix[6] * adobeR + matrix[7] * adobeG + matrix[8] * adobeB;
// Gannma transformation.
// Convert linear color to sRgb gamma color.
sR = sR <= 0.0031308 ? 12.92 * sR : 1.055 * Math.pow(sR, 1 / 2.4) - 0.055;
sG = sG <= 0.0031308 ? 12.92 * sG : 1.055 * Math.pow(sG, 1 / 2.4) - 0.055;
sB = sB <= 0.0031308 ? 12.92 * sB : 1.055 * Math.pow(sB, 1 / 2.4) - 0.055;
......
......@@ -60,13 +60,21 @@ function ImageRequestTask(id, cache, piexLoader, request, callback) {
this.contentType_ = null;
/**
* IFD data of the fetched image. Only RAW images provide non-null ifd
* data at this time; images on Drive might provide ifd in future.
* IFD data of the fetched image. Only RAW images provide a non-null
* ifd at this time. Drive images might provide an ifd in future.
* @type {?string}
* @private
*/
this.ifd_ = null;
/**
* The color space of the fetched image. Only RAW images provide a
* color space at this time, being 'sRgb' or 'adobeRgb'.
* @type {?string}
* @private
*/
this.colorSpace_ = null;
/**
* Used to download remote images using http:// or https:// protocols.
* @type {XMLHttpRequest}
......@@ -336,9 +344,7 @@ ImageRequestTask.prototype.downloadOriginal_ = function(onSuccess, onFailure) {
function(data) {
this.request_.orientation =
ImageOrientation.fromExifOrientation(data.orientation);
const isAdobeRgb = data.colorSpace === 'adobeRgb';
this.request_.colorSpace =
isAdobeRgb ? ColorSpace.ADOBE_RGB : ColorSpace.SRGB;
this.colorSpace_ = data.colorSpace;
this.ifd_ = data.ifd;
this.contentType_ = data.mimeType;
const blob = new Blob([data.thumbnail], {type: data.mimeType});
......@@ -560,24 +566,27 @@ ImageRequestTask.prototype.sendImageData_ = function(width, height, data) {
};
/**
* Handler, when contents are loaded into the image element. Performs resizing
* and finalizes the request process.
* Handler, when contents are loaded into the image element. Performs image
* processing operations if needed, and finalizes the request process.
* @private
*/
ImageRequestTask.prototype.onImageLoad_ = function() {
const imageColorSpace = this.colorSpace_ || 'sRgb';
// Perform processing if the url is not a data url, or if there are some
// operations requested.
let imageChanged = false;
if (!(this.request_.url.match(/^data/) ||
this.request_.url.match(/^drivefs:/)) ||
ImageLoaderUtil.shouldProcess(
this.image_.width, this.image_.height, this.request_)) {
this.image_.width, this.image_.height, this.request_) ||
(imageColorSpace !== 'sRgb')) {
ImageLoaderUtil.resizeAndCrop(this.image_, this.canvas_, this.request_);
ImageLoaderUtil.convertColorSpace(
this.canvas_, this.request_.colorSpace || ColorSpace.SRGB);
this.sendImage_(true); // Image changed.
} else {
this.sendImage_(false); // Image not changed.
ImageLoaderUtil.convertColorSpace(this.canvas_, imageColorSpace);
imageChanged = true; // The image is now on the <canvas>.
}
this.sendImage_(imageChanged);
this.cleanup_();
this.downloadCallback_();
};
......
......@@ -4,16 +4,6 @@
'use strict';
/**
* Color space.
*
* @enum {string}
*/
const ColorSpace = {
SRGB: 'sRgb',
ADOBE_RGB: 'adobeRgb'
};
/**
* Response status.
*
......@@ -143,13 +133,6 @@ class LoadImageRequest {
this.cache;
/** @type {number|undefined} */
this.priority;
/**
* ColorSpace, only used for piex images.
*
* @type{ColorSpace|undefined}
*/
this.colorSpace;
}
/**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment