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

[Alexander Zvegintsev]

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,