[OpenJDK 2D-Dev] <AWT Dev> RFR: 8081315: 8077982 giflib upgrade breaks system giflib builds with earlier versions

[Andrew Hughes]

----- 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,
-- 
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