working on #162, cleaning up image saving logic

core/src/processing/core/PImage.java

@@ -3313,58 +3313,27 @@ int testFunction(int dst, int src) {
    * @param filename a sequence of letters and numbers
    */
   public boolean save(String filename) {  // ignore
-    boolean success;
+    String path;
 
     if (parent != null) {
       // use savePath(), so that the intermediate directories are created
-      filename = parent.savePath(filename);
+      path = parent.savePath(filename);
 
     } else {
       File file = new File(filename);
       if (file.isAbsolute()) {
         // make sure that the intermediate folders have been created
         PApplet.createPath(file);
+        path = file.getAbsolutePath();
       } else {
         String msg =
-            "PImage.save() requires an absolute path. " +
-                "Use createImage(), or pass savePath() to save().";
+          "PImage.save() requires an absolute path. " +
+          "Use createImage(), or pass savePath() to save().";
         PGraphics.showException(msg);
+        return false;
       }
     }
-
-    // Make sure the pixel data is ready to go
-    loadPixels();
-
-    try {
-      final String lower = filename.toLowerCase();
-
-      if (saveImpl(filename)) {
-        return true;
-      }
-
-      if (lower.endsWith(".tga")) {
-        OutputStream os = new BufferedOutputStream(new FileOutputStream(filename), 32768);
-        success = saveTGA(os); //, pixels, width, height, format);
-        os.close();
-
-      } else {  // fall-through case is TIFF
-        // add a default extension and save uncompressed
-        // TODO this is the only place in the api that we mess w/ file names,
-        // and while arguably useful, seems like a weird precedent [fry 200816]
-        if (!lower.endsWith(".tif") && !lower.endsWith(".tiff")) {
-          filename += ".tif";
-        }
-        OutputStream os = new BufferedOutputStream(new FileOutputStream(filename), 32768);
-        success = saveTIFF(os); //, pixels, width, height);
-        os.close();
-      }
-
-    } catch (IOException e) {
-      System.err.println("Error while saving image.");
-      e.printStackTrace();
-      success = false;
-    }
-    return success;
+    return saveImpl(path);
   }
 
 
@@ -3376,11 +3345,42 @@ int testFunction(int dst, int src) {
    * @param path must be a full path (not relative or simply a filename)
    */
   protected boolean saveImpl(String path) {
-    // TODO Imperfect/temporary solution for current 4.x releases
-    // https://github.com/processing/processing4/wiki/Exorcising-AWT
-    //if (!PApplet.disableAWT) {  // TODO necessary? will this trigger NEWT?
-    return ShimAWT.saveImage(this, path);
-    //}
-    //return false;
+    // Make sure the pixel data is ready to go
+    loadPixels();
+    boolean success;
+
+    try {
+      final String lower = path.toLowerCase();
+
+      if (lower.endsWith(".tga")) {
+        OutputStream os = new BufferedOutputStream(new FileOutputStream(path), 32768);
+        success = saveTGA(os); //, pixels, width, height, format);
+        os.close();
+
+        /*
+      } else {  // fall-through case is TIFF
+        // Add a default extension and save uncompressed.
+        // This is the only place in the API that we mess
+        // with file names, and while arguably useful,
+        // it seems like a weird outlier. [fry 200816]
+        if (!lower.endsWith(".tif") && !lower.endsWith(".tiff")) {
+          path += ".tif";
+        }
+        OutputStream os = new BufferedOutputStream(new FileOutputStream(path), 32768);
+        success = saveTIFF(os); //, pixels, width, height);
+        os.close();
+        */
+
+      } else {
+        // TODO Imperfect, possibly temporary solution for 4.x releases
+        //      https://github.com/processing/processing4/wiki/Exorcising-AWT
+        success = ShimAWT.saveImage(this, path);
+      }
+    } catch (IOException e) {
+      System.err.println("Error while saving " + path);
+      e.printStackTrace();
+      success = false;
+    }
+    return success;
   }
 }

core/src/processing/opengl/PGraphicsOpenGL.java

@@ -760,16 +760,29 @@ public class PGraphicsOpenGL extends PGraphics {
     // ASYNC save frame using PBOs not yet available on Android
     //return super.save(filename);
 
+    // In 4.0 beta 5, the loadPixels() call is moved into saveImpl(),
+    // otherwise it undermines part of the point to having optimized
+    // image writing methods in subclasses (which presumably might be
+    // able to write directly from frame buffer to file).
+    loadPixels();
+
     if (getHint(DISABLE_ASYNC_SAVEFRAME)) {
-      // Act as an opaque surface for the purposes of saving.
       if (primaryGraphics) {
+        // Act as an opaque surface while saving
         int prevFormat = format;
         format = RGB;
+        if (pixels == null) {
+          // Workaround for an NPE caused by resize events:
+          // https://github.com/processing/processing4/issues/162
+          // But there's a larger problem at play here:
+          // https://github.com/processing/processing4/issues/385
+          System.err.println("Skipping save() because pixels not ready.");
+          return false;
+        }
         boolean result = super.saveImpl(filename);
         format = prevFormat;
         return result;
       }
-
       return super.saveImpl(filename);
     }
 
@@ -798,6 +811,7 @@ public class PGraphicsOpenGL extends PGraphics {
       asyncPixelReader.readAndSaveAsync(parent.sketchFile(filename));
 
       if (needEndDraw) endDraw();
+
     } else {
       // async transfer is not supported or
       // pixels are already in memory, just do async save
@@ -5801,8 +5815,7 @@ public class PGraphicsOpenGL extends PGraphics {
   // LOAD/UPDATE TEXTURE
 
 
-  // Loads the current contents of the renderer's drawing surface into the
-  // its texture.
+  // Load the current contents of the drawing surface into a texture.
   public void loadTexture() {
     boolean needEndDraw = false;
     if (!drawing) {

core/todo.txt

@@ -12,11 +12,14 @@ X   this may be because of gaps between EDT and updates
 X   fixing the surface.setSize() issue may fix it
 X   https://github.com/processing/processing4/issues/386
 
+_ rework saveImpl() for images and how it interacts with the ShimAWT default
+_ remove the ancient TIFF saving code
 
 _ CODED is inconsistent between the default renderer and P2D/P3D
 _   includes recommendations how to fix
 _   https://github.com/processing/processing4/issues/376
 
+
 macos-aarch64
 _ JOGL is broken on aarch64
 _   https://github.com/processing/processing4/issues/370