Null-Globe bug in FlyToOrbitViewAnimator

[wcmatthysen]

There is a bug in the FlyToOrbitViewAnimator that causes a NullPointerException during startup. I've seen this issue manifest itself in the gov.nasa.worldwindx.examples.dataimport.ImportImagery example application. However, the issue only pops up when I set the GDAL_DATA and GDAL_PLUGINS manually in the launch script. The script I'm using is:

#!/bin/sh

export GDAL_DATA=/usr/share/gdal/2.2
export GDAL_PLUGINS=/usr/lib/gdalplugins
export LD_LIBRARY_PATH=/usr/lib/jni:/usr/lib/grass74/lib

java -Xmx1024m -Djava.library.path=/usr/lib/jni -classpath ./worldwind.jar:./worldwindx.jar:/usr/share/java/gdal.jar:./jogl-all.jar:./gluegen-rt.jar gov.nasa.worldwindx.examples.dataimport.ImportImagery

This results in the following exception being printed out continuously:

Jun 25, 2019 1:53:19 PM gov.nasa.worldwind.WorldWindowGLAutoDrawable display
SEVERE: Exception while attempting to repaint WorldWindow
java.lang.NullPointerException
        at gov.nasa.worldwind.view.orbit.FlyToOrbitViewAnimator$OnSurfacePositionAnimator.nextPosition(FlyToOrbitViewAnimator.java:124)
        at gov.nasa.worldwind.animation.PositionAnimator.setImpl(PositionAnimator.java:78)
        at gov.nasa.worldwind.animation.BasicAnimator.set(BasicAnimator.java:66)
        at gov.nasa.worldwind.animation.CompoundAnimator.setImpl(CompoundAnimator.java:89)
        at gov.nasa.worldwind.animation.BasicAnimator.set(BasicAnimator.java:66)
        at gov.nasa.worldwind.animation.BasicAnimator.next(BasicAnimator.java:55)
        at gov.nasa.worldwind.animation.AnimationController.stepAnimators(AnimationController.java:78)
        at gov.nasa.worldwind.view.orbit.OrbitViewInputHandler.apply(OrbitViewInputHandler.java:522)
        at gov.nasa.worldwind.view.BasicView.apply(BasicView.java:172)
        at gov.nasa.worldwind.AbstractSceneController.applyView(AbstractSceneController.java:538)
        at gov.nasa.worldwind.BasicSceneController.doNormalRepaint(BasicSceneController.java:40)
        at gov.nasa.worldwind.BasicSceneController.doRepaint(BasicSceneController.java:30)
        at gov.nasa.worldwind.AbstractSceneController.repaint(AbstractSceneController.java:400)
        at gov.nasa.worldwind.WorldWindowGLAutoDrawable.doDisplay(WorldWindowGLAutoDrawable.java:459)
        at gov.nasa.worldwind.WorldWindowGLAutoDrawable.display(WorldWindowGLAutoDrawable.java:332)
        at jogamp.opengl.GLDrawableHelper.displayImpl(GLDrawableHelper.java:692)
        at jogamp.opengl.GLDrawableHelper.display(GLDrawableHelper.java:674)
        at com.jogamp.opengl.awt.GLCanvas$11.run(GLCanvas.java:1424)
        at jogamp.opengl.GLDrawableHelper.invokeGLImpl(GLDrawableHelper.java:1293)
        at jogamp.opengl.GLDrawableHelper.invokeGL(GLDrawableHelper.java:1147)
        at com.jogamp.opengl.awt.GLCanvas$12.run(GLCanvas.java:1438)
        at com.jogamp.opengl.Threading.invoke(Threading.java:223)
        at com.jogamp.opengl.awt.GLCanvas.display(GLCanvas.java:505)
        at com.jogamp.opengl.awt.GLCanvas.paint(GLCanvas.java:559)
        at com.jogamp.opengl.awt.GLCanvas.update(GLCanvas.java:866)
        at sun.awt.RepaintArea.updateComponent(RepaintArea.java:255)
        at sun.awt.X11.XRepaintArea.updateComponent(XRepaintArea.java:51)
        at sun.awt.RepaintArea.paint(RepaintArea.java:232)
        at sun.awt.X11.XComponentPeer.handleEvent(XComponentPeer.java:584)
        at java.awt.Component.dispatchEventImpl(Component.java:4965)
        at java.awt.Component.dispatchEvent(Component.java:4711)
        at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:760)
        at java.awt.EventQueue.access$500(EventQueue.java:97)
        at java.awt.EventQueue$3.run(EventQueue.java:709)
        at java.awt.EventQueue$3.run(EventQueue.java:703)
        at java.security.AccessController.doPrivileged(Native Method)
        at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
        at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:84)
        at java.awt.EventQueue$4.run(EventQueue.java:733)
        at java.awt.EventQueue$4.run(EventQueue.java:731)
        at java.security.AccessController.doPrivileged(Native Method)
        at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
        at java.awt.EventQueue.dispatchEvent(EventQueue.java:730)
        at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:205)
        at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
        at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
        at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
        at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
        at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)

