Bug fix so that textures are render context-aware

Replace Texture.java with the following.  I have only included the portions that have significant changes.  i also highlighted these sections with the comment tag "MSLI".  I have only tested the fix in my own code, which takes advantage of context switching to achieve multiple views of multiple models.  Others will have to confirm if ithe fixes negatively impact other aspects of JME.



Also, I tweaked DisplaySystem.java so that it does not use "com.sun…" packages.  I posted that separately.



–jon



=========Texture.java=====================

package com.jme.image;



import java.io.IOException;

import java.util.HashMap;

import java.util.Map;



import com.jme.math.FastMath;

import com.jme.math.Matrix4f;

import com.jme.math.Quaternion;

import com.jme.math.Vector3f;

import com.jme.renderer.ColorRGBA;

import com.jme.util.TextureKey;

import com.jme.util.TextureManager;

import com.jme.util.export.InputCapsule;

import com.jme.util.export.JMEExporter;

import com.jme.util.export.JMEImporter;

import com.jme.util.export.OutputCapsule;

import com.jme.util.export.Savable;



/**

  • <code>Texture</code> defines a texture object to be used to display an image
  • on a piece of geometry. The image to be displayed is defined by the
  • <code>Image</code> class. All attributes required for texture mapping are
  • contained within this class. This includes mipmapping if desired,
  • magnificationFilter options, apply options and correction options. Default
  • values are as follows: minificationFilter - NearestNeighborNoMipMaps,
  • magnificationFilter - NearestNeighbor, wrap - EdgeClamp on S,T and R, apply -
  • Modulate, enivoronment - None.
  • <p>
  • MSLI:
  • <b>The problem:</b> Currently, the texture ID is the ID that is actually bound in
  • OpenGL. However, OpenGL is context aware, whereas the texture ID maintained
  • by texture is not. As such, when the system applies texture state and a
  • texture is found with a non-zero texture ID, it is assumed bound regardless
  • of the context, which means it actually may not be bound within the current
  • context, or it may be bound with a different texture ID.
  • <p>
  • <b>The solution:</b> Ideally, texture ID should be associated with the texture
  • state, which is context aware, not the texture. As an expedient workaround,
  • the texture ID accessors in Texture are modified to be context aware, and a
  • static useContext() method is added to globally establish the current
  • context.
  • @author Jon Barrilleaux
  • :MSLI
  • <p>
  • @see com.jme.image.Image
  • @author Mark Powell
  • @author Joshua Slack
  • @version $Id: Texture.java 4131 2009-03-19 20:15:28Z blaine.dev $

    */

    public abstract class Texture implements Savable {

        private static final long serialVersionUID = -3642148179543729674L;

       



    // MSLI:

    /**
  • Establishes the current "render context" for all textures. Specifically
  • affects the mapping of texture ID by context.
  • @param contextKey Shared opaque context "key". Never null. Should be the
  • same object used for all other context switching, which is typically the
  • JME canvas object.
  • @see {@link #setTextureId(int)}
  • @see {@link #getTextureId()}

    */

    public static void useContext(Object contextKey) {

        if(contextKey == null) throw new IllegalArgumentException();

        CONTEXT_KEY = contextKey;

        }



    private static Object CONTEXT_KEY = null; // null until first use

    private Map<Object,Integer> _textureIdMap =

    new HashMap<Object,Integer>(); // maps context key to texture ID

    // :MSLI





    //MSLI:

    /**
  • Gets the texture ID associated with this texture and the current global
  • texture context.
  • @return The texture ID (>=0). If zero, the texture has not been loaded.
  • If non-zero, the texture is assumed to be bindable to that ID in the
  • current render context.
  • @see #useContext(Object)
  • @throws IllegalStateException if setContext() has not yet been called.

    */

    public int getTextureId() {

    if(CONTEXT_KEY == null) throw new IllegalStateException(

    "Must first call setContext().");

    Integer textureId = _textureIdMap.get(CONTEXT_KEY);

    if (textureId == null) return 0;

    return textureId;

    }



    /**
  • Sets the texture ID associated with this texture and the current global
  • texture context.
  • @param The texture ID (>=0). If zero, the texture will be forced to
  • reload. If non-zero, the texture is assumed to be bindable to that ID in
  • the current render context.
  • @see #useContext(Object)
  • @throws IllegalStateException if setContext() has not yet been called.

    */

    public void setTextureId(int textureId) {

    if(CONTEXT_KEY == null) throw new IllegalStateException(

    "Must first call setContext().");

    if (textureId < 0) throw new IllegalArgumentException();

    _textureIdMap.put(CONTEXT_KEY, textureId);

    }

       

    //    /

    //    * <code>getTextureId</code> returns the texture id of this texture. This

    //    * id is required to be unique to any other texture objects running in the

    //    * same JVM. However, no guarantees are made that it will be unique, and as

    //    * such, the user is responsible for this.

    //    *

    //    * @return the id of the texture.

    //    */

    //    public int getTextureId() {

    //        return textureId;

    //    }

    //

    //    /


    //    * <code>setTextureId</code> sets the texture id for this texture. Zero

    //    * means no id is set.

    //    *

    //    * @param textureId

    //    *            the texture id of this texture.

    //    */

    //    public void setTextureId(int textureId) {

    //        this.textureId = textureId;

    //    }

    //:MSLI


Would you mind posting this as a diff patch?

With a bit of searching I stumbled across the magic link to allow file upload…



Attached is a patch for both Texture and DisplaySystem.



–jon

Some of the mods here, have troubles with attachements :)  so i'll post the patch as plain text again.


