[Geoserver-devel] New PNG encoder: how about a GSIP to merge it into core on trunk?

Andrea_Aime4 · November 24, 2013, 5:34pm

Hi,
thinking about the new PNG encoder, I was wondering, what if I make a GSIP to include it
on trunk as the default PNG encoder (so move the code in the wms module),
replacing also the usual checkbox about choosing the
“native” PNG encoder with a combo box with a drop down having 3 options, jdk, native
or PNGJ.

Rationale:

the new PNG encoder is pure java, requires no extra installations, and provides both performance
and output size benefits
has a good amount of testing, with checks it can encode more color models than GeoServer
normally produces in output (all the ones contained in the PNG spec)
we still have three+ months ahead before the 2.5.0 release, so there is time to kick its tires
for any residual corner case the drop down choice will allow admins to switch to another
encoder

I understand this encoder is “new”, but so were the new KML module and the new shapefile
modules last release (so far it seems it went well for those two, only a few issues were reported),
and that’s precisely the reason for a GSIP.

Just wanted to test the waters and see if anyone would be strongly opposed to the move.

Cheers
Andrea

–

==
Our support, Your Success! Visit http://opensdi.geo-solutions.it for more information.

Ing. Andrea Aime

@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

jive · November 25, 2013, 10:28am

I would bee keen to see it land as an option in trunk. I would like it if we could strike a balance between discussion and breaking out a GSIP proposal.

I do not see this change as particularly strategic, destabilising, or as a scary API or UI change. It looks like exactly the kind of RnD we expect on trunk and think we could proceed based on discussion.

···

Jody Garnett

On Mon, Nov 25, 2013 at 4:34 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

Hi,
thinking about the new PNG encoder, I was wondering, what if I make a GSIP to include it
on trunk as the default PNG encoder (so move the code in the wms module),
replacing also the usual checkbox about choosing the
“native” PNG encoder with a combo box with a drop down having 3 options, jdk, native
or PNGJ.

Rationale:

the new PNG encoder is pure java, requires no extra installations, and provides both performance
and output size benefits

has a good amount of testing, with checks it can encode more color models than GeoServer
normally produces in output (all the ones contained in the PNG spec)

we still have three+ months ahead before the 2.5.0 release, so there is time to kick its tires

for any residual corner case the drop down choice will allow admins to switch to another
encoder

I understand this encoder is “new”, but so were the new KML module and the new shapefile
modules last release (so far it seems it went well for those two, only a few issues were reported),
and that’s precisely the reason for a GSIP.

Just wanted to test the waters and see if anyone would be strongly opposed to the move.

Cheers
Andrea

–

==
Our support, Your Success! Visit http://opensdi.geo-solutions.it for more information.

Ing. Andrea Aime

@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing
conversations that shape the rapidly evolving mobile landscape. Sign up now.
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk

Geoserver-devel mailing list
Geoserver-devel@anonymised.comsts.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Justin_Deoliveira · November 25, 2013, 3:59pm

No objection here to making it the default on trunk. Imo now is the best time to do it, while we are in dev phase.

···

On Mon, Nov 25, 2013 at 3:28 AM, Jody Garnett <jody.garnett@anonymised.com> wrote:

I would bee keen to see it land as an option in trunk. I would like it if we could strike a balance between discussion and breaking out a GSIP proposal.

I do not see this change as particularly strategic, destabilising, or as a scary API or UI change. It looks like exactly the kind of RnD we expect on trunk and think we could proceed based on discussion.

Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing
conversations that shape the rapidly evolving mobile landscape. Sign up now.
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk

Geoserver-devel mailing list
Geoserver-devel@anonymised.comsts.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

–

Justin Deoliveira
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive

Jody Garnett

On Mon, Nov 25, 2013 at 4:34 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

Hi,
thinking about the new PNG encoder, I was wondering, what if I make a GSIP to include it
on trunk as the default PNG encoder (so move the code in the wms module),
replacing also the usual checkbox about choosing the
“native” PNG encoder with a combo box with a drop down having 3 options, jdk, native
or PNGJ.

Rationale:

the new PNG encoder is pure java, requires no extra installations, and provides both performance
and output size benefits

has a good amount of testing, with checks it can encode more color models than GeoServer
normally produces in output (all the ones contained in the PNG spec)

we still have three+ months ahead before the 2.5.0 release, so there is time to kick its tires