I've dug around in the code a bit and I believe that it is a timing issue that causes this bug. When the environment variables are not set, the GDAL initialisation code attempts to search for the correct values for the GDAL_DATA and GDAL_PLUGINS variables. This search causes enough time to go by so that the View can be initialised resulting in no breakage. Thus, I think I've traced the issue to the FlyToOrbitViewAnimator class itself. The bugfix is in PR #56.

[wcmatthysen]

@gbburkhardt, I don't know if you have also run into this bug. I'm not sure if it is just a Linux issue.

[gbburkhardt]

No, I haven't seen it. I have noticed that the recursive search for the data or plugins directories can take a long time on Linux because one of the of the search paths is "/usr/lib". A quick fix that only addresses the timing issue but not the underlying synchronization would be to replace "/usr/lib" with "/usr/lib/gdalplugins". I'll have to look at the original library load mechanism to see if it would have prevented a timing issue like this, or if a new one needs to be created.

If you use the "GDAL_DRIVER_PATH" environment variable instead of "GDAL_PLUGINS", you shouldn't see the error.

[wcmatthysen]

@gbburkhardt, you are right, I see that I incorrectly used GDAL_PLUGINS. If I change the script to use GDAL_DRIVER_PATH instead (as you mentioned):

#!/bin/sh

export GDAL_DATA=/usr/share/gdal/2.2
export GDAL_DRIVER_PATH=/usr/lib/gdalplugins
export LD_LIBRARY_PATH=/usr/lib/jni:/usr/lib/grass74/lib

java -Xmx1024m -Djava.library.path=/usr/lib/jni -classpath ./worldwind.jar:./worldwindx.jar:/usr/share/java/gdal.jar:./jogl-all.jar:./gluegen-rt.jar gov.nasa.worldwindx.examples.dataimport.ImportImagery

then I still get the NullPointerException. However, if I comment out the GDAL_DATA variable the example starts up fine. I don't think the issue is on the GDAL side itself. I think the example causes the GDAL initialisation code to run allowing for the timing issues to show up which causes the bug in FlyToOrbitViewAnimator to appear. We can still add more specific search paths for the GDAL startup logic, but I think it won't fix this issue. The issue is with the View having a null Globe at startup, and when the FlyToOrbitViewAnimator is used to specify an initial position it gets this null Globe (if queried too early) which causes it to break.

[gbburkhardt]

Ah, I misunderstood the timing problem.  When the variables are properly defined, the GDAL code runs faster than if they are not, and then the error occurs.  I'll take a look at your fix tonight or tomorrow.  We typically have been running this code without the variables defined, so that introduces a bit of delay.

…

On 6/26/2019 3:48 AM, Wiehann Matthysen wrote: @gbburkhardt <https://github.com/gbburkhardt>, you are right, I see that I incorrectly used |GDAL_PLUGINS|. If I change the script to use |GDAL_DRIVER_PATH| instead (as you mentioned): |#!/bin/sh export GDAL_DATA=/usr/share/gdal/2.2 export GDAL_DRIVER_PATH=/usr/lib/gdalplugins export LD_LIBRARY_PATH=/usr/lib/jni:/usr/lib/grass74/lib java -Xmx1024m -Djava.library.path=/usr/lib/jni -classpath ./worldwind.jar:./worldwindx.jar:/usr/share/java/gdal.jar:./jogl-all.jar:./gluegen-rt.jar gov.nasa.worldwindx.examples.dataimport.ImportImagery | then I still get the |NullPointerException|. However, if I comment out the |GDAL_DATA| variable the example starts up fine. I don't think the issue is on the GDAL side itself. I think the example causes the GDAL initialisation code to run allowing for the timing issues to show up which causes the bug in |FlyToOrbitViewAnimator| to appear. We can still add more specific search paths for the GDAL startup logic, but I think it won't fix this issue. The issue is with the |View| having a null |Globe| at startup, and when the |FlyToOrbitViewAnimator| is used to specify an initial position it gets this null |Globe| (if queried too early) which causes it to break. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#55?email_source=notifications&email_token=ABI6HFHKHUNE7PRHSNU2W6DP4MNLBA5CNFSM4H3HQQJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYSU6CQ#issuecomment-505761546>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABI6HFFLOD36XLZXRQZLA3LP4MNLBANCNFSM4H3HQQJA>.

