Grayscale PNG conversion increase brightness #437

Closed
iBotPeaches opened this Issue Mar 18, 2015 · 10 comments

Comments

Projects
None yet
1 participant
@iBotPeaches
Owner

iBotPeaches commented Mar 18, 2015

Original issue 326 created by peacech on 2012-04-27T06:35:20.000Z:

What steps will reproduce the problem?

  1. Decode apk containing grayscale 9patch png
    2.
    3.

What is the expected output? What do you see instead?
The png has same brightness, instead the resulting png is brighter.

What version of the product are you using? On what operating system?
I think this is actually java problem.

Please provide any additional information below.

I fixed the problem using this snippet (note: messy code, I'm not a java programmer) in method decode in Res9patchStreamDecoder class:

BufferedImage im2 = new BufferedImage(w + 2, h + 2, BufferedImage.TYPE_4BYTE_ABGR);
Raster src = im.getRaster();
WritableRaster dst = im2.getRaster();
int nbands = im.getSampleModel().getNumBands();
int[] bands = new int[4];
if (nbands == 2) {
bands[0] = bands[1] = bands[2] = 0;
bands[3] = 1;
} else {
bands[0] = 0; bands[1] = 1; bands[2] = 2; bands[3] = 3;
}
int[] band = null;
for (int y = 0; y < h; y++) {
for (int bi = 0; bi < 4; bi++) {
band = src.getSamples(0, y, w, 1, bands[bi], band);
dst.setSamples(1, y + 1, w, 1, bi, band);
}
}

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 18, 2015

Owner

Comment #1 originally posted by yyjdelete on 2012-05-16T05:53:42.000Z:

I try the patch and it do work well

What`s more, the code "
NinePatch np = getNinePatch(data);
" just after the patch, need to be move up to before the patch for fixing ArrayIndexOutOfBoundsException when getSamples if "cant find 9patch chunk in file"

Owner

iBotPeaches commented Mar 18, 2015

Comment #1 originally posted by yyjdelete on 2012-05-16T05:53:42.000Z:

I try the patch and it do work well

What`s more, the code "
NinePatch np = getNinePatch(data);
" just after the patch, need to be move up to before the patch for fixing ArrayIndexOutOfBoundsException when getSamples if "cant find 9patch chunk in file"

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 18, 2015

Owner

Comment #2 originally posted by connor.tumbleson on 2012-11-16T13:19:59.000Z:

I have tested this code personally in Apktool v1.4.7 and it had to be reverted on Apktool v1.4.8 due to Index OutOfBounds frequent errors (probably with bad PNGs). Will need to re-work fix.

Owner

iBotPeaches commented Mar 18, 2015

Comment #2 originally posted by connor.tumbleson on 2012-11-16T13:19:59.000Z:

I have tested this code personally in Apktool v1.4.7 and it had to be reverted on Apktool v1.4.8 due to Index OutOfBounds frequent errors (probably with bad PNGs). Will need to re-work fix.

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 18, 2015

Owner

Comment #3 originally posted by connor.tumbleson on 2013-03-20T11:27:12.000Z:

Issue 432 has been merged into this issue.

Owner

iBotPeaches commented Mar 18, 2015

Comment #3 originally posted by connor.tumbleson on 2013-03-20T11:27:12.000Z:

Issue 432 has been merged into this issue.

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 18, 2015

Owner

Comment #4 originally posted by hackermiww on 2013-03-20T12:08:57.000Z:

this is a jdk bug, here the bug report http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5051418.

is there a method to avoid the problem?

Owner

iBotPeaches commented Mar 18, 2015

Comment #4 originally posted by hackermiww on 2013-03-20T12:08:57.000Z:

this is a jdk bug, here the bug report http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5051418.

is there a method to avoid the problem?

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 18, 2015

Owner

Comment #5 originally posted by connor.tumbleson on 2013-03-26T15:25:03.000Z:

The JDK website has been down a lot, so I'm copying information here.

A TYPE_CUSTOM BufferedImage with the same SampleModel, ColorModel and pixel data as a TYPE_BYTE_GRAY BufferedImage is rendered differently (it shows up a lighter gray). A test case that demonstrates this is attached.

The Java Advanced Imaging API (JAI) uses a custom subclass of WritableRaster in some instances, which when converted to a BufferedImage, results in a TYPE_CUSTOM BufferedImage, causing lighter rendering to be noticed by customers. This issue was reported by a JAI customer:

http://archives.java.sun.com/cgi-bin/wa?A2=ind0405&L=jai-interest&F=&S=&P=1121

EVALUATION

The root of problem can be reduced to he question
"What kind of data is supposed to be stored in data buffer?
Is it always sRGB or native representation for used color space?"

There are three ways to access image pixel data (data buffer):

a) use setRGB/getRGB
b) use optimized blits
c) direct access to the data buffer.

