photopea / UPNG.js

Fast and advanced PNG (APNG) decoder and encoder (lossy / lossless)
MIT License
2.1k stars 259 forks source link

Error in `UPNG.encode.compress` when encoding RGBA images #5

Closed mn4367 closed 7 years ago

mn4367 commented 7 years ago

I tried to create a Node.js module from UPNG.js. To do so I added

var pako = require("pako");

as the first line and

module.exports = UPNG;

as the last line in UPNG.js.

Then I loaded pug.png from the samples page at https://blog.photopea.com/png-minifier-inside-photopea.html. The following code was used (running in Node.js):

"use strict";
var UPNG = require("./UPNG");
var FS = require("fs");
var input = FS.readFileSync('pug.png');
var decoded = UPNG.decode(input);
var encoded = new Uint8Array(UPNG.encode(decoded.data, decoded.width, decoded.height, 0));
FS.writeFileSync('test.png', encoded);

I receive the following error:

/Users/.../JS/UPNG.js/UPNG.js:119
    var img32 = new Uint32Array(img.buffer);
                ^

RangeError: byte length of Uint32Array should be a multiple of 4
    at new Uint32Array (native)
    at Function.UPNG.encode.compress (/Users/.../JS/UPNG.js/UPNG.js:119:14)
    at Object.UPNG.encode (/Users/.../JS/UPNG.js/UPNG.js:66:25)
    at Object.<anonymous> (/Users/.../JS/UPNG.js/UPNG-cli.js:13:20)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.runMain (module.js:605:

With another test file (attached zip) I get:

unknown chunk type sBIT 3

and the resulting file has scrambled content.

Archive.zip

mn4367 commented 7 years ago

I tested both files also with Jimp and there were no problems loading them.

photopea commented 7 years ago

Hi, you must push your decoded result through UPNG.toRGBA8 before encoding it again, I have added an example at the main page.

PNG images with two colors may use 1 bit per pixel. UPNG.decode() returns pixel data in this original form. Some users may want to keep the data in this form, instead of converting them to RGBA8 (which would make the image 32 times bigger in this case - four bytes per pixel). That is why I separated the decoding into two steps.

Do you think we should convert to RGBA8 right inside UPNG.decode() ? Also, PNGs can have 16 bits per channel sometimes (8 bytes per four-channel pixel), so after converting them to RGBA8, the quality would be lost. 16-bits PNGs can not be encoded by UPNG.js yet.

mn4367 commented 7 years ago

Hi, you must push your decoded result through UPNG.toRGBA8 before encoding it again, I have added an example at the main page.

Ah OK, now it works, thank you very much!

Do you think we should convert to RGBA8 right inside UPNG.decode() ? Also, PNGs can have 16 bits per channel sometimes (8 bytes per four-channel pixel), so after converting them to RGBA8, the quality would be lost. 16-bits PNGs can not be encoded by UPNG.js yet.

Wouldn't this compromise a bit of the flexibility since the output of decode would then be always RBGA8? And with regard to the "auto-loss" with 16Bits this isn't a good solution, I think.

At least for my use case I don't need it, but other users may think different.

Thanks again!

photopea commented 7 years ago

Ok, I will keep it as it is.