I have noticed that the ModelLoader.loadModel method sets up its own resource locator for the textures… It however would cause a NPE when called on a file with no directories (say like ModelLoader.loadModel( new File( "model.3ds" ) ); because this file does not have a parent!.
I think it would be nice to have a test for null here and/or remove this resource locator… (I know neakor asked for it, and Darkfrog added it, but still ;))
Thanks
Shouldn't whatever class the NPE occurs in rather throw a FileNotFoundException in that case?
hevee said:
Shouldn't whatever class the NPE occurs in rather throw a FileNotFoundException in that case?
Forget I ever wrote that, it's nonsense. File doesn't use the native filesystem to check for parents when using getParentFile(), but for some reason I was assuming that. I'm sorry, but I blame the mistake on today being monday :D
Actually I am looking at ModelLoader in further detail and it uses the filename as a URL and the tries to call openStream() on it!, I would say that this should use the ResourceLocator instead of the absolute path given by a File. Anyone agrees? Or is this a design choice I am not aware of?
Also, it seems wasteful to have so many inner classes (one per format type and called XXXCallable) I suggest making the inner interface ModelLoaderCallable a class. I am working on it, but I want to know if it makes sense.
Thanks
Ok, here goes nothing! I removed the adding of the resource location for the parent of the model, and revamped the lading schemes.
This is the Diff Patch:
<edit>Replaced code with diff patch</edit>
### Eclipse Workspace Patch 1.0
#P jme
Index: src/com/jmex/model/util/ModelLoader.java
===================================================================
RCS file: /cvs/jme/src/com/jmex/model/util/ModelLoader.java,v
retrieving revision 1.15
diff -u -r1.15 ModelLoader.java
--- src/com/jmex/model/util/ModelLoader.java 14 Sep 2007 22:09:11 -0000 1.15
+++ src/com/jmex/model/util/ModelLoader.java 13 Nov 2007 00:57:38 -0000
@@ -38,7 +38,6 @@
import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
-import java.net.URISyntaxException;
import java.net.URL;
import java.util.HashMap;
import java.util.Map;
@@ -100,7 +99,7 @@
chooser.setCurrentDirectory(directory);
if (chooser.showOpenDialog(null) == JFileChooser.APPROVE_OPTION) {
File file = chooser.getSelectedFile();
- if (isValidModelFile(file)) {
+ if (isValidModelFile(file.getName())) {
// Set it in preferences so we remember next time
preferences.put("StartDirectory", file.getAbsolutePath());
@@ -129,7 +128,7 @@
loading.setActive(true);
loading.setProgress(0.5f, "Loading Model: " + file.getName());
long time = System.currentTimeMillis();
- final Node modelNode = loadModel(file);
+ final Node modelNode = loadModel(file.getName());
outputElapsed(time);
if (modelNode != null) {
modelNode.updateRenderState();
@@ -201,24 +200,24 @@
logger.info("Took " + elapsed + " seconds to load the model.");
}
- public static Node loadModel( final File file ) throws Exception {
+ public static Node loadModel( final String file ) throws Exception {
// Add to resource locator
- SimpleResourceLocator locator = new SimpleResourceLocator(file.getParentFile().toURI());
- ResourceLocatorTool.addResourceLocator(ResourceLocatorTool.TYPE_TEXTURE, locator);
+ //SimpleResourceLocator locator = new SimpleResourceLocator(file.getParentFile().toURI());
+ //ResourceLocatorTool.addResourceLocator(ResourceLocatorTool.TYPE_TEXTURE, locator);
String extension = extensionOf( file );
ModelLoaderCallable callable = loaders.get( extension );
if ( callable == null ) {
- throw new UnsupportedOperationException( "Unknown file type: " + file.getName() );
+ throw new UnsupportedOperationException( "Unknown file type: " + file );
}
callable.setFile( file );
Future<Node> future = GameTaskQueueManager.getManager().update( callable );
return future.get();
}
- private static String extensionOf( File file ) {
- String fileName = file.getName().toUpperCase();
+ private static String extensionOf( String file ) {
+ String fileName = file.toUpperCase();
int lastDot = fileName.lastIndexOf( '.' );
String extension;
if ( lastDot >= 0 ) {
@@ -258,7 +257,7 @@
return model;
}
- public static boolean isValidModelFile( File file ) {
+ public static boolean isValidModelFile( String file ) {
return loaders.containsKey( extensionOf( file ) );
}
@@ -268,152 +267,67 @@
loaders.put( "DAE", new DAECallable() );
loaders.put( "JME", new JMECallable() );
// Note that .OBJ Ambient colors are multiplied. I would strongly suggest making them black.
- loaders.put( "OBJ", new OBJCallable() );
+ loaders.put( "OBJ", new ModelLoaderCallable(
+ new ObjToJme(), new String[]{ "mtllib", "texdir" } )
+ );
// Note that some .3DS Animations from Blender may not work. I'd suggest using a version of 3DSMax.
- loaders.put( "3DS", new TDSCallable() );
- loaders.put( "ASE", new ASECallable() );
- loaders.put( "MD2", new MD2Callable() );
- loaders.put( "MD3", new MD3Callable() );
- loaders.put( "MS3D", new MilkCallable() );
- }
-
- public interface ModelLoaderCallable extends Callable<Node> {
- public void setFile( File file );
- }
-
- private static class JMECallable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
- public Node call() throws Exception {
- return (Node) BinaryImporter.getInstance().load( file );
- }
- }
-
- private static class DAECallable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
- public Node call() throws Exception {
- ColladaImporter.load( file.toURI().toURL().openStream(), "Model" );
- Node model = ColladaImporter.getModel();
- return model;
- }
- }
-
- private static class OBJCallable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
- public Node call() throws Exception {
- FormatConverter converter = new ObjToJme();
- URL url = file.toURI().toURL();
- converter.setProperty( "mtllib", url );
- converter.setProperty( "texdir", url );
+ loaders.put( "3DS", new ModelLoaderCallable(
+ new MaxToJme(), new String[]{ "texurl", "texdir" } )
+ );
+ loaders.put( "ASE", new ModelLoaderCallable( new AseToJme() ) );
+ loaders.put( "MD2", new ModelLoaderCallable( new Md2ToJme() ) );
+ loaders.put( "MD3", new ModelLoaderCallable( new Md3ToJme() ) );
+ loaders.put( "MS3D", new ModelLoaderCallable( new MilkToJme() ) );
+ }
+
+ static class ModelLoaderCallable implements Callable<Node> {
+ protected String file, props[];
+ protected FormatConverter converter;
+ public ModelLoaderCallable( FormatConverter conv ) { this( conv, null ); }
+ public ModelLoaderCallable( FormatConverter conv, String[] properties )
+ {
+ converter = conv;
+ setProperties( properties );
+ }
+ public void setProperties( String[] p )
+ {
+ props = ( p == null ? new String[0] : p );
+ }
+ public void setFile( String file ) { this.file = file; }
+ protected URL getURL() { return ResourceLocatorTool.locateResource( ResourceLocatorTool.TYPE_MODEL, file ); }
+ public Node call() throws Exception
+ {
+ if( converter == null )
+ return null;
+ URL url = getURL();
+ for( int i = 0; i < props.length; i++ )
+ converter.setProperty( props[i], url );
ByteArrayOutputStream bos = new ByteArrayOutputStream();
converter.convert( url.openStream(), bos );
Savable savable = BinaryImporter.getInstance().load( new ByteArrayInputStream( bos.toByteArray() ) );
if ( savable instanceof Node ) {
return (Node) savable;
} else {
- Node model = new Node( "Imported Model " + file.getName() );
+ Node model = new Node( "Imported Model " + file );
model.attachChild( (Spatial) savable );
return model;
}
- }
- }
-
- private static class TDSCallable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
- public Node call() throws Exception {
- FormatConverter converter = new MaxToJme();
- URL url = file.toURI().toURL();
- converter.setProperty( "texurl", url );
- ByteArrayOutputStream bos = new ByteArrayOutputStream();
- converter.convert( url.openStream(), bos );
- Node model = (Node) BinaryImporter.getInstance().load( new ByteArrayInputStream( bos.toByteArray() ) );
- return model;
}
}
- private static class ASECallable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
- public Node call() throws Exception {
- FormatConverter converter = new AseToJme();
- URL url = file.toURI().toURL();
- ByteArrayOutputStream bos = new ByteArrayOutputStream();
- converter.convert( url.openStream(), bos );
- Node model = (Node) BinaryImporter.getInstance().load( new ByteArrayInputStream( bos.toByteArray() ) );
- return model;
- }
- }
-
- private static class MD2Callable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
- public Node call() throws Exception {
- FormatConverter converter = new Md2ToJme();
- URL url = file.toURI().toURL();
- ByteArrayOutputStream bos = new ByteArrayOutputStream();
- converter.convert( url.openStream(), bos );
- Node model = (Node) BinaryImporter.getInstance().load( new ByteArrayInputStream( bos.toByteArray() ) );
- return model;
- }
- }
-
- private static class MD3Callable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
+ private static class JMECallable extends ModelLoaderCallable {
+ public JMECallable() { super( null, null ); }
public Node call() throws Exception {
- FormatConverter converter = new Md3ToJme();
- URL url = file.toURI().toURL();
- ByteArrayOutputStream bos = new ByteArrayOutputStream();
- converter.convert( url.openStream(), bos );
- Node model = (Node) BinaryImporter.getInstance().load( new ByteArrayInputStream( bos.toByteArray() ) );
- return model;
+ return (Node) BinaryImporter.getInstance().load( getURL() );
}
}
- private static class MilkCallable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
+ private static class DAECallable extends ModelLoaderCallable {
+ public DAECallable() { super( null, null ); }
public Node call() throws Exception {
- FormatConverter converter = new MilkToJme();
- URL url = file.toURI().toURL();
- ByteArrayOutputStream bos = new ByteArrayOutputStream();
- converter.convert( url.openStream(), bos );
- Node model = (Node) BinaryImporter.getInstance().load( new ByteArrayInputStream( bos.toByteArray() ) );
+ ColladaImporter.load( getURL().openStream(), "Model" );
+ Node model = ColladaImporter.getModel();
return model;
}
}
Maybe this is obvious, but why do you remove the SimpleResourceLocator lines for Texture?
First because the ModelLoader never removes it, so it is not a very clean programming style. Second, because I consider that behavior a side-effect that can be avoided, assuming the user knows more than what the engine is trying to guess (that is, that the textures are in the same directory), and third ans most important, because I am lazy and didn't want to implement the parent operator on a platform independent way… (actually it was mainly the third one ;))
If you think it should be there, then I will add it… but should it register and then unregister the location or simply add it every time it is called?
I noticed that the ModelLoader has a main method, and thus it needs some changes to work with the new way of doing things!
I order for ti to work properly, it requires teh following three lines:
File parent = file.getParentFile();
ResourceLocatorTool.addResourceLocator( ResourceLocatorTool.TYPE_MODEL, new SimpleResourceLocator( parent.toURI() ) );
ResourceLocatorTool.addResourceLocator( ResourceLocatorTool.TYPE_TEXTURE, new SimpleResourceLocator( parent.toURI() ) );
Right after the creation of the selected file.
Sorry to bring this dead thread back to life, but I noticed that ModelLoader has been updated, but this patch was never applied. Was that on purpose? If so, I will just have to re-patch my local copy with this version, and that’s not a big deal, but if the patch was indeed useful, then it has been lost in the new version :’(
I was waiting for your reply and then missed it. Sorry. Would you mind recreating the patch?
Well I seem to have overlooked this answer also, so no worries… :roll:
Here goes the patch:
### Eclipse Workspace Patch 1.0
#P jme
Index: src/com/jmex/model/util/ModelLoader.java
===================================================================
RCS file: /cvs/jme/src/com/jmex/model/util/ModelLoader.java,v
retrieving revision 1.16
diff -u -r1.16 ModelLoader.java
--- src/com/jmex/model/util/ModelLoader.java 4 Jan 2008 17:05:17 -0000 1.16
+++ src/com/jmex/model/util/ModelLoader.java 23 Jan 2008 04:15:26 -0000
@@ -129,7 +129,13 @@
loading.setActive(true);
loading.setProgress(0.5f, "Loading Model: " + file.getName());
long time = System.currentTimeMillis();
- final Node modelNode = loadModel(file);
+
+ // Add to resource locator
+ SimpleResourceLocator locator = new SimpleResourceLocator(file.getParentFile().toURI());
+ ResourceLocatorTool.addResourceLocator(ResourceLocatorTool.TYPE_TEXTURE, locator);
+ ResourceLocatorTool.addResourceLocator(ResourceLocatorTool.TYPE_MODEL, locator);
+
+ final Node modelNode = loadModel(file);
outputElapsed(time);
if (modelNode != null) {
modelNode.updateRenderState();
@@ -202,23 +208,23 @@
}
public static Node loadModel( final File file ) throws Exception {
- // Add to resource locator
- SimpleResourceLocator locator = new SimpleResourceLocator(file.getParentFile().toURI());
- ResourceLocatorTool.addResourceLocator(ResourceLocatorTool.TYPE_TEXTURE, locator);
+ return loadModel( file.getName() );
+ }
+ public static Node loadModel( final String file ) throws Exception {
String extension = extensionOf( file );
ModelLoaderCallable callable = loaders.get( extension );
if ( callable == null ) {
- throw new UnsupportedOperationException( "Unknown file type: " + file.getName() );
+ throw new UnsupportedOperationException( "Unknown file type: " + file );
}
callable.setFile( file );
Future<Node> future = GameTaskQueueManager.getManager().update( callable );
return future.get();
}
- private static String extensionOf( File file ) {
- String fileName = file.getName().toUpperCase();
+ private static String extensionOf( String file ) {
+ String fileName = file.toUpperCase();
int lastDot = fileName.lastIndexOf( '.' );
String extension;
if ( lastDot >= 0 ) {
@@ -259,6 +265,9 @@
}
public static boolean isValidModelFile( File file ) {
+ return isValidModelFile( file.getName() );
+ }
+ public static boolean isValidModelFile( String file ) {
return loaders.containsKey( extensionOf( file ) );
}
@@ -268,170 +277,70 @@
loaders.put( "DAE", new DAECallable() );
loaders.put( "JME", new JMECallable() );
// Note that .OBJ Ambient colors are multiplied. I would strongly suggest making them black.
- loaders.put( "OBJ", new OBJCallable() );
+ loaders.put( "OBJ", new ModelLoaderCallable(
+ new ObjToJme(), new String[]{ "mtllib", "texdir" } )
+ );
// Note that some .3DS Animations from Blender may not work. I'd suggest using a version of 3DSMax.
- loaders.put( "3DS", new TDSCallable() );
- loaders.put( "ASE", new ASECallable() );
- loaders.put( "MD2", new MD2Callable() );
- loaders.put( "MD3", new MD3Callable() );
- loaders.put( "MS3D", new MilkCallable() );
- loaders.put( "X3D", new X3DCallable() );
- }
-
- public interface ModelLoaderCallable extends Callable<Node> {
- public void setFile( File file );
- }
-
- private static class JMECallable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
- public Node call() throws Exception {
- return (Node) BinaryImporter.getInstance().load( file );
- }
- }
-
- private static class DAECallable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
- public Node call() throws Exception {
- ColladaImporter.load( file.toURI().toURL().openStream(), "Model" );
- Node model = ColladaImporter.getModel();
- return model;
- }
- }
-
- private static class OBJCallable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
- public Node call() throws Exception {
- FormatConverter converter = new ObjToJme();
- URL url = file.toURI().toURL();
- converter.setProperty( "mtllib", url );
- converter.setProperty( "texdir", url );
+ loaders.put( "3DS", new ModelLoaderCallable(
+ new MaxToJme(), new String[]{ "texurl", "texdir" } )
+ );
+ loaders.put( "ASE", new ModelLoaderCallable( new AseToJme() ) );
+ loaders.put( "MD2", new ModelLoaderCallable( new Md2ToJme() ) );
+ loaders.put( "MD3", new ModelLoaderCallable( new Md3ToJme() ) );
+ loaders.put( "MS3D", new ModelLoaderCallable( new MilkToJme() ) );
+ try {
+ loaders.put( "X3D", new ModelLoaderCallable( new X3dToJme() ) );
+ } catch( InstantiationException ex ) { logger.logp( Level.SEVERE, ModelLoader.class.toString(),
+ "<static>", "InstantiationException", ex ); }
+ }
+
+ static class ModelLoaderCallable implements Callable<Node> {
+ protected String file, props[];
+ protected FormatConverter converter;
+ public ModelLoaderCallable( FormatConverter conv ) { this( conv, null ); }
+ public ModelLoaderCallable( FormatConverter conv, String[] properties )
+ {
+ converter = conv;
+ setProperties( properties );
+ }
+ public void setProperties( String[] p )
+ {
+ props = ( p == null ? new String[0] : p );
+ }
+ public void setFile( String file ) { this.file = file; }
+ protected URL getURL() { return ResourceLocatorTool.locateResource( ResourceLocatorTool.TYPE_MODEL, file ); }
+ public Node call() throws Exception
+ {
+ if( converter == null )
+ return null;
+ URL url = getURL();
+ for( int i = 0; i < props.length; i++ )
+ converter.setProperty( props[i], url );
ByteArrayOutputStream bos = new ByteArrayOutputStream();
converter.convert( url.openStream(), bos );
Savable savable = BinaryImporter.getInstance().load( new ByteArrayInputStream( bos.toByteArray() ) );
if ( savable instanceof Node ) {
return (Node) savable;
} else {
- Node model = new Node( "Imported Model " + file.getName() );
+ Node model = new Node( "Imported Model " + file );
model.attachChild( (Spatial) savable );
return model;
}
}
}
- private static class TDSCallable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
- public Node call() throws Exception {
- FormatConverter converter = new MaxToJme();
- URL url = file.toURI().toURL();
- converter.setProperty( "texurl", url );
- ByteArrayOutputStream bos = new ByteArrayOutputStream();
- converter.convert( url.openStream(), bos );
- Node model = (Node) BinaryImporter.getInstance().load( new ByteArrayInputStream( bos.toByteArray() ) );
- return model;
- }
- }
-
- private static class ASECallable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
- public Node call() throws Exception {
- FormatConverter converter = new AseToJme();
- URL url = file.toURI().toURL();
- ByteArrayOutputStream bos = new ByteArrayOutputStream();
- converter.convert( url.openStream(), bos );
- Node model = (Node) BinaryImporter.getInstance().load( new ByteArrayInputStream( bos.toByteArray() ) );
- return model;
- }
- }
-
- private static class MD2Callable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
- public Node call() throws Exception {
- FormatConverter converter = new Md2ToJme();
- URL url = file.toURI().toURL();
- ByteArrayOutputStream bos = new ByteArrayOutputStream();
- converter.convert( url.openStream(), bos );
- Node model = (Node) BinaryImporter.getInstance().load( new ByteArrayInputStream( bos.toByteArray() ) );
- return model;
- }
- }
-
- private static class MD3Callable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
- public Node call() throws Exception {
- FormatConverter converter = new Md3ToJme();
- URL url = file.toURI().toURL();
- ByteArrayOutputStream bos = new ByteArrayOutputStream();
- converter.convert( url.openStream(), bos );
- Node model = (Node) BinaryImporter.getInstance().load( new ByteArrayInputStream( bos.toByteArray() ) );
- return model;
- }
- }
-
- private static class X3DCallable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
+ private static class JMECallable extends ModelLoaderCallable {
+ public JMECallable() { super( null, null ); }
public Node call() throws Exception {
- FormatConverter converter = new X3dToJme();
- URL url = file.toURI().toURL();
- ByteArrayOutputStream bos = new ByteArrayOutputStream();
- converter.convert( url.openStream(), bos );
- Node model = (Node) BinaryImporter.getInstance().load( new ByteArrayInputStream( bos.toByteArray() ) );
- return model;
+ return (Node) BinaryImporter.getInstance().load( getURL() );
}
}
- private static class MilkCallable implements ModelLoaderCallable {
- private File file;
-
- public void setFile( File file ) {
- this.file = file;
- }
-
+ private static class DAECallable extends ModelLoaderCallable {
+ public DAECallable() { super( null, null ); }
public Node call() throws Exception {
- FormatConverter converter = new MilkToJme();
- URL url = file.toURI().toURL();
- ByteArrayOutputStream bos = new ByteArrayOutputStream();
- converter.convert( url.openStream(), bos );
- Node model = (Node) BinaryImporter.getInstance().load( new ByteArrayInputStream( bos.toByteArray() ) );
+ ColladaImporter.load( getURL().openStream(), "Model" );
+ Node model = ColladaImporter.getModel();
return model;
}
}
It patches the current version from CVS, and thus includes the X3D loader, but I was not able to test that because I don't have an X3D file myself, and there is none in the jmetest package.
applied. Thanks again.