for any residual corner case the drop down choice will allow admins to switch to another
encoder

I understand this encoder is “new”, but so were the new KML module and the new shapefile
modules last release (so far it seems it went well for those two, only a few issues were reported),
and that’s precisely the reason for a GSIP.

Just wanted to test the waters and see if anyone would be strongly opposed to the move.

Cheers
Andrea

–

==
Our support, Your Success! Visit http://opensdi.geo-solutions.it for more information.

Ing. Andrea Aime

@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing
conversations that shape the rapidly evolving mobile landscape. Sign up now.
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk

Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Ben_Caradoc-Davies · November 26, 2013, 2:57am

I am also +1 on making the new encoder the default on master. I think a GSIP is required.

I appreciate Jody's caution about making brand new code the default for core functionality; in general, a gradual transition is a good practice. However:
(1) this is only an implementation change and does not alter interface or formats,
(2) the new implementation offers benefits to pure Java users,
(3) making it the default on master makes adoption more likely, as non-default behaviour is often not tested, and
(4) the timing (well before 2.5.0) is just great! If we were a month out, I would be resistant about making this the default on master, but this time scale will give it a decent test.

Andrea also has support on the users list.

I would be +1 to make the new encoder an option on master right now without a GSIP.

Kind regards,
Ben.

On 25/11/13 23:59, Justin Deoliveira wrote:

No objection here to making it the default on trunk. Imo now is the best
time to do it, while we are in dev phase.

On Mon, Nov 25, 2013 at 3:28 AM, Jody Garnett <jody.garnett@anonymised.com
<mailto:jody.garnett@anonymised.com>> wrote:

    I would bee keen to see it land as an option in trunk. I would like
    it if we could strike a balance between discussion and breaking out
    a GSIP proposal.

    I do not see this change as particularly strategic, destabilising,
    or as a scary API or UI change. It looks like exactly the kind of
    RnD we expect on trunk and think we could proceed based on discussion.

    Jody Garnett

    On Mon, Nov 25, 2013 at 4:34 AM, Andrea Aime
    <andrea.aime@anonymised.com <mailto:andrea.aime@anonymised.com>>
    wrote:

        Hi,
        thinking about the new PNG encoder, I was wondering, what if I
        make a GSIP to include it
        on trunk as the default PNG encoder (so move the code in the wms
        module),
        replacing also the usual checkbox about choosing the
        "native" PNG encoder with a combo box with a drop down having 3
        options, jdk, native
        or PNGJ.

        Rationale:
        * the new PNG encoder is pure java, requires no extra
        installations, and provides both performance
           and output size benefits
        * has a good amount of testing, with checks it can encode more
        color models than GeoServer
           normally produces in output (all the ones contained in the
        PNG spec)
        * we still have three+ months ahead before the 2.5.0 release, so
        there is time to kick its tires
        * for any residual corner case the drop down choice will allow
        admins to switch to another
           encoder

        I understand this encoder is "new", but so were the new KML
        module and the new shapefile
        modules last release (so far it seems it went well for those
        two, only a few issues were reported),
        and that's precisely the reason for a GSIP.

        Just wanted to test the waters and see if anyone would be
        strongly opposed to the move.

        Cheers
        Andrea

        --
        ==
        Our support, Your Success! Visit http://opensdi.geo-solutions.it
        for more information.
        ==

        Ing. Andrea Aime
        @geowolf
        Technical Lead

        GeoSolutions S.A.S.
        Via Poggio alle Viti 1187
        55054 Massarosa (LU)
        Italy
        phone: +39 0584 962313 <tel:%2B39%200584%20962313>
        fax: +39 0584 1660272 <tel:%2B39%200584%201660272>
        mob: +39 339 8844549 <tel:%2B39%20%C2%A0339%208844549>

        http://www.geo-solutions.it
        http://twitter.com/geosolutions_it

        -------------------------------------------------------

        ------------------------------------------------------------------------------
        Shape the Mobile Experience: Free Subscription
        Software experts and developers: Be at the forefront of tech
        innovation.
        Intel(R) Software Adrenaline delivers strategic insight and
        game-changing
        conversations that shape the rapidly evolving mobile landscape.
        Sign up now.
        http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
        _______________________________________________
        Geoserver-devel mailing list
        Geoserver-devel@lists.sourceforge.net
        <mailto:Geoserver-devel@lists.sourceforge.net>
        https://lists.sourceforge.net/lists/listinfo/geoserver-devel

    ------------------------------------------------------------------------------
    Shape the Mobile Experience: Free Subscription
    Software experts and developers: Be at the forefront of tech innovation.
    Intel(R) Software Adrenaline delivers strategic insight and
    game-changing
    conversations that shape the rapidly evolving mobile landscape. Sign
    up now.
    http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
    _______________________________________________
    Geoserver-devel mailing list
    Geoserver-devel@lists.sourceforge.net
    <mailto:Geoserver-devel@lists.sourceforge.net>
    https://lists.sourceforge.net/lists/listinfo/geoserver-devel

