imglib / imglib2-ij

Translation between ImgLib & ImageJ data structures (both 1.x and 2)
Other
4 stars 8 forks source link

PlugInFilterRunner breaks Undo for single-image TYPE_CONVERSION #15

Closed mountain-maennlein closed 6 years ago

mountain-maennlein commented 6 years ago

PlugInFilterRunner breaks Undo for certain code paths.

I am looking at the code in PlugInFilterRunner.java, from:

https://imagej.nih.gov/ij/developer/source/ij/plugin/filter/PlugInFilterRunner.java.html

It appears that the bug is in line 115. Lines 115--120 of the code are:

                if ((flags&PlugInFilter.NO_CHANGES)==0) {   // (filters doing no modifications don't change undo status)
                    if (snapshotDone)
                        Undo.setup(Undo.FILTER, imp);
                    else
                        Undo.reset();
                }

Line 115 tests for PlugInFilter.NO_CHANGES NOT being set, which means that the plugin has stated that it WILL (at least potentially) change the image. If the condition tests true (that is, NO_CHANGES is not set), then the subsequent lines incorrectly clear the Undo information.

I do not understand all of the code paths here, but, simplistically, it would appear that replacing line 115 with:

                if ((flags&PlugInFilter.NO_CHANGES)!=0) {   // (filters doing no modifications don't change undo status)

would fix the bug.

As a workaround, a (single-image) TYPE_CONVERSION PlugInFilter can return, e.g.:

   return DOES_8G | NO_CHANGES;

from its setup() method (falsely saying that it won't change the image), in effect, reversing the incorrect NO_CHANGES test in PlugInFilterRunner, and preventing PlugInFilterRunner from clearing the Undo information.

Here is code for a plugin that illustrates the bug and the workaround (but not the proposed actual fix):

// PlugInFilterUndo.java

// contents of plugins.config file:
//
// MM, "PlugInFilterUndo", mm.PlugInFilterUndo("")
// MM, "PlugInFilterUndo -- NO_CHANGES", mm.PlugInFilterUndo("NO_CHANGES")

package mm;

import ij.ImagePlus;
import ij.Undo;
import ij.plugin.filter.PlugInFilter;
import ij.process.ImageProcessor;
import ij.process.ShortProcessor;

/**
 * example for broken undo in PlugInFilterRunner
 */

public class PlugInFilterUndo implements PlugInFilter {

  private ImagePlus imp = null;

  public int setup (String arg, ImagePlus image) {
    imp = image;  // remember image for run method
    if (arg.equals ("NO_CHANGES"))  return DOES_8G | NO_CHANGES;
    else                            return DOES_8G;
  }

  public void run (ImageProcessor ip) {
    ShortProcessor sp = imp.getProcessor().convertToShortProcessor();
    short pixels[] = (short[]) sp.getPixels();
    for (int i = 0; i < pixels.length; i++)
      pixels[i] = (short) (((i / 32) % 2 == 0) ? 16384 : 49152);
    Undo.setup (Undo.TYPE_CONVERSION, imp);
    imp.setProcessor (sp);
    return;
  }

}

To run this code, build and install the plugin (including the plugins.config given as comments in the above java code.) Then open (or create) an 8-bit image and run the command:

MM > PlugInFilterUndo -- NO_CHANGES

This version uses the "NO_CHANGES" workaround, and will convert the image to a 16-bit image (and change the values of the pixels). Now running:

Edit > Undo

will revert the image back to the 8-bit original.

Contrariwise, running the version without the workaround:

MM > PlugInFilterUndo

will perform the same conversion to a 16-bit image, but, because of the PlugInFilterRunner bug, subsequently running:

Edit > Undo

will now not work.

This is all analyzed and tested with:

ImageJ 2.0.0-rc-65/1.52b; Java 1.8.0_66 [64-bit]; Linux 4.4.0-121-generic;

on:

ubuntu 16.04 LTS, 64-bit

Thanks, mm

imagejan commented 6 years ago

This issue belongs in the imagej/imagej1 repository.

rasband commented 6 years ago

The PlugInFilterRunner appears to be working as expected. Run a plugin like this

public class Inverter_ implements PlugInFilter { private ImagePlus imp = null;

public int setup (String arg, ImagePlus image) { imp = image; return DOES_8G; } public void run (ImageProcessor ip) { ip.invert(); imp.updateAndDraw(); } }

on an 8-bit image and you can undo the inversion by pressing "z" (Edit>Undo). Change "return DOES_8G;" to "return DOES_8G|NO_CHANGES;" and pressing "z" does nothing because PlugInFilterRunner has not setup an undo buffer. The PlugInFilter interface is not designed to support undo of images converted to a different type. In this case it, is better to use the PlugIn interface.

mountain-maennlein commented 6 years ago

This issue was moved to imagej/imagej1#58