Indexed PNG

classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

Indexed PNG

chrtom
I've extended the code of PNGImage.java to support indexed PNG files. I
needed this to be able to load map tiles from OpenStreetMap.

I've revised the constructor of PNGImage to take the necessary steps to
resolve the palette data with the helper methods of pngj.

Maybe you are interested in integrating this feature in future version
of JOGL. A patch is attached.

PNGImage.patch (9K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

gouessej
Administrator
Thank you. I thought it was already supported.

Edit.: Your patch may require some more work.
Julien Gouesse | Personal blog | Website
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

Sven Gothel
Administrator
In reply to this post by chrtom
On 03/01/2013 12:31 PM, ct [via jogamp] wrote:
> I've extended the code of PNGImage.java to support indexed PNG files. I
> needed this to be able to load map tiles from OpenStreetMap.
>
> I've revised the constructor of PNGImage to take the necessary steps to
> resolve the palette data with the helper methods of pngj.
>
> Maybe you are interested in integrating this feature in future version
> of JOGL. A patch is attached.
>

Thank you.

[Note: Patches w/o white space changes are better to read for differences]

I will review your patch and put it into consideration w/ Hernan's solution
  http://forum.jogamp.org/Able-to-disable-JOGL-s-PNG-decoder-tp4027701p4027739.html

~Sven



signature.asc (911 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

gouessej
Administrator
Sven, do you plan to use Hernan's solution? I can't remove all AWT dependencies from TUER because of this limitation :s
Julien Gouesse | Personal blog | Website
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

Sven Gothel
Administrator
On 03/11/2013 10:29 PM, gouessej [via jogamp] wrote:
> Sven, do you plan to use Hernan's solution?
Yes .. and I will also consider 'ct's patch.

> I can't remove all AWT
> dependencies from TUER because of this limitation :s

No problem, will be fixed.

I am still testing a solution for Bug 665, after that I will do this one.

~Sven



signature.asc (911 bytes) Download Attachment
ct
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

ct
Basically, my patch is based on a test case from PNGJ, as mentioned by Hernan.

The only thing one needs to do is to convert a scanline of palette indices to a scanline of colors. In my patch I've done exactly that by using PNGJ's helper classes. Further, the getPixelRGBA8 method now takes a scanline of int[], rather than of class ImageLine. This allows me to handle scanlines uniformly. I also cleaned up the PNGImage constructor a bit.
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

gouessej
Administrator
I have just modified your patch a little bit:
https://github.com/gouessej/jogl/commit/9fb623ef3c3feb30200c512e09b3507cccb76602
Julien Gouesse | Personal blog | Website
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

Sven Gothel
Administrator
On 03/13/2013 09:58 PM, gouessej [via jogamp] wrote:
> I have just modified your patch a little bit:
> https://github.com/gouessej/jogl/commit/9fb623ef3c3feb30200c512e09b3507cccb76602

Thank you - it has no white space changes, hence enhance to read the diff.

So I assume CT's change is almost the same here - since I cannot diff
w/o having to do lots of work (pull merge and diff w/o white space change).

Looks good, similar to Hernan's code
  https://code.google.com/p/pngj/source/browse/pnjg/src/ar/com/hjg/pngj/test/SampleConvPalToTrueColor.java

Do we have a unit test ?

Could you extend 'TestPNGTextureFromFileNEWT', which currently tests:

  normal           test-ntscN01-160x90.png
  interlaced       test-ntscI01-160x90.png
  interlaced-gamma test-ntscIG01-160x90.png

and add for indexed:
  palette RGB      test-ntscP301-160x90.png
  palette RGBA     test-ntscP401-160x90.png

maybe we could also need (while we are at):

  normal RGBA       test-ntscN401-160x90.png
  interlaced RGBA   test-ntscI401-160x90.png

Please tell me if you like to do so, otherwise I will add the above
after the merge and test.

~Sven



signature.asc (911 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

gouessej
Administrator
I can do that in about ten hours. Yes, the fix I created by using ct's patch uses the same helper than Hernan's code.
Julien Gouesse | Personal blog | Website
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

gouessej
Administrator
In reply to this post by Sven Gothel
This is in the pull request 60.
Julien Gouesse | Personal blog | Website
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

gouessej
Administrator
In reply to this post by Sven Gothel
The unit test seems to fail, there is still something wrong.
Julien Gouesse | Personal blog | Website
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

Sven Gothel
Administrator
On 03/15/2013 10:41 AM, gouessej [via jogamp] wrote:
> The unit test seems to fail, there is still something wrong.

Even though that is not good - great that the unit test catches it.

~Sven



signature.asc (911 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

gouessej
Administrator
I suspect it was already not working before modifying the code, I will give it another try.
Julien Gouesse | Personal blog | Website
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

Sven Gothel
Administrator
On 03/15/2013 04:04 PM, gouessej [via jogamp] wrote:
> I suspect it was already not working before modifying the code, I will give it
> another try.

The unit tests are ofc working as they are (w/o indexed PNG),
dunno what you mean.

~Sven


signature.asc (911 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

gouessej
Administrator
This post was updated on .
The loading of test-ntscP301-160x90.png fails and it prevents all tests from working.

Edit.: I have forgotten to add the files :s
Julien Gouesse | Personal blog | Website
Reply | Threaded
Open this post in threaded view
|

Re: Indexed PNG

gouessej
Administrator
In reply to this post by Sven Gothel
My latest commit fixes the remaining bug, it's ok now, you can test it again and merge :)
Julien Gouesse | Personal blog | Website