--
*Justin Deoliveira*
Vice President, Engineering | Boundless
jdeolive@anonymised.com <mailto:jdeolive@anonymised.com>
@j_deolive <https://twitter.com/j_deolive>

------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing
conversations that shape the rapidly evolving mobile landscape. Sign up now.
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk

_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

--
Ben Caradoc-Davies <Ben.Caradoc-Davies@anonymised.com>
Software Engineer
CSIRO Earth Science and Resource Engineering
Australian Resources Research Centre

Simon_Hartley · November 29, 2013, 11:06am

When playing with the code I created a 4 bit grayscale image.

The reason I use ushort is because it results in better quality images than if I use byte (I had issues with lines not drawing smoothly).

BufferedImage bi = ImageTypeSpecifier

.createGrayscale(4, DataBuffer.TYPE_USHORT, false)

.createBufferedImage(width, height);

The ScanlineProviderFactory didn’t like this because of the use of USHORT in combination with an IndexColorModel.

One thing I needed to do to get it to work is create a constructor on RasterShortSingleBandProvider which takes a bit depth (4 in this case).

Looking at the code, can the final <else if> ever work?

The previous <else if> checks if the ColorModel is an instance of IndexColorModel and then the next <else if>,
knowing that it isn’t an IndexColorModel, casts the ColorModel to IndexColorModel.

I hope this helps.

Simon Hartley

.

From: Andrea Aime [mailto:andrea.aime@anonymised.com]
Sent: 24 November 2013 17:34
To: Geoserver-devel
Subject: [Geoserver-devel] New PNG encoder: how about a GSIP to merge it into core on trunk?

Hi,

thinking about the new PNG encoder, I was wondering, what if I make a GSIP to include it

on trunk as the default PNG encoder (so move the code in the wms module),

replacing also the usual checkbox about choosing the

“native” PNG encoder with a combo box with a drop down having 3 options, jdk, native

or PNGJ.

Rationale:

the new PNG encoder is pure java, requires no extra installations, and provides both performance

and output size benefits

has a good amount of testing, with checks it can encode more color models than GeoServer

normally produces in output (all the ones contained in the PNG spec)

we still have three+ months ahead before the 2.5.0 release, so there is time to kick its tires
for any residual corner case the drop down choice will allow admins to switch to another

encoder

I understand this encoder is “new”, but so were the new KML module and the new shapefile

modules last release (so far it seems it went well for those two, only a few issues were reported),

and that’s precisely the reason for a GSIP.

Just wanted to test the waters and see if anyone would be strongly opposed to the move.

Cheers

Andrea

–

==

Our support, Your Success! Visit http://opensdi.geo-solutions.it for more information.

==

Ing. Andrea Aime

@geowolf

Technical Lead

GeoSolutions S.A.S.

Via Poggio alle Viti 1187

55054 Massarosa (LU)

Italy

phone: +39 0584 962313

fax: +39 0584 1660272

mob: +39 339 8844549

http://www.geo-solutions.it

http://twitter.com/geosolutions_it

*** This communication has been sent from World Fuel Services
Corporation or its subsidiaries or its affiliates for the intended recipient
only and may contain proprietary, confidential or privileged information.
If you are not the intended recipient, any review, disclosure, copying,
use, or distribution of the information included in this communication
and any attachments is strictly prohibited. If you have received this
communication in error, please notify us immediately by replying to this
communication and delete the communication, including any
attachments, from your computer. Electronic communications sent to or
from World Fuel Services Corporation or its subsidiaries or its affiliates
may be monitored for quality assurance and compliance purposes.***

Andrea_Aime4 · November 29, 2013, 11:16am

On Fri, Nov 29, 2013 at 12:06 PM, Simon Hartley <SHartley@anonymised.com>wrote:

When playing with the code I created a 4 bit grayscale image.

The reason I use ushort is because it results in better quality images
than if I use byte (I had issues with lines not drawing smoothly).

BufferedImage bi = ImageTypeSpecifier

.createGrayscale(4,
DataBuffer.TYPE_USHORT, false)

.createBufferedImage(width, height);

The ScanlineProviderFactory didn’t like this because of the use of USHORT
in combination with an IndexColorModel.

One thing I needed to do to get it to work is create a constructor on
RasterShortSingleBandProvider which takes a bit depth (4 in this case).

Weird setup indeed. Yes, what you propose should work.
More in general, this PNG encoder should probably have a fallback mode, if
we cannot create a scanline provider,
it should fall back on the JDK one I guess, to make sure it does not break
when plugging custom raster sources
like yours.

Looking at the code, can the final <*else if*> ever work?

The previous <*else if> *checks if the ColorModel is an instance of
IndexColorModel and then the next <*else if>*,
knowing that it isn’t an IndexColorModel, casts the ColorModel to
IndexColorModel.

No, you're right, it does not. Weird, I guess none of the official PNG
color models ever get there

Cheers
Andrea

--

Our support, Your Success! Visit http://opensdi.geo-solutions.it for more
information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

Simon_Hartley · November 29, 2013, 2:04pm

Just for some insight:

I was trying to optimise file size by using a 4 bit palette and this was a suitable and simple way of doing so.

Another tip I found was to avoid the use of anti-aliasing, because it ends up generating intermediate colours, which makes PNG compression more difficult even though the palette bit-depth hasn’t changed.

When testing with native JAI and I/O, I found that when using a non-standard case, Sun/Oracle don’t seem to bother optimising. Judging by the file sizes, they fall back to non-native.

Simon

.

From: andrea.aime@anonymised.com [mailto:andrea.aime@…403…] On Behalf Of Andrea Aime
Sent: 29 November 2013 11:16
To: Simon Hartley
Cc: Geoserver-devel
Subject: Re: [Geoserver-devel] New PNG encoder: how about a GSIP to merge it into core on trunk?

On Fri, Nov 29, 2013 at 12:06 PM, Simon Hartley <SHartley@anonymised.com> wrote:

When playing with the code I created a 4 bit grayscale image.

The reason I use ushort is because it results in better quality images than if I use byte (I had issues with lines not drawing smoothly).

BufferedImage bi = ImageTypeSpecifier

.createGrayscale(4, DataBuffer.TYPE_USHORT, false)

.createBufferedImage(width, height);

The ScanlineProviderFactory didn’t like this because of the use of USHORT in combination with an IndexColorModel.

One thing I needed to do to get it to work is create a constructor on RasterShortSingleBandProvider which takes a bit depth (4 in this case).

Weird setup indeed. Yes, what you propose should work.

More in general, this PNG encoder should probably have a fallback mode, if we cannot create a scanline provider,

it should fall back on the JDK one I guess, to make sure it does not break when plugging custom raster sources

like yours.

Looking at the code, can the final <else if> ever work?

The previous <else if> checks if the ColorModel is an instance of IndexColorModel and then the next <else if>,
knowing that it isn’t an IndexColorModel, casts the ColorModel to IndexColorModel.

No, you’re right, it does not. Weird, I guess none of the official PNG color models ever get there

Cheers

Andrea

–

==

Our support, Your Success! Visit http://opensdi.geo-solutions.it for more information.

==

Ing. Andrea Aime

@geowolf

Technical Lead

GeoSolutions S.A.S.

Via Poggio alle Viti 1187

55054 Massarosa (LU)

Italy

phone: +39 0584 962313

fax: +39 0584 1660272

mob: +39 339 8844549

http://www.geo-solutions.it

http://twitter.com/geosolutions_it

*** This communication has been sent from World Fuel Services
Corporation or its subsidiaries or its affiliates for the intended recipient
only and may contain proprietary, confidential or privileged information.
If you are not the intended recipient, any review, disclosure, copying,
use, or distribution of the information included in this communication
and any attachments is strictly prohibited. If you have received this
communication in error, please notify us immediately by replying to this
communication and delete the communication, including any
attachments, from your computer. Electronic communications sent to or
from World Fuel Services Corporation or its subsidiaries or its affiliates
may be monitored for quality assurance and compliance purposes.***