next »

[ChrisE]

Marvin,

This one is pretty straightforward.

The Bug

Passing an absolute path for an image to TextureLoader.getInstance().getTexture() results in a texture that will crash upon use.

The Problem

Stepping through it we see that in AbstractTextureLoader.java (around line 590 in AbstractTextureLoader::getInputStream) the absolute path is not used to find a file.
In the docs higher up, it says that as a last step if no texture stream locators can find the file then a normal attempt will be made. None is, however, as a failed attempt results in a null "in" InputStream, which is used to construct a bad return BufferedInputStream.

This should return a null object instead of a broken stream which later causes a texture crash; null objects are handled properly by the calling code.

However, to fully meet the spec, a BufferedInputStream should be created from the path (in turn from a FileInputStream) and returned if usable (a null object being returned otherwise).

The Solution

Add some code to attempt a file opening from an absolute path using normal Java calls if the TSLs are unable to find it, and make sure to return a null object if something doesn't work.

The patch should look like this:

Code:

Index: AbstractTextureLoader.java
===================================================================
--- AbstractTextureLoader.java (revision 398)
+++ AbstractTextureLoader.java (working copy)
@@ -30,7 +30,11 @@
 package org.jagatoo.loaders.textures;
 
 import java.io.BufferedInputStream;
+import java.io.BufferedReader;
 import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FileReader;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.MalformedURLException;
@@ -613,6 +617,22 @@
             return ( (BufferedInputStream)in );
         }
         
+        ///////////////////////// NEWCODE
+       
+        if (in == null)
+        {
+        try {
+        BufferedInputStream is =  new BufferedInputStream(new FileInputStream(name));
+        return is;
+ } catch (FileNotFoundException e) {
+ // TODO Auto-generated catch block
+ e.printStackTrace();
+ return null;
+ }
+        }
+       
+        /////////////////////// NEWCODE
+       
         return ( new BufferedInputStream( in ) );
     }

If that works, would you mind rolling it into Xith and (possibly) updating the cooker version on the frontpage? My build environment is under duress right now due to work.

-Chris

[ChrisE]

The Workaround
The workaround right now for this is to explicitly construct a URL path on the local system (creating a File, getting its URI, converting the URI to a URL) and calling the loadTexture() function like so:

Code:

Texture tex;
URL imageURL;
File imageFile = new File(imagefilename);

try {
imageURL = imageFile.toURI().toURL();
} catch (MalformedURLException e) {
// TODO Auto-generated catch block
e.printStackTrace();
imageURL = null;
}
tex = TextureLoader.getInstance().loadTexture(imageURL, MipmapMode.MULTI_LEVEL_MIPMAP);

[Marvin Fröhlich]

Where did you find the specs about the fallback to direct file access? I can't find it anywhere. Despite that it'd be wrong. If you want that behavior, register an appropriate TextureStreamLocator for that. It's definitely not meant to load directly from the filesysten by an absolute file access. Projects need to stay platform independent.

You can query the name of a file, that is accessed relatively, if you don't register any TSLs. Though you should always try to control the TSLs yourself.

I have committed a patch for the null stream handling though, but I haven't created a new build. Too short in time currently. Sorry.

[ChrisE]

Marvin, thanks for the NPE fix.

I think that my expectation about absolute paths came from misreading the class Javadoc for AbstractTextureLoader--the last line about "At last the fallback..." threw me off.

Thanks again.

-Chris