From 47bf12cbac1005f6e4940b5c51a93f0acb7d74a9 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 2 Sep 2018 13:06:28 +0200 Subject: [PATCH 1/2] Change `JpegImage._isColorConversionNeeded` into a getter, rather than a regular function Given how `_isColorConversionNeeded` is used, and that it always returns a boolean value, having it be a getter seems more appropriate. --- src/core/jpg.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/jpg.js b/src/core/jpg.js index b85e1d0ed..a08e86ae3 100644 --- a/src/core/jpg.js +++ b/src/core/jpg.js @@ -1024,7 +1024,7 @@ var JpegImage = (function JpegImageClosure() { return data; }, - _isColorConversionNeeded() { + get _isColorConversionNeeded() { if (this.adobe) { // The adobe transform marker overrides any previous setting. return !!this.adobe.transformCode; @@ -1180,10 +1180,10 @@ var JpegImage = (function JpegImageClosure() { rgbData[offset++] = grayColor; } return rgbData; - } else if (this.numComponents === 3 && this._isColorConversionNeeded()) { + } else if (this.numComponents === 3 && this._isColorConversionNeeded) { return this._convertYccToRgb(data); } else if (this.numComponents === 4) { - if (this._isColorConversionNeeded()) { + if (this._isColorConversionNeeded) { if (forceRGBoutput) { return this._convertYcckToRgb(data); } From 663922f93fceca6ed59d7d52ef108c883073bf89 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 2 Sep 2018 13:55:27 +0200 Subject: [PATCH 2/2] Add a new parameter to `JpegImage.getData` to indicate the source of the image data (issue 9513) The purpose of this patch is to provide a better default behaviour when `JpegImage` is used to parse standalone JPEG images with CMYK colour spaces. Since the issue that the patch concerns is somewhat of a special-case, the implementation utilizes the already existing decode support in an attempt to minimize the impact w.r.t. code size. *Please note:* It's always possible for the user of `JpegImage` to control image inversion, and thus override the new behaviour, by simply passing a custom `decodeTransform` array upon initialization. --- examples/image_decoders/jpeg_viewer.js | 6 ++++- src/core/jpeg_stream.js | 8 +++++-- src/core/jpg.js | 33 +++++++++++++++++++------- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/examples/image_decoders/jpeg_viewer.js b/examples/image_decoders/jpeg_viewer.js index 5bc81ecf3..d5dd54d2c 100644 --- a/examples/image_decoders/jpeg_viewer.js +++ b/examples/image_decoders/jpeg_viewer.js @@ -58,7 +58,11 @@ var jpegImage = new pdfjsImageDecoders.JpegImage(); jpegImage.parse(typedArrayImage); var width = jpegImage.width, height = jpegImage.height; -var jpegData = jpegImage.getData(width, height, /* forceRGB = */ true); +var jpegData = jpegImage.getData({ + width, + height, + forceRGB: true, +}); // Render the JPEG image on a . // diff --git a/src/core/jpeg_stream.js b/src/core/jpeg_stream.js index cc8070a67..eaa1f0acb 100644 --- a/src/core/jpeg_stream.js +++ b/src/core/jpeg_stream.js @@ -97,8 +97,12 @@ let JpegStream = (function JpegStreamClosure() { const jpegImage = new JpegImage(jpegOptions); jpegImage.parse(this.bytes); - let data = jpegImage.getData(this.drawWidth, this.drawHeight, - this.forceRGB); + let data = jpegImage.getData({ + width: this.drawWidth, + height: this.drawHeight, + forceRGB: this.forceRGB, + isSourcePDF: true, + }); this.buffer = data; this.bufferLength = data.length; this.eof = true; diff --git a/src/core/jpg.js b/src/core/jpg.js index a08e86ae3..3d97c7687 100644 --- a/src/core/jpg.js +++ b/src/core/jpg.js @@ -975,7 +975,7 @@ var JpegImage = (function JpegImageClosure() { this.numComponents = this.components.length; }, - _getLinearizedBlockData: function getLinearizedBlockData(width, height) { + _getLinearizedBlockData(width, height, isSourcePDF = false) { var scaleX = this.width / width, scaleY = this.height / height; var component, componentScaleX, componentScaleY, blocksPerScanline; @@ -1013,7 +1013,24 @@ var JpegImage = (function JpegImageClosure() { } // decodeTransform contains pairs of multiplier (-256..256) and additive - const transform = this._decodeTransform; + let transform = this._decodeTransform; + + // In PDF files, JPEG images with CMYK colour spaces are usually inverted + // (this can be observed by extracting the raw image data). + // Since the conversion algorithms (see below) were written primarily for + // the PDF use-cases, attempting to use `JpegImage` to parse standalone + // JPEG (CMYK) images may thus result in inverted images (see issue 9513). + // + // Unfortunately it's not (always) possible to tell, from the image data + // alone, if it needs to be inverted. Thus in an attempt to provide better + // out-of-box behaviour when `JpegImage` is used standalone, default to + // inverting JPEG (CMYK) images if and only if the image data does *not* + // come from a PDF file and no `decodeTransform` was passed by the user. + if (!transform && numComponents === 4 && !isSourcePDF) { + transform = new Int32Array([ + -256, 255, -256, 255, -256, 255, -256, 255]); + } + if (transform) { for (i = 0; i < dataLength;) { for (j = 0, k = 0; j < numComponents; j++, i++, k += 2) { @@ -1162,14 +1179,14 @@ var JpegImage = (function JpegImageClosure() { return data.subarray(0, offset); }, - getData: function getData(width, height, forceRGBoutput) { + getData({ width, height, forceRGB = false, isSourcePDF = false, }) { if (this.numComponents > 4) { throw new JpegError('Unsupported color mode'); } - // type of data: Uint8Array(width * height * numComponents) - var data = this._getLinearizedBlockData(width, height); + // Type of data: Uint8ClampedArray(width * height * numComponents) + var data = this._getLinearizedBlockData(width, height, isSourcePDF); - if (this.numComponents === 1 && forceRGBoutput) { + if (this.numComponents === 1 && forceRGB) { var dataLength = data.length; var rgbData = new Uint8ClampedArray(dataLength * 3); var offset = 0; @@ -1184,11 +1201,11 @@ var JpegImage = (function JpegImageClosure() { return this._convertYccToRgb(data); } else if (this.numComponents === 4) { if (this._isColorConversionNeeded) { - if (forceRGBoutput) { + if (forceRGB) { return this._convertYcckToRgb(data); } return this._convertYcckToCmyk(data); - } else if (forceRGBoutput) { + } else if (forceRGB) { return this._convertCmykToRgb(data); } }