The difference between these approaches shows up when image uses color
model based on non-sRGB color space (e.g. grayscale images).

At the moment our implementation is following:

  • In a) case colors are actually converted from sRGB to image color
    space and vice versa.
  • In case b) results depend on implementation of the blit.
    Generic blits (e.g. AnyToIntArgb and IntArgbToAny) are implemented using
    setRGB/getRGB and therefore colors are stored in "native" form.
    However, other blits (e.g. ByteGrayToIntArgb and IntArgbToByteGray)
    do not perform any conversion and therefore we may have sRGB data in
    the buffer.
  • In case of c) no convertion is performed and raw data is used.

Note that if pixel data are used by paired methods only (e.g. setRGB/getRGB
or set with blit and get with blit) we end up with same output as input.
However, if blit without conversion is used to set data and then data
are accessed with getRGB() result is different from original data.

Fortunately this inconsistency has limited impact because it is
applicable to non sRGB color spaces only.
Still it is easy to reproduce with grayscale images. E.g.

  • create TYPE_BYTE_GRAY and corresponding custom image.
  • fill the data buffer directly with same data.
  • draw image to some sRGB destination.

As the result the custom image will be look brighter then
TYPE_BYTE_GRAY. It is because the TYPE_BYTE_GRAY is drawn by optimized
blits (without "gamma correction") whereas custom image is draw by using
getRGB method (with "gamma correction").

Note that similar problem theoretically may happen with other color
spaces like linear RGB.
It is not easily reproducible now because we have no predefined image
types based on linear RGB color space.

Also note that generic blits use setRGB/getRGB so for custom images
results are always consistent.

It seems that solution is to add conversion logic to blits (because it
seems reasonable to expect that raw data buffer contains converted data).
It certainly will have performance hit and have to be done carefully.

For instance at the moment it seems we only have to worry about "gamma
correction" and can probably use something similar to LUT-based
approach that was added to ComponentColorModel in 1.3.x.

Owner

iBotPeaches commented Mar 18, 2015

Comment #5 originally posted by connor.tumbleson on 2013-03-26T15:25:03.000Z:

The JDK website has been down a lot, so I'm copying information here.

A TYPE_CUSTOM BufferedImage with the same SampleModel, ColorModel and pixel data as a TYPE_BYTE_GRAY BufferedImage is rendered differently (it shows up a lighter gray). A test case that demonstrates this is attached.

The Java Advanced Imaging API (JAI) uses a custom subclass of WritableRaster in some instances, which when converted to a BufferedImage, results in a TYPE_CUSTOM BufferedImage, causing lighter rendering to be noticed by customers. This issue was reported by a JAI customer:

http://archives.java.sun.com/cgi-bin/wa?A2=ind0405&L=jai-interest&F=&S=&P=1121

EVALUATION

The root of problem can be reduced to he question
"What kind of data is supposed to be stored in data buffer?
Is it always sRGB or native representation for used color space?"

There are three ways to access image pixel data (data buffer):

a) use setRGB/getRGB
b) use optimized blits
c) direct access to the data buffer.

The difference between these approaches shows up when image uses color
model based on non-sRGB color space (e.g. grayscale images).

At the moment our implementation is following:

  • In a) case colors are actually converted from sRGB to image color
    space and vice versa.
  • In case b) results depend on implementation of the blit.
    Generic blits (e.g. AnyToIntArgb and IntArgbToAny) are implemented using
    setRGB/getRGB and therefore colors are stored in "native" form.
    However, other blits (e.g. ByteGrayToIntArgb and IntArgbToByteGray)
    do not perform any conversion and therefore we may have sRGB data in
    the buffer.
  • In case of c) no convertion is performed and raw data is used.

