Ticket #688 (closed defect: fixed)

Opened 8 years ago

Last modified 8 years ago

Macros that open images do not display them

Reported by: afraser Owned by: bdezonia
Priority: critical Milestone: imagej2-b1-initial
Component: Legacy Compatibility Version:
Severity: minor Keywords:
Cc: curtis Blocked By:
Blocking:

Description (last modified by afraser) (diff)

Note: this bug makes it difficult to write macros that easily reproduce a certain behavior, so I marked it as major.

Macros that open images run("AuPbSn 40 (56K)"); do not show them in a display. However, if you run "invert" or presumably any other command on the current image, the image appears.

to reproduce:
Plugins > New > Macro
--paste: run("AuPbSn 40 (56K)");
Macros > Run Macro OR Macros > Evaluate Line
--no display appears but you'll see the status bar indicate that it's loading
cmd+shift+i
--display appears (image now inverted)

Close the text window and reopen Plugins > New > Macro
The image pops up.

It seems this problem is because the plugin being run through the text menu is not being run though the legacy layer.

Change History

comment:1 Changed 8 years ago by afraser

  • Priority changed from minor to major
  • Description modified (diff)

comment:2 Changed 8 years ago by afraser

  • Description modified (diff)

comment:3 Changed 8 years ago by bdezonia

  • Component changed from ij-ext to ij-legacy

comment:4 Changed 8 years ago by curtis

  • Owner changed from curtis to bdezonia
  • Priority changed from major to critical
  • Status changed from new to assigned

comment:5 Changed 8 years ago by bdezonia

Cannot get the image to appear via running invert or anything else.

Regardless the issue is that the plugin hatches a thread and returns. The thread does the image creation work but LegacyPlugin never knows about it. From LegacyPlugin's view the plugin did nothing.

Could monitor threads hatched by a plugin and only gather outputs after threads complete. This would change LegacyOutputTracker's image list to not be ThreadLocal. This is probably okay.

comment:6 Changed 8 years ago by bdezonia

This is actually more complicated. The scenario given will not work in that the macros thread does not terminate. So executing one line will still never return and monitoring threads will be ineffective. Monitoring threads seems bug prone also.

What needs to happen is that ImagePlus::show() hooking in ImagePlusMethods should not register that an image has changed but rather it should hatch a display if needed.

comment:7 follow-up: ↓ 10 Changed 8 years ago by bdezonia

  • Cc curtis added

Further playing with this. Made an uncommitted test change to LegacyService::legacyImageChanged() to always register the imageplus. This will create a display if it oesn't exist.

Code solves immediate problem: the example bug is fixed. But fix is incomplete as any update of data from within example macro will not harmonize and thus display will be incorrect. It might also be possible to create zombie ImagePluses that steal memory (via LegacyOutputTracker being told at wrong time to save reference).

We have treated legacy plugins as having outputs when in fact they don't. We've captured their drawing methods and just recorded the imageplus as an output.

In general our solution cannot handle multithreaded plugins. A better approach: more nearly mirror IJ1. Make LegacyPlugin have NO outputs. Have application own a thread running some new harmonizer that is a combination of LegacyImageMap and Harmonizer. ImagePlusMethods could request harmonization upon detection of draw/show kinds of calls. This new harmonizer would always be translating back and forth as needed. LegacyPlugin::run() could request harmonizations before calling the IJ1 plugin. Thus multithreaded plugins always display data as expected (unless they are highly interactive).

This needs further discussion before we make any decisions.

comment:8 Changed 8 years ago by bdezonia

See also #760

comment:9 Changed 8 years ago by bdezonia

After further discussion it was decided that the idea of having a separate thread running harmonization was ideally necessary but pragmatically not the best approach. It is covering what are expected to be edge cases. If we find there are issues with other IJ1 commands then we can revisit this idea.

At one time we were doing immediate harmonization and it was slow and bug prone. Thats why we started aggregating results. Now to fix this we'd need to make the spearate thread idea gather info on changed imagepluses and harmonize only after updateAndDraw() (or similar method) activity dropped off. There was some question that due to plugins ability to modify pixels by reference that even this would be an issue (though I can't see the scenario).

For now will implement a partial solution that involves joining all threads spawned by a plugin before harmonizing back to IJ2.

comment:10 in reply to: ↑ 7 Changed 8 years ago by bdezonia

Replying to bdezonia:

Further playing with this. Made an uncommitted test change to LegacyService::legacyImageChanged() to always register the imageplus. This will create a display if it oesn't exist.

Code solves immediate problem: the example bug is fixed. But fix is incomplete as any update of data from within example macro will not harmonize and thus display will be incorrect. It might also be possible to create zombie ImagePluses that steal memory (via LegacyOutputTracker being told at wrong time to save reference).

We have treated legacy plugins as having outputs when in fact they don't. We've captured their drawing methods and just recorded the imageplus as an output.

In general our solution cannot handle multithreaded plugins. A better approach: more nearly mirror IJ1. Make LegacyPlugin have NO outputs. Have application own a thread running some new harmonizer that is a combination of LegacyImageMap and Harmonizer. ImagePlusMethods could request harmonization upon detection of draw/show kinds of calls. This new harmonizer would always be translating back and forth as needed. LegacyPlugin::run() could request harmonizations before calling the IJ1 plugin. Thus multithreaded plugins always display data as expected (unless they are highly interactive).

This needs further discussion before we make any decisions.

Note that harmnonizer thread only needs to update displays from imagepluses. The other direction can be controled by legacyplugin.

comment:11 Changed 8 years ago by curtis

Thanks Barry. The problem with immediate synchronization is that—because pixel values can be modified directly in the array, with no possibility of detecting it—any synchronization must always compare and/or update pixel values, which is slow. In an ideal world, we would always know *how* an image changed, so that we can mirror only those changes in a performant way to the other space. For example, if the image autoscaling min/max values change, we merely update those, and not the pixel values.

Unfortunately, the pixel values in an array-backed image can change at any time without warning. So originally we decided it would be safest to aggregate all changes and do a full synchronization at the completion of the plugin execution. If you have an idea for a better way to do it, we should definitely discuss—perhaps after the first though as you note above.

comment:12 Changed 8 years ago by bdezonia

  • Status changed from assigned to closed
  • Resolution set to fixed

Updated LegacyPlugin and related classes to create displays as needed and wait for hatched threads to terminate before synchronizing displays with ImagePluses. It works but print statements show it can take significant time for the threads to terminate (try Compile and Run for instance). Nonetheless this works and no zombies get created. Keep an eye on plugin running time to see if it becomes a problem.

Relevant changes in b50bfa536f5793433a7a8e7e3763eb1adb1a7bc0, a7109f7ebaf8390aa4faa6992826448b1a7abea0, and 2aefe82e261569be2236549eb47544bb15e2e4ae.

comment:13 Changed 8 years ago by bdezonia

One more relevant change: 55ae6857f6fd31f5199844b4892de05d285e0c84

Note: See TracTickets for help on using tickets.