Index: src/com/jme/image/Texture.java
===================================================================
--- src/com/jme/image/Texture.java   (revision 4288)
+++ src/com/jme/image/Texture.java   (working copy)
@@ -33,6 +33,8 @@
 package com.jme.image;
 
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 
 import com.jme.math.FastMath;
 import com.jme.math.Matrix4f;
@@ -48,23 +50,51 @@
 import com.jme.util.export.Savable;
 
 /**
- * <code>Texture</code> defines a texture object to be used to display an
- * image on a piece of geometry. The image to be displayed is defined by the
+ * <code>Texture</code> defines a texture object to be used to display an image
+ * on a piece of geometry. The image to be displayed is defined by the
  * <code>Image</code> class. All attributes required for texture mapping are
  * contained within this class. This includes mipmapping if desired,
  * magnificationFilter options, apply options and correction options. Default
  * values are as follows: minificationFilter - NearestNeighborNoMipMaps,
  * magnificationFilter - NearestNeighbor, wrap - EdgeClamp on S,T and R, apply -
- * Modulate, enivoronment - None.
- *
+ * Modulate, environment - None.
+ * <p>
+ * Ideally, texture ID should be associated with TextureState, which is render
+ * context aware, not Texture. As an expedient workaround, however, the texture
+ * ID accessors here in Texture have been modified to be context aware. The
+ * default context key is "null", which allows Texture to be used without
+ * contexts. To use it with contexts, the client must call useContext() to
+ * establish the current global context for texturing before accessing the
+ * texture ID.
+ * <p>
  * @see com.jme.image.Image
  * @author Mark Powell
  * @author Joshua Slack
+ * @author Jon Barrilleaux - Added context awareness.
  * @version $Id$
  */
 public abstract class Texture implements Savable {
     private static final long serialVersionUID = -3642148179543729674L;
 
+   /**
+    * Establishes the current "render context" for all textures. Specifically
+    * affects the mapping of texture ID by context. Defaults to the "default"
+    * context.
+    * @param contextKey Shared opaque context "key". Default context if null.
+    * If context switching is required, should be the same non-null object used
+    * for all other context switching, which is typically the JME canvas
+    * object.
+    * @see {@link #setTextureId(int)}
+    * @see {@link #getTextureId()}
+    */
+   public static void useContext(Object contextKey) {
+      CONTEXT_KEY = contextKey;
+   }
+
+   private static Object CONTEXT_KEY = null; // default context is null
+   private Map<Object,Integer> _textureIdMap = null; // lazy build, maps context key to texture ID
+   private int _defaultTextureId = 0; // for default context
+
     public static boolean DEFAULT_STORE_TEXTURE = false;
 
     public enum Type {
@@ -564,7 +594,6 @@
 
     private float anisotropicFilterPercent = 0.0f;
 
-    private transient int textureId;
     private ApplyMode apply = ApplyMode.Modulate;
     private MinificationFilter minificationFilter = MinificationFilter.NearestNeighborNoMipMaps;
     private MagnificationFilter magnificationFilter = MagnificationFilter.NearestNeighbor;
@@ -698,28 +727,49 @@
         updateMemoryReq();
     }
 
-    /**
-     * <code>getTextureId</code> returns the texture id of this texture. This
-     * id is required to be unique to any other texture objects running in the
-     * same JVM. However, no guarantees are made that it will be unique, and as
-     * such, the user is responsible for this.
-     *
-     * @return the id of the texture.
-     */
-    public int getTextureId() {
-        return textureId;
-    }
+   /**
+    * Gets the texture ID associated with this texture and the current global
+    * texture context.
+    * @return The texture ID (>=0). If zero, the texture has not been loaded.
+    * If non-zero, the texture is assumed to be bindable to this ID in the
+    * current render context.
+    * @see #useContext(Object)
+    */
+   public int getTextureId() {
+      if(CONTEXT_KEY == null) {
+         return _defaultTextureId;
+      } else {
+         if(_textureIdMap == null) {
+            return 0;
+         } else {
+            Integer textureId = _textureIdMap.get(CONTEXT_KEY);
+            if (textureId == null) return 0;
+            return textureId;
+         }
+      }
+   }
 
-    /**
-     * <code>setTextureId</code> sets the texture id for this texture. Zero
-     * means no id is set.
-     *
-     * @param textureId
-     *            the texture id of this texture.
-     */
-    public void setTextureId(int textureId) {
-        this.textureId = textureId;
-    }
+   /**
+    * Sets the texture ID associated with this texture and the current global
+    * texture context.
+    * @param The texture ID (>=0). If zero, the texture will be forced to
+    * reload. If non-zero, the texture is assumed to be loaded and therefore
+    * bindable to this ID in the current render context.
+    * @see #useContext(Object)
+    */
+   public void setTextureId(int textureId) {
+      if (textureId < 0) throw new IllegalArgumentException(
+      "Texture ID must be >=0. ID=" + textureId);
+      
+      if(CONTEXT_KEY == null) {
+         _defaultTextureId = textureId;
+      } else {
+         if(_textureIdMap == null) {
+            _textureIdMap = new HashMap<Object,Integer>();
+         }
+         _textureIdMap.put(CONTEXT_KEY, textureId);
+      }
+   }
 
     /**
      * <code>getImage</code> returns the image data that makes up this
@@ -1173,9 +1223,9 @@
         }
         
         Texture that = (Texture) other;
-        if (this.textureId != that.textureId)
+        if (getTextureId() != that.getTextureId())
             return false;
-        if (this.textureId == 0) {
+        if (getTextureId() == 0) {
             if (this.getImage() != null
                     && !this.getImage().equals(that.getImage()))
                 return false;
@@ -1264,7 +1314,7 @@
         rVal.setImage(image); // NOT CLONED.
         rVal.memReq = memReq;
         rVal.setImageLocation(imageLocation);
-        rVal.setTextureId(textureId);
+        rVal.setTextureId(getTextureId());
         rVal.setBlendColor(blendColor != null ? blendColor.clone() : null);
         if (scale != null)
             rVal.setScale(scale);
Index: src/com/jme/system/DisplaySystem.java
===================================================================
--- src/com/jme/system/DisplaySystem.java   (revision 4288)
+++ src/com/jme/system/DisplaySystem.java   (working copy)
@@ -38,8 +38,8 @@
 import java.util.Map;
 import java.util.logging.Logger;
 
-import sun.misc.Service;
-import sun.misc.ServiceConfigurationError;
+import java.util.ServiceConfigurationError;
+import java.util.ServiceLoader;
 
 import com.jme.image.Image;
 import com.jme.input.joystick.JoystickInput;
@@ -72,6 +72,7 @@
  * @author Mark Powell
  * @author Gregg Patton
  * @author Joshua Slack - Optimizations, Headless rendering, RenderContexts, AWT integration
+ * @author Jon Barrilleaux - Use java.util instead of com.sun.
  * @version $Id$
  * @see com.jme.renderer.Renderer
  */
@@ -191,8 +192,8 @@
     private static Map<String, SystemProvider> getSystemProviderMap()
             throws ServiceConfigurationError {
         if (systemProviderMap.isEmpty()) {
-            @SuppressWarnings("unchecked")
-            Iterator<SystemProvider> displayProviders = Service.providers(SystemProvider.class);
+            ServiceLoader<SystemProvider> loader = ServiceLoader.load(SystemProvider.class);
+            Iterator<SystemProvider> displayProviders = loader.iterator();
             while (displayProviders.hasNext()) {
           

I don't see anything I don't like with the texture patch (hopefully momoko_fan will have a look, he knows much more about the context stuff).  But doesn't the DisplaySystem patch require Java 6? 



(Java 6 has not, and probably will never be, released for 32bit OSX  :?; and while it is an issue that will only affect a portion of people, I myself have a 32bit MAC…  most advanced Java development platform my butt.)



One of these days I will have some time to check out one of the 'alternatives' out there, but for now I am just stuck with Java 5.

I think the use of the ServiceLoader class should be removed completely as its been there for ages and has not been used at all. Its rather hard to make a Renderer implementation from scratch and besides we already have LWJGL and JOGL, what else could we possibly need?



About the texture patch, I don't really understand it so I can't really comment on it. Like I said, this patch wouldn't be necessary if proper context sharing were implemented, so somebody will have to go into the DisplaySystem implementations and make the necessary changes. This change would have absolutely no effect on existing code, except that it would allow all textures, shaders, and vertex buffers to be used across multiple displays/canvases.