[gbburkhardt]

I've tried running this on my Ubuntu 18.04 system. I don't see the exception occurring, with both variables defined, so there's no searching of the directories. But it's an old system, probably slower than yours. It appears to me that the GDAL initialization will be complete when the call to installSurfaceImage returns (about line 76 in InstallImagery.java). The code you show having the exception won't run until ExampleUtil.goTo() is called, and that's guaranteed to be after GDAL initialization.

I need to spend some more time on this. I can't find where the 'globe' gets set in BasicView. It looks like all the WorldWindow stuff and drawing context gets set up in AppFrame before the application code in InstallImagery is run.

I worry about the fix you propose since during the calls that 'globe' is null in nextPosition(), the intermediate elevations won't be computed.

[gbburkhardt]

So, what I think is happening (on your very fast machine) is the thread that's created in InstallImagery.AppFrame:53 is getting to the SwingUtilities.invokeLater() call at line 81 before the AppFrame gets to the EventQueue.invokeLater in ApplicationTemplate.AppFrame:312. If the latter gets to post its event first, the frame will be set visible, and the globe will be create first.

I think what you really want to do is rearrange the AppFrame code so that it's guaranteed that the event that sets the frame visible is posted before the thread that runs InstallImagery:installImagery() is created.

That problem probably exists in all of these example applications. The fix your proposing addresses a symptom, but not the underlying structural flaw. Let's try to fix that.

[gbburkhardt]

If you have time, try the changes in this branch. I hope that they fix the problem. You must have a very fast machine and a fast disks. For the problem you've reported, the complete production of the image file has to complete, so there's a significant amount of disk IO before the ExampleUtil.goTo(getWwd(), sector); call in InstallImagery.

I'm pretty sure that the change from 'invokeLater' to 'invokeAndWait' is required. The paint event that finally triggers the first setting of 'globe' in BasicOrbitView.doApply():633 doesn't get generated until a lot of the components of the frame are constructed.

[wcmatthysen]

@gbburkhardt, I'll try it out and let you know.

[wcmatthysen]

@gbburkhardt, I tested out the proposed fix you made in the Null-Globe branch. However, I'm picking up the bug in the ImportImagery example and not the InstallImagery one. I changed the ImportImagery example by adding similar changes to the ones you added to InstallImagery (i.e. I moved the thread-initialisation logic to a start() method that overrides the ApplicationTemplate.AppFrame.start() method as follows:

public class ImportImagery extends ApplicationTemplate
{
    // The data to import.
    protected static final String IMAGE_PATH = "gov/nasa/worldwindx/examples/data/craterlake-imagery-30m.tif";

    public static class AppFrame extends ApplicationTemplate.AppFrame
    {
        public AppFrame()
        {
            // Show the WAIT cursor because the import may take a while.
            this.setCursor(new Cursor(Cursor.WAIT_CURSOR));
        }

