fix perpetual texture reload

panda/src/glstuff/glGraphicsStateGuardian_src.cxx

@@ -7586,45 +7586,37 @@ upload_texture_image(CLP(TextureContext) *gtc,
 
   int highest_level = 0;
 
+  bool needs_reload = false;
   if (!gtc->_already_applied ||
       gtc->_uses_mipmaps != uses_mipmaps ||
       gtc->_internal_format != internal_format ||
       gtc->_width != width ||
       gtc->_height != height ||
       gtc->_depth != depth) {
-    // We need to reload a new image.
+    // We need to reload a new GL Texture object.
+    needs_reload = true;
+  }
+
+  if (!needs_reload) {
+    // Try to subload the image over the existing GL Texture object,
+    // possibly saving on texture memory fragmentation.
 
     if (GLCAT.is_debug()) {
       GLCAT.debug()
-        << "loading new texture object, " << width << " x " << height
-        << " x " << depth << ", mipmaps " << num_ram_mipmap_levels 
+        << "subloading existing texture object, " << width << " x " << height
+        << " x " << depth << ", mipmaps " << num_ram_mipmap_levels
         << ", uses_mipmaps = " << uses_mipmaps << "\n";
     }
 
-    if (num_ram_mipmap_levels == 0) {
-      if (external_format == GL_DEPTH_STENCIL_EXT || external_format == GL_DEPTH_COMPONENT) {
-        GLP(TexImage2D)(page_target, 0, internal_format,
-                        width, height, 0,
-                        external_format, GL_UNSIGNED_INT_24_8_EXT, NULL);
-      } else {
-        GLP(TexImage2D)(page_target, 0, internal_format,
-                        width, height, 0,
-                        external_format, GL_UNSIGNED_BYTE, NULL);
-      }
-    }
-    
     for (int n = mipmap_bias; n < num_ram_mipmap_levels; ++n) {
-      CPTA_uchar ptimage = tex->get_ram_mipmap_image(n);
-      if (ptimage == (const unsigned char *)NULL) {
-        if (GLCAT.is_debug()) {
-          GLCAT.debug()
-            << "No mipmap level " << n << " defined for " << tex->get_name()
-            << "\n";
-        }
+      const unsigned char *image_ptr = tex->get_ram_mipmap_image(n);
+      if (image_ptr == (const unsigned char *)NULL) {
+        GLCAT.warning()
+          << "No mipmap level " << n << " defined for " << tex->get_name()
+          << "\n";
         // No mipmap level n; stop here.
         break;
       }
-      const unsigned char *image_ptr = ptimage;
 
       size_t image_size = tex->get_ram_mipmap_image_size(n);
       if (one_page_only) {
@@ -7650,25 +7642,22 @@ upload_texture_image(CLP(TextureContext) *gtc,
       switch (texture_target) {
       case GL_TEXTURE_1D:
         if (image_compression == Texture::CM_off) {
-          GLP(TexImage1D)(page_target, n - mipmap_bias, internal_format,
-                          width, 0,
-                          external_format, component_type, image_ptr);
+          GLP(TexSubImage1D)(page_target, n - mipmap_bias, 0, width,
+                             external_format, component_type, image_ptr);
         } else {
-          _glCompressedTexImage1D(page_target, n - mipmap_bias, external_format, width,
-                                  0, image_size, image_ptr);
+          _glCompressedTexSubImage1D(page_target, n - mipmap_bias, 0, width,
+                                     external_format, image_size, image_ptr);
         }
         break;
 
       case GL_TEXTURE_3D:
         if (_supports_3d_texture) {
           if (image_compression == Texture::CM_off) {
-            _glTexImage3D(page_target, n - mipmap_bias, internal_format,
-                          width, height, depth, 0,
-                          external_format, component_type, image_ptr);
+            _glTexSubImage3D(page_target, n - mipmap_bias, 0, 0, 0, width, height, depth,
+                             external_format, component_type, image_ptr);
           } else {
-            _glCompressedTexImage3D(page_target, n - mipmap_bias, external_format, width,
-                                    height, depth,
-                                    0, image_size, image_ptr);
+            _glCompressedTexSubImage3D(page_target, n - mipmap_bias, 0, 0, 0, width, height, depth,
+                                       external_format, image_size, image_ptr);
           }
         } else {
           report_my_gl_errors();
@@ -7678,37 +7667,70 @@ upload_texture_image(CLP(TextureContext) *gtc,
 
       default:
         if (image_compression == Texture::CM_off) {
-          GLP(TexImage2D)(page_target, n - mipmap_bias, internal_format,
-                          width, height, 0,
-                          external_format, component_type, image_ptr);
+          if (n==0) {
+            // It's unfortunate that we can't adjust the width, too,
+            // but TexSubImage2D doesn't accept a row-stride parameter.
+            height = tex->get_y_size() - tex->get_pad_y_size();
+          }
+          GLP(TexSubImage2D)(page_target, n - mipmap_bias, 0, 0, width, height,
+                             external_format, component_type, image_ptr);
         } else {
-          _glCompressedTexImage2D(page_target, n - mipmap_bias, external_format, width, height,
-                                  0, image_size, image_ptr);
+          _glCompressedTexSubImage2D(page_target, n - mipmap_bias, 0, 0, width, height,
+                                     external_format, image_size, image_ptr);
         }
+        break;
       }
 
       highest_level = n;
     }
-  } else {
-    // We can reload the image over the previous image, possibly
-    // saving on texture memory fragmentation.
 
+    // Did that fail?  If it did, we'll immediately try again, this
+    // time loading the texture from scratch.
+    GLenum error_code = GLP(GetError)();
+    if (error_code != GL_NO_ERROR) {
+      if (GLCAT.is_debug()) {
+        GLCAT.debug()
+          << "GL texture subload failed for " << tex->get_name()
+          << " : " << get_error_string(error_code) << "\n";
+      }
+      needs_reload = true;
+    }
+  }
+
+  if (needs_reload) {
+    // Load the image up from scratch, creating a new GL Texture
+    // object.
     if (GLCAT.is_debug()) {
       GLCAT.debug()
-        << "subloading existing texture object, " << width << " x " << height
-        << " x " << depth << ", mipmaps " << num_ram_mipmap_levels
+        << "loading new texture object, " << width << " x " << height
+        << " x " << depth << ", mipmaps " << num_ram_mipmap_levels 
         << ", uses_mipmaps = " << uses_mipmaps << "\n";
     }
 
+    if (num_ram_mipmap_levels == 0) {
+      if (external_format == GL_DEPTH_STENCIL_EXT || external_format == GL_DEPTH_COMPONENT) {
+        GLP(TexImage2D)(page_target, 0, internal_format,
+                        width, height, 0,
+                        external_format, GL_UNSIGNED_INT_24_8_EXT, NULL);
+      } else {
+        GLP(TexImage2D)(page_target, 0, internal_format,
+                        width, height, 0,
+                        external_format, GL_UNSIGNED_BYTE, NULL);
+      }
+    }
+    
     for (int n = mipmap_bias; n < num_ram_mipmap_levels; ++n) {
-      const unsigned char *image_ptr = tex->get_ram_mipmap_image(n);
-      if (image_ptr == (const unsigned char *)NULL) {
-        GLCAT.warning()
-          << "No mipmap level " << n << " defined for " << tex->get_name()
-          << "\n";
+      CPTA_uchar ptimage = tex->get_ram_mipmap_image(n);
+      if (ptimage == (const unsigned char *)NULL) {
+        if (GLCAT.is_debug()) {
+          GLCAT.debug()
+            << "No mipmap level " << n << " defined for " << tex->get_name()
+            << "\n";
+        }
         // No mipmap level n; stop here.
         break;
       }
+      const unsigned char *image_ptr = ptimage;
 
       size_t image_size = tex->get_ram_mipmap_image_size(n);
       if (one_page_only) {
@@ -7734,22 +7756,25 @@ upload_texture_image(CLP(TextureContext) *gtc,
       switch (texture_target) {
       case GL_TEXTURE_1D:
         if (image_compression == Texture::CM_off) {
-          GLP(TexSubImage1D)(page_target, n - mipmap_bias, 0, width,
-                             external_format, component_type, image_ptr);
+          GLP(TexImage1D)(page_target, n - mipmap_bias, internal_format,
+                          width, 0,
+                          external_format, component_type, image_ptr);
         } else {
-          _glCompressedTexSubImage1D(page_target, n - mipmap_bias, 0, width,
-                                     external_format, image_size, image_ptr);
+          _glCompressedTexImage1D(page_target, n - mipmap_bias, external_format, width,
+                                  0, image_size, image_ptr);
         }
         break;
 
       case GL_TEXTURE_3D:
         if (_supports_3d_texture) {
           if (image_compression == Texture::CM_off) {
-            _glTexSubImage3D(page_target, n - mipmap_bias, 0, 0, 0, width, height, depth,
-                             external_format, component_type, image_ptr);
+            _glTexImage3D(page_target, n - mipmap_bias, internal_format,
+                          width, height, depth, 0,
+                          external_format, component_type, image_ptr);
           } else {
-            _glCompressedTexSubImage3D(page_target, n - mipmap_bias, 0, 0, 0, width, height, depth,
-                                       external_format, image_size, image_ptr);
+            _glCompressedTexImage3D(page_target, n - mipmap_bias, external_format, width,
+                                    height, depth,
+                                    0, image_size, image_ptr);
           }
         } else {
           report_my_gl_errors();
@@ -7759,22 +7784,29 @@ upload_texture_image(CLP(TextureContext) *gtc,
 
       default:
         if (image_compression == Texture::CM_off) {
-          if (n==0) {
-            // It's unfortunate that we can't adjust the width, too,
-            // but TexSubImage2D doesn't accept a row-stride parameter.
-            height = tex->get_y_size() - tex->get_pad_y_size();
-          }
-          GLP(TexSubImage2D)(page_target, n - mipmap_bias, 0, 0, width, height,
-                             external_format, component_type, image_ptr);
+          GLP(TexImage2D)(page_target, n - mipmap_bias, internal_format,
+                          width, height, 0,
+                          external_format, component_type, image_ptr);
         } else {
-          _glCompressedTexSubImage2D(page_target, n - mipmap_bias, 0, 0, width, height,
-                                     external_format, image_size, image_ptr);
+          _glCompressedTexImage2D(page_target, n - mipmap_bias, external_format, width, height,
+                                  0, image_size, image_ptr);
         }
-        break;
       }
 
       highest_level = n;
     }
+
+    // Report the error message explicitly if the GL texture creation
+    // failed.
+    GLenum error_code = GLP(GetError)();
+    if (error_code != GL_NO_ERROR) {
+      GLCAT.error()
+        << "GL texture creation failed for " << tex->get_name()
+        << " : " << get_error_string(error_code) << "\n";
+      
+      gtc->_already_applied = false;
+      return false;
+    }
   }
 
   if (is_at_least_version(1, 2)) {
@@ -7793,16 +7825,7 @@ upload_texture_image(CLP(TextureContext) *gtc,
     }
   }
 
-  // Report the error message explicitly if the GL texture creation
-  // failed.
-  GLenum error_code = GLP(GetError)();
-  if (error_code != GL_NO_ERROR) {
-    GLCAT.error()
-      << "GL texture creation failed for " << tex->get_name()
-      << " : " << get_error_string(error_code) << "\n";
-
-    return false;
-  }
+  report_my_gl_errors();
 
   return true;
 }

panda/src/gobj/texture.cxx

@@ -705,7 +705,12 @@ reload() {
   if (_loaded_from_image && !_fullpath.empty()) {
     do_clear_ram_image();
     do_unlock_and_reload_ram_image(true);
-    return do_has_ram_image();
+    if (do_has_ram_image()) {
+      // An explicit call to reload() should increment image_modified.
+      ++_image_modified;
+      return true;
+    }
+    return false;
   }
 
   // We don't have a filename to load from.
@@ -2918,16 +2923,38 @@ do_unlock_and_reload_ram_image(bool allow_compression) {
     // properties have changed during the reload (for instance,
     // because we reloaded a txo), it won't contaminate the original
     // texture.
-    _x_size = tex->_x_size;
-    _y_size = tex->_y_size;
-    _z_size = tex->_z_size;
     _orig_file_x_size = tex->_orig_file_x_size;
     _orig_file_y_size = tex->_orig_file_y_size;
-    _num_components = tex->_num_components;
-    _component_width = tex->_component_width;
-    _texture_type = tex->_texture_type;
-    _format = tex->_format;
-    _component_type = tex->_component_type;
+ 
+    // If any of *these* properties have changed, the texture has
+    // changed in some fundamental way.  Update it appropriately.
+    if (tex->_x_size != _x_size ||
+        tex->_y_size != _y_size ||
+        tex->_z_size != _z_size ||
+        tex->_num_components != _num_components ||
+        tex->_component_width != _component_width ||
+        tex->_texture_type != _texture_type ||
+        tex->_format != _format ||
+        tex->_component_type != _component_type) {
+
+      _x_size = tex->_x_size;
+      _y_size = tex->_y_size;
+      _z_size = tex->_z_size;
+
+      _num_components = tex->_num_components;
+      _component_width = tex->_component_width;
+      _texture_type = tex->_texture_type;
+      _format = tex->_format;
+      _component_type = tex->_component_type;
+
+      // Normally, we don't update the _modified semaphores in a
+      // do_blah method, but we'll make an exception in this case,
+      // because it's easiest to modify this here, and only when we
+      // know it's needed.
+      ++_properties_modified;
+      ++_image_modified;
+    }
+
     _keep_ram_image = tex->_keep_ram_image;
     _ram_image_compression = tex->_ram_image_compression;
     _ram_images = tex->_ram_images;
@@ -2935,13 +2962,10 @@ do_unlock_and_reload_ram_image(bool allow_compression) {
     nassertv(_reloading);
     _reloading = false;
 
-    // Normally, we don't update the _modified semaphores in a do_blah
-    // method, but we'll make an exception in this case, because it's
-    // easiest to modify these here, and only when we know it's
-    // needed.
-    ++_image_modified;
-    ++_properties_modified;
-
+    // We don't generally increment the _image_modified semaphore,
+    // because this is just a reload, and presumably the image hasn't
+    // changed (unless we hit the if condition above).
+    
     _cvar.notify_all();
   }
 }
@@ -4022,9 +4046,9 @@ get_ram_image_as(const string &requested_format) {
           nassertr(_num_components != 3, CPTA_uchar(get_class_type()));
           component = _num_components - 1;
         } else if (format.at(s) == '0') {
-          newdata[p * format.size() + s] =  0;
+          newdata[p * format.size() + s] = 0x00;
         } else if (format.at(s) == '1') {
-          newdata[p * format.size() + s] = -1;
+          newdata[p * format.size() + s] = 0xff;
         } else {
           gobj_cat.error() << "Unexpected component character '"
             << format.at(s) << "', expected one of RGBA!\n";