WorldWindEarth / WorldWindJava

A community supported fork of the NASA WorldWind Java SDK (WWJ) is for building cross-platform 3D geospatial desktop applications in Java.
https://worldwind.earth/WorldWindJava/
47 stars 13 forks source link

Gdal no alpha band fix #54

Closed gbburkhardt closed 5 years ago

gbburkhardt commented 5 years ago

Description of the Change

Pull request 42 introduced a bug. When source image file has no alpha band, the resulting dataset would have an alpha band that was all zeros, making it completely transparent. I believe that GDAL is handling alpha bands inconsistently in 'reprojectImage', but that's another investigation.

GDAL 3.0.0 has introduced a requirement for another environment variable to be set, due to its use of the PROJ.6 library. The GDAL_README.txt file has been updated to note this, and a check added to GDALUtils to warn the user if GDAL 3.0.0 is being used, and the required environment variable is not set.

GDAL_README.txt has been updated for clarity, and to remove the invalid reference to a NASA web page on WebStart.

There was a minor bug in the message formatting code that I've fixed. See Logging.java.

I've added a Netbeans configuration file that turns on logging when running sample applications using Gradle. I think it's important that logging be enabled by default so users can see what's happening. There's a JVM property that needs to be set. If anyone knows a better way to do this for Gradle, I'm open to it. Ideally it should be in the 'build.gradle' file, but I haven't been able to figure out how to do that for 'Run/Debug' file.

Why Should This Be In Core?

Bug fixes, better info/warning messages.

Benefits

Potential Drawbacks

Applicable Issues

gbburkhardt commented 5 years ago

This change to GDALDataRaster can be tested by generating a test file with no alpha band. The InstallImagery sample application is a good one to use. Use this command to create a test file: gdal_translate -b 1 -b 2 -b 3 craterlake-imagery-30m.tif craterlake-imagery-noalpha.tif Change the file used in the InstallImagery example. There should be no difference in the display than when using the original file, except for the background of the image, since there's no alpha band in the test file.

One can see what the contents of the alpha band are after the reprojectimage at about line 1117 in GDALDataRaster using code like:

gdal.ReprojectImage(srcDS, destDS, s_srs_wkt, t_srs_wkt, gdalconst.GRA_Average); projTime = System.currentTimeMillis() - start;

alpha = destDS.GetRasterBand(4); buf = new byte[roiWidth*roiHeight]; alpha.ReadRaster(0, 0, roiWidth, roiHeight, buf);

and then examine the contents of the alpha band in the destination dataset using a debugger.

wcmatthysen commented 5 years ago

@gbburkhardt, looks good and ready to merge.

gbburkhardt commented 5 years ago

Based on our discussion yesterday, I just revised the GDAL_README.txt file. Could you please pick up that change, too?

wcmatthysen commented 5 years ago

@gbburkhardt, did you commit the change? I already merged this branch. I can revert the merge if you want so that you can still add to this pull-request and then we do a final merge.

gbburkhardt commented 5 years ago

I made the commit minutes after you did the merge. But I must not have pushed it. So yes, please revert the merge. I won't be able to do the push for 6 hours or so. Thanks.

On Tue, Jun 25, 2019, 08:06 Wiehann Matthysen notifications@github.com wrote:

Did you commit the change? I already merged this branch. I can revert the merge if you want so that you can still add to this pull-request and then we do a final merge.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/WorldWindEarth/WorldWindJava/pull/54?email_source=notifications&email_token=ABI6HFE457FGJHJTGMDHJXDP4IC5RA5CNFSM4H2ZIOSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYQAMKA#issuecomment-505415208, or mute the thread https://github.com/notifications/unsubscribe-auth/ABI6HFFFIGUTQURT63OGTVDP4IC5RANCNFSM4H2ZIOSA .

wcmatthysen commented 5 years ago

I looked at how to do this and I found this explanation on how to revert a Github pull-request. It looks as though we are going to have to create a new pull-request to revert the merge. I suggest that instead of us going that route you just pull the changes from develop by running:

git pull origin develop

so that you are up-to-date and then create a new branch and pull-request for the new changes that you want in. You can then just go ahead and merge it with develop. The amount of effort is going to be less that way than undoing the merge and redoing everything again. Sorry about that.