        @Override
        public void start()
        {
            // Import the imagery on a thread other than the event-dispatch thread to avoid freezing the UI.
            Thread t = new Thread(new Runnable()
            {
                public void run()
                {
                    importImagery();

                    // Restore the cursor.
                    setCursor(Cursor.getDefaultCursor());
                }
            });

            t.start();
        }

Unfortunately, I'm still getting the abovementioned exception. The only way it works is if I add a Thread.sleep() just before the call to importImagery(). Thus, I have to add a delay before the thread calls importImagery() to avoid the exception. I suspect the invokeAndWait is not enough to ensure that the View is properly initialised before the call to ExampleUtil.goTo() is made. The frame is only set to visible inside this invokeAndWait and the View might just not be ready with a non-null Globe.

I have seen this issue pop up numerous times in production on systems I'm working on. I've had to resort to building a custom version of WorldWind just to fix this or else I sometimes get this exception at startup if the timing is wrong. This leaves me with a broken globe that is completely greyed-out. I suspect it is a timing issue where in certain circumstances; especially when you try to do a go-to animation to a specific position at startup; the View doesn't have a Globe ready when FlyToOrbitViewAnimator tries to advance the view. In fact, if you look the javadoc of the View.getGlobe() method you'll see:

/**
 * Gets the globe associated with this view. The Globe is updated at the beginning of each frame, during
 * <code>View.apply()</code>. <code>View.getGlobe()</code> returns null if called before the first invocation of
 * <code>View.apply()</code>.
 *
 * @return the globe being rendered by this view, or null before the first invocation of <code>View.apply()</code>.
 */
Globe getGlobe();

So, I don't know what the best fix is for this. In my opinion the FlyToOrbitViewAnimator should be robust enough to deal with a potential null Globe. If I add the fixes I made on my branch, then this fixes the issue completely. However, I get that this should be sanity checked a bit because the fix in my branch will result in elevations to be completely ignored if the Globe is null which might not be the correct behaviour.

To trace this issue you can look at the OrbitViewInputHandler.goTo() method. This is the method that gets called at the end of the chain when invoking ExampleUtil.goTo(). If you look at OrbitViewInputHandler.goTo() you'll see it calls addPanToAnimator() which in turn calls FlyToOrbitViewAnimator.createFlyToOrbitViewAnimator(). This method creates an OnSurfacePositionAnimator with a Globe parameter (by calling OrbitView.getGlobe()) that is null because the getGlobe() method was called before the first View.apply() call. That is why I attempted to move the getGlobe() call to the OnSurfacePositionAnimator by passing it the OrbitView and relying on it to get the Globe itself from the OrbitView in the nextPosition() method.

[wcmatthysen]

@gbburkhardt, I've looked at the changes I made to the FlyToOrbitViewAnimator class. If I remove the null-check for the Globe in the nextPosition() method, everything still works. So, the Globe should be non-null when the nextPosition() method is executed. Thus, just by calling getGlobe() at a later stage, instead of calling it earlier in the createFlyToOrbitViewAnimator() method fixes the issue. The OnSurfacePositionAnimator now looks like:

protected static class OnSurfacePositionAnimator extends PositionAnimator
{
    OrbitView orbitView;
    int altitudeMode;
    boolean useMidZoom = true;

    public OnSurfacePositionAnimator(OrbitView orbitView, Interpolator interpolator,
        Position begin,
        Position end,
        PropertyAccessor.PositionAccessor propertyAccessor, int altitudeMode)
    {
        super(interpolator, begin, end, propertyAccessor);
        this.orbitView = orbitView;
        this.altitudeMode = altitudeMode;
    }

    @Override
    protected Position nextPosition(double interpolant)
    {
        final int MAX_SMOOTHING = 1;

        final double CENTER_START = this.useMidZoom ? 0.2 : 0.0;
        final double CENTER_STOP = this.useMidZoom ? 0.8 : 0.8;
        double latLonInterpolant = AnimationSupport.basicInterpolant(interpolant, CENTER_START, CENTER_STOP,
            MAX_SMOOTHING);

        // Invoke the standard next position functionality.
        Position pos = super.nextPosition(latLonInterpolant);

        // Check the altitude mode. If the altitude mode depends on the surface elevation we will reevaluate the
        // end position altitude. When the animation starts we may not have accurate elevation data available for
        // the end position, so recalculating the elevation as we go ensures that the animation will end at the
        // correct altitude.
        double endElevation = 0.0;
        boolean overrideEndElevation = false;
        
        Globe globe = this.orbitView.getGlobe();

        if (this.altitudeMode == WorldWind.CLAMP_TO_GROUND)
        {
            overrideEndElevation = true;
            endElevation = globe.getElevation(getEnd().getLatitude(), getEnd().getLongitude());
        }
        else if (this.altitudeMode == WorldWind.RELATIVE_TO_GROUND)
        {
            overrideEndElevation = true;
            endElevation = globe.getElevation(getEnd().getLatitude(), getEnd().getLongitude())
                + getEnd().getAltitude();
        }

        if (overrideEndElevation)
        {
            LatLon ll = pos; // Use interpolated lat/lon.
            double e1 = getBegin().getElevation();
            pos = new Position(ll, (1 - latLonInterpolant) * e1 + latLonInterpolant * endElevation);
        }

        return pos;
    }
}

[gbburkhardt]

I like this change much better. It makes sense to only access the globe from the view, instead of keeping a local copy. The constructor for OnSurfacePositionAnimator shouldn't, and doesn't have to, expect that the globe had been created when OnSurfacePositionAnimator object is created. The view, on the other hand, does. Very good.

Just out of curiosity, what computer configuration are you using? I couldn't any of mind to trigger the exception with both GDAL env variables defined. WorldWind was always in the middle of raster production at the point that the globe passed to BasicView.

[wcmatthysen]

@gbburkhardt, I'm running an Intel Core i7-8700K processor. It might be my hard-drive as I'm using a Samsung 850 Evo SSD which is quite fast. I modified PR #56 by removing the null Globe check. The changes should now correspond with the abovementioned discussion. Let me know what you think and whether we should merge the changes into develop.