Note that if pixel data are used by paired methods only (e.g. setRGB/getRGB
or set with blit and get with blit) we end up with same output as input.
However, if blit without conversion is used to set data and then data
are accessed with getRGB() result is different from original data.

Fortunately this inconsistency has limited impact because it is
applicable to non sRGB color spaces only.
Still it is easy to reproduce with grayscale images. E.g.

  • create TYPE_BYTE_GRAY and corresponding custom image.
  • fill the data buffer directly with same data.
  • draw image to some sRGB destination.

As the result the custom image will be look brighter then
TYPE_BYTE_GRAY. It is because the TYPE_BYTE_GRAY is drawn by optimized
blits (without "gamma correction") whereas custom image is draw by using
getRGB method (with "gamma correction").

Note that similar problem theoretically may happen with other color
spaces like linear RGB.
It is not easily reproducible now because we have no predefined image
types based on linear RGB color space.

Also note that generic blits use setRGB/getRGB so for custom images
results are always consistent.

It seems that solution is to add conversion logic to blits (because it
seems reasonable to expect that raw data buffer contains converted data).
It certainly will have performance hit and have to be done carefully.

For instance at the moment it seems we only have to worry about "gamma
correction" and can probably use something similar to LUT-based
approach that was added to ComponentColorModel in 1.3.x.

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 18, 2015

Owner

Comment #6 originally posted by christiaan.van.dijk@hccnet.nl on 2013-07-07T19:30:56.000Z:

Hi, I ran into this problem when customizing a ROM, after some puzzling I found a solution which gives good results as far as I could test.

Solution is to use the ImageTypeSpecifier class, with this class a new BufferedImage with the same parameters as the original can be created, even if the returned type is unknown!
In this way we can keep the new image always in the same format as the original and have no conversion issues giving the increase in brightness.
Checked on a ZTE ROM but needs more testing, not verified on phone yet but resulting image files look good. Thought it would be good to share; just included the complete file, not too big.

Owner

iBotPeaches commented Mar 18, 2015

Comment #6 originally posted by christiaan.van.dijk@hccnet.nl on 2013-07-07T19:30:56.000Z:

Hi, I ran into this problem when customizing a ROM, after some puzzling I found a solution which gives good results as far as I could test.

Solution is to use the ImageTypeSpecifier class, with this class a new BufferedImage with the same parameters as the original can be created, even if the returned type is unknown!
In this way we can keep the new image always in the same format as the original and have no conversion issues giving the increase in brightness.
Checked on a ZTE ROM but needs more testing, not verified on phone yet but resulting image files look good. Thought it would be good to share; just included the complete file, not too big.

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 18, 2015

Owner

Comment #7 originally posted by connor.tumbleson on 2013-07-08T16:19:47.000Z:

Thank you :)

It passed all my testing so far and seems to work great. I will run a few more tests and pass it to some testers and then merge it in.

I see no color changes on multiple decodes and rebuilds.

Thanks again.

Owner

iBotPeaches commented Mar 18, 2015

Comment #7 originally posted by connor.tumbleson on 2013-07-08T16:19:47.000Z:

Thank you :)

It passed all my testing so far and seems to work great. I will run a few more tests and pass it to some testers and then merge it in.

I see no color changes on multiple decodes and rebuilds.

Thanks again.

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 18, 2015

Owner

Comment #8 originally posted by connor.tumbleson on 2013-07-08T16:19:58.000Z:

<empty>

Owner

iBotPeaches commented Mar 18, 2015

Comment #8 originally posted by connor.tumbleson on 2013-07-08T16:19:58.000Z:

<empty>

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 18, 2015

Owner

Comment #9 originally posted by christiaan.van.dijk@hccnet.nl on 2013-07-08T19:22:32.000Z:

No problem; glad I could help to improve such a great tool! Keep it up!

Owner

iBotPeaches commented Mar 18, 2015

Comment #9 originally posted by christiaan.van.dijk@hccnet.nl on 2013-07-08T19:22:32.000Z:

No problem; glad I could help to improve such a great tool! Keep it up!

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 18, 2015

Owner

Comment #10 originally posted by connor.tumbleson on 2013-07-08T21:11:49.000Z:

Fixed in commit: 63b0dd1

Owner

iBotPeaches commented Mar 18, 2015

Comment #10 originally posted by connor.tumbleson on 2013-07-08T21:11:49.000Z:

Fixed in commit: 63b0dd1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment