[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] <AWT Dev> RFR: 8081315: 8077982 giflib upgrade breaks system giflib builds with
From:       Andrew Hughes <gnu.andrew () redhat ! com>
Date:       2015-06-08 15:50:31
Message-ID: 1250294060.12499386.1433778631021.JavaMail.zimbra () redhat ! com
[Download RAW message or body]

----- Original Message -----
> Sure, looks good to me.
> 
> Thanks,
> 
> Alexander.
> 
> On 06/05/2015 06:34 PM, Andrew Hughes wrote:
> > ----- Original Message -----
> > > Hi Andrew,
> > > 
> > > We have removed a workaround[0] for interlaced images support, there is
> > > no need in it after 5.0.0 [1]:
> > > > DGifSlurp() and EGifSpew() now read and write interlaced images properly.
> > > I didn't test it, but it seems to me that interlaced images will look
> > > bad after building against GIFLIB < 5.
> > > So I suggest to revert SplashDecodeGif() to its state before GIFLIB
> > > upgrade and make "interlaced if" conditional[3]:
> > > 
> > > diff -r 58535e641739
> > > src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c
> > > --- a/src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c
> > > +++ b/src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c
> > > @@ -213,16 +213,14 @@ SplashDecodeGif(Splash * splash, GifFile
> > > byte_t *pSrc = image->RasterBits;
> > > ImageFormat srcFormat;
> > > ImageRect srcRect, dstRect;
> > > -            int pass, npass;
> > > +            int pass = 4, npass = 5;
> > > 
> > > +#if GIFLIB_MAJOR < 5
> > > if (desc->Interlace) {
> > > pass = 0;
> > > npass = 4;
> > > }
> > > -            else {
> > > -                pass = 4;
> > > -                npass = 5;
> > > -            }
> > > +#endif
> > > 
> > > srcFormat.colorMap = colorMapBuf;
> > > srcFormat.depthBytes = 1;
> > > 
> > > And again, I haven't tested it against GIFLIB 4.*.
> > > 
> > > [0]
> > > http://cr.openjdk.java.net/~azvegint/jdk/9/8077982/00/src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c.sdiff.html
> > >  [1] http://sourceforge.net/p/giflib/code/ci/master/tree/NEWS
> > > [3] http://cr.openjdk.java.net/~azvegint/jdk/9/8081315/03/
> > > 
> > > Thanks,
> > > 
> > > Alexander.
> > > 
> > Thanks for the heads up on this. It's not very obvious from the code.
> > 
> > I went back through the original 8077982 changes to that file and
> > produced the resulting webrev:
> > 
> > http://cr.openjdk.java.net/~andrew/8081315/webrev.03/
> > 
> > Comparing with the original (giflib < 5) splashscreen_gif.c now gives:
> > 
> > @@ -213,16 +213,16 @@
> > byte_t *pSrc = image->RasterBits;
> > ImageFormat srcFormat;
> > ImageRect srcRect, dstRect;
> > -            int pass, npass;
> > +            int pass = 4, npass = 5;
> > 
> > +#if GIFLIB_MAJOR < 5
> > +	    /* Interlaced gif support is broken in giflib < 5
> > +	       so we need to work around this */
> > if (desc->Interlace) {
> > pass = 0;
> > npass = 4;
> > }
> > -            else {
> > -                pass = 4;
> > -                npass = 5;
> > -            }
> > +#endif
> > 
> > srcFormat.colorMap = colorMapBuf;
> > srcFormat.depthBytes = 1;
> > 
> > much as you have above.
> > 
> > Ok to push this version?
> > 
> > Thanks,
> 
> 

Thanks. Pushed:

http://hg.openjdk.java.net/jdk9/client/jdk/rev/f10c6da6698b
-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222

PGP Key: rsa4096/248BDC07 (hkp://keys.gnupg.net)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07


[prev in list] [next in list] [prev in thread] [next in thread]