Merge pull request qupath#379 from petebankhead/pete-m7

Pete m7

CHANGELOG.md

@@ -1,3 +1,13 @@
+## Version 0.2.0-m7
+This is a *milestone* (i.e. still in development) version made available to try out new features early.
+* Fixed bug that could cause QuPath to freeze when selecting objects with a mini-viewer active, see https://github.com/qupath/qupath/issues/377
+* Improved performance converting shapes to geometries, see https://github.com/qupath/qupath/issues/378
+* Improved robustness when drawing complex shapes, see https://github.com/qupath/qupath/issues/376
+* Improved stability when script directories cannot be found, see https://github.com/qupath/qupath/issues/373
+* Prompt to save each image when closing a project with multiple viewers active
+* Updated 'Rotate annotation' command to use JTS
+
+
 ## Version 0.2.0-m6
 This is a *milestone* (i.e. still in development) version made available to try out new features early.
 ### Important bug fix!

STARTUP.md

@@ -1,7 +1,7 @@
-QuPath v0.2.0-m6
+QuPath v0.2.0-m7
 ================
 
-Welcome to QuPath v0.2.0-m6!
+Welcome to QuPath v0.2.0-m7!
 
 You can read more about what is different at https://qupath.github.io/
 

VERSION

@@ -1 +1 @@
-0.2.0-m6
+0.2.0-m7

qupath-core-processing/src/main/java/qupath/imagej/tools/IJTools.java

@@ -75,7 +75,6 @@
 import qupath.lib.regions.ImagePlane;
 import qupath.lib.regions.ImageRegion;
 import qupath.lib.regions.RegionRequest;
-import qupath.lib.roi.AreaROI;
 import qupath.lib.roi.EllipseROI;
 import qupath.lib.roi.LineROI;
 import qupath.lib.roi.PointsROI;

qupath-core/src/main/java/qupath/lib/images/servers/LabeledImageServer.java

@@ -35,6 +35,7 @@
 import qupath.lib.images.servers.TileRequest;
 import qupath.lib.images.servers.ImageServerBuilder.ServerBuilder;
 import qupath.lib.objects.PathAnnotationObject;
+import qupath.lib.objects.PathDetectionObject;
 import qupath.lib.objects.PathObject;
 import qupath.lib.objects.classes.PathClass;
 import qupath.lib.objects.classes.PathClassFactory;
@@ -64,7 +65,7 @@ public class LabeledImageServer extends AbstractTileableImageServer implements G
 	private ImageServerMetadata originalMetadata;
 
 	private PathObjectHierarchy hierarchy;
-
+		
 	private ColorModel colorModel;
 	private boolean multichannelOutput;
 
@@ -150,6 +151,8 @@ private static class LabeledServerParameters {
 		 */
 		private PathClass unannotatedClass = PathClassFactory.getPathClass("Unannotated " + UUID.randomUUID().toString());
 
+		private Class<? extends PathObject> objectClass = PathAnnotationObject.class;
+
 		private float lineThickness = 1.0f;
 		private Map<PathClass, Integer> labels = new LinkedHashMap<>();
 		private Map<PathClass, Integer> boundaryLabels = new LinkedHashMap<>();
@@ -163,6 +166,7 @@ private static class LabeledServerParameters {
 		LabeledServerParameters(LabeledServerParameters params) {
 			this.unannotatedClass = params.unannotatedClass;
 			this.lineThickness = params.lineThickness;
+			this.objectClass = params.objectClass;
 			this.labels = new LinkedHashMap<>(params.labels);
 			this.boundaryLabels = new LinkedHashMap<>(params.boundaryLabels);
 			this.labelColors = new LinkedHashMap<>(params.labelColors);
@@ -191,6 +195,27 @@ public Builder(ImageData<BufferedImage> imageData) {
 			this.imageData = imageData;
 		}
 
+		/**
+		 * Use detections rather than annotations for labels.
+		 * The default is to use annotations.
+		 * @return
+		 * @see #useAnnotations()
+		 */
+		public Builder useDetections() {
+			params.objectClass = PathDetectionObject.class;
+			return this;
+		}
+
+		/**
+		 * Use annotations for labels. This is the default.
+		 * @return
+		 * @see #useDetections()
+		 */
+		public Builder useAnnotations() {
+			params.objectClass = PathAnnotationObject.class;
+			return this;
+		}
+
 		/**
 		 * Specify downsample factor. This is <i>very</i> important because it defines 
 		 * the resolution at which shapes will be drawn and the line thickness is determined.
@@ -428,7 +453,7 @@ protected String createID() {
 	 */
 	@Override
 	public boolean isEmptyRegion(RegionRequest request) {
-		return !hierarchy.getObjectsForRegion(PathAnnotationObject.class, request, null).stream().anyMatch( p -> {
+		return !hierarchy.getObjectsForRegion(params.objectClass, request, null).stream().anyMatch( p -> {
 				return params.labels.keySet().contains(p.getPathClass()) || params.boundaryLabels.keySet().contains(p.getPathClass());
 			});
 	}
@@ -466,7 +491,7 @@ protected BufferedImage createDefaultRGBImage(int width, int height) {
 	protected BufferedImage readTile(TileRequest tileRequest) throws IOException {
 		long startTime = System.currentTimeMillis();
 
-		var pathObjects = hierarchy.getObjectsForRegion(PathAnnotationObject.class, tileRequest.getRegionRequest(), null);
+		var pathObjects = hierarchy.getObjectsForRegion(params.objectClass, tileRequest.getRegionRequest(), null);
 		BufferedImage img;
 		if (multichannelOutput) {
 			img = createMultichannelTile(tileRequest, pathObjects);
@@ -534,8 +559,10 @@ private BufferedImage createBinaryTile(TileRequest tileRequest, Collection<PathO
 				if (entry.getValue() != label)
 					continue;
 				var pathClass = entry.getKey();
+				boolean noClass = pathClass == null || pathClass == PathClassFactory.getPathClassUnclassified();
 				for (var pathObject : pathObjects) {
-					if (pathObject.getPathClass() == pathClass) {
+					if (pathObject.getPathClass() == pathClass ||
+							(noClass && pathObject.getPathClass() == null)) {
 						var roi = pathObject.getROI();
 						if (roi.isArea())
 							g2d.fill(roi.getShape());
@@ -605,10 +632,12 @@ private BufferedImage createIndexedColorTile(TileRequest tileRequest, Collection
 			// We want to order consistently to avoid confusing overlaps
 			for (var entry : params.labels.entrySet()) {
 				var pathClass = entry.getKey();
+				boolean noClass = pathClass == null || pathClass == PathClassFactory.getPathClassUnclassified();
 				int c = entry.getValue();
 				color = ColorToolsAwt.getCachedColor(c, c, c);
 				for (var pathObject : pathObjects) {
-					if (pathObject.getPathClass() == pathClass) {
+					if (pathObject.getPathClass() == pathClass ||
+							(noClass && pathObject.getPathClass() == null)) {
 						var roi = pathObject.getROI();
 						g2d.setColor(color);
 						if (roi.isArea())

qupath-core/src/main/java/qupath/lib/roi/GeometryTools.java

@@ -88,6 +88,25 @@ public static Geometry shapeToGeometry(Shape shape) {
 //    	return ShapeReader.read(shape, DEFAULT_INSTANCE.flatness, DEFAULT_INSTANCE.factory);
     }
 
+
+    /**
+     * Create a rectangular Geometry for the specified bounding box.
+     * @param x
+     * @param y
+     * @param width
+     * @param height
+     * @return
+     */
+    public static Geometry createRectangle(double x, double y, double width, double height) {
+    	var shapeFactory = new GeometricShapeFactory(DEFAULT_FACTORY);
+		shapeFactory.setEnvelope(
+				new Envelope(
+						x, x+width, y, y+height)
+				);
+		return shapeFactory.createRectangle();
+    }
+
+
     /**
      * Convert a JTS Geometry to a QuPath ROI.
      * @param geometry
@@ -138,16 +157,51 @@ public static Geometry regionToGeometry(ImageRegion region) {
      * @return
      */
     public static Geometry union(Collection<? extends Geometry> geometries) {
+    	return union(geometries, false);
+    }
+
+
+    /**
+     * Calculate the union of multiple Geometry objects.
+     * @param geometries
+     * @param fastUnion if true, it can be assumed that the Geometries are valid and cannot overlap. This may permit a faster union operation.
+     * @return
+     */
+    private static Geometry union(Collection<? extends Geometry> geometries, boolean fastUnion) {
     	if (geometries.isEmpty())
     		return DEFAULT_INSTANCE.factory.createPolygon();
     	if (geometries.size() == 1)
     		return geometries.iterator().next();
-//    	return DEFAULT_INSTANCE.factory.createGeometryCollection(geometries.toArray(Geometry[]::new)).buffer(0);
-    	return UnaryUnionOp.union(geometries);
+    	if (fastUnion) {
+    		var geometryArray = geometries.toArray(Geometry[]::new);
+    		double areaSum = Arrays.stream(geometryArray).mapToDouble(g -> g.getArea()).sum();
+    		var union = DEFAULT_INSTANCE.factory.createGeometryCollection(geometryArray).buffer(0);
+    		double areaUnion = Arrays.stream(geometryArray).mapToDouble(g -> g.getArea()).sum();
+    		if (GeneralTools.almostTheSame(areaSum, areaUnion, 0.00001))
+    			return union;
+    		logger.warn("Fast union failed with different areas ({} before vs {} after)", areaSum, areaUnion);
+    	}
+    	try {
+    		return UnaryUnionOp.union(geometries);
+    	} catch (Exception e) {
+    		// Throw exception if we have no other options
+    		if (fastUnion)
+    			throw e;
+    		else {
+    			// Try again with other path
+    			logger.warn("Exception attempting default union: {}", e.getLocalizedMessage());
+    			return union(geometries, true);
+    		}
+    	}
     }
 
 
-
+    /**
+     * Ensure a GeometryCollection contains only Geometries of the same type (Polygonal, Lineal or Puntal).
+     * Other geometries (with lower dimension) are discarded.
+     * @param geometry
+     * @return
+     */
     public static Geometry homogenizeGeometryCollection(Geometry geometry) {
     	if (geometry instanceof Polygonal || geometry instanceof Puntal || geometry instanceof Lineal) {
     		return geometry;
@@ -421,7 +475,9 @@ private static Geometry convertAreaToGeometry(final Area area, final AffineTrans
 				};
 				areaTempSigned += 0.5 * (x0 * y1 - x1 * y0);
 				// Add polygon if it has just been closed
-				if (closed) {
+				if (closed && points.size() == 1) {
+					logger.warn("Cannot create polygon from cordinate array of length 1!");
+				} else if (closed) {
 					points.closeRing();
 					Coordinate[] coords = points.toCoordinateArray();
 	//				for (Coordinate c : coords)
@@ -433,14 +489,36 @@ private static Geometry convertAreaToGeometry(final Area area, final AffineTrans
 					TopologyValidationError error = new IsValidOp(polygon).getValidationError();
 					if (error != null) {
 						logger.debug("Invalid polygon detected! Attempting to correct {}", error.toString());
+
 						double areaBefore = polygon.getArea();
-						double distance = GeometrySnapper.computeOverlaySnapTolerance(polygon);
-						Geometry geom = GeometrySnapper.snapToSelf(polygon,
-								distance,
-								true);
+
+//						// Faster method of fixing polygons... main disadvantage is that it doesn't always work
+//						var polygonizer = new Polygonizer();
+//						var union = factory.createLineString(coords).union(factory.createPoint(coords[0]));
+//						polygonizer.add(union);
+//						var polygons = polygonizer.getPolygons();
+//						var iter2 = polygons.iterator();
+//						Geometry geom = (Geometry)iter2.next();
+//						while (iter2.hasNext())
+//							geom = geom.symDifference((Geometry)iter2.next());
+////						factory.buildGeometry(polygons);
+
+						// Try fast buffer trick to make valid (but sometimes this can 'break', e.g. with bow-tie shapes)
+						Geometry geom = polygon.buffer(0);
 						double areaAfter = geom.getArea();
-						if (!GeneralTools.almostTheSame(areaBefore, areaAfter, 0.001)) {
-							logger.debug("Unable to fix geometry (area before: {}, area after: {}, tolerance: {})", areaBefore, areaAfter, distance);
+						if (!GeneralTools.almostTheSame(areaBefore, areaAfter, 0.0001)) {
+							// Resort to the slow method of fixing polygons if we have to
+							logger.debug("Unable to fix Geometry with buffer(0) - will try snapToSelf instead");
+							double distance = GeometrySnapper.computeOverlaySnapTolerance(polygon);
+							geom = GeometrySnapper.snapToSelf(polygon,
+									distance,
+									true);
+							areaAfter = geom.getArea();
+						}
+
+						// Sanity check & warning if something went wrong
+						if (!GeneralTools.almostTheSame(areaBefore, areaAfter, 0.0001)) {
+							logger.warn("Unable to fix geometry (area before: {}, area after: {})", areaBefore, areaAfter);
 							logger.trace("Original geometry: {}", polygon);
 							logger.trace("Will attempt to proceed using {}", geom);
 						} else {
@@ -494,13 +572,13 @@ private static Geometry convertAreaToGeometry(final Area area, final AffineTrans
 			Geometry geometry;
 			Geometry geometryOuter;
 			if (holes.isEmpty()) {
-				// If we have no holes, just just the outer geometry
-				geometryOuter = union(outer);
+				// If we have no holes, just use the outer geometry
+				geometryOuter = union(outer, true);
 				geometry = geometryOuter;
 			} else if (outer.size() == 1) {
 				// If we just have one outer geometry, remove all the holes
-				geometryOuter = union(outer);
-				geometry = geometryOuter.difference(union(holes));
+				geometryOuter = union(outer, true);
+				geometry = geometryOuter.difference(union(holes, true));
 			} else {
 				// We need to handle holes... and, in particular, additional objects that may be nested within holes.
 				// To do that, we iterate through the holes and try to match these with the containing polygon, updating it accordingly.

qupath-core/src/main/java/qupath/lib/roi/RoiTools.java

@@ -450,6 +450,7 @@ public static Area getArea(final ROI roi) {
 	 * @return
 	 */
 	public static List<ROI> makeTiles(final ROI roi, final int tileWidth, final int tileHeight, final boolean trimToROI) {
+		// TODO: Convert to use JTS Geometries rather than AWT Areas.
 		// Create a collection of tiles
 		Rectangle bounds = AwtTools.getBounds(roi);
 		Area area = getArea(roi);
@@ -525,7 +526,7 @@ public static Collection<? extends ROI> computeTiledROIs(ROI parentROI, Immutabl
 
 		List<ROI> pathROIs = new ArrayList<>();
 
-		Area area = getArea(pathArea);
+		Geometry area = pathArea.getGeometry();
 
 		double xMin = bounds.getMinX();
 		double yMin = bounds.getMinY();
@@ -537,14 +538,17 @@ public static Collection<? extends ROI> computeTiledROIs(ROI parentROI, Immutabl
 		// Center the tiles
 		xMin = (int)(bounds.getCenterX() - (nx * w * .5));
 		yMin = (int)(bounds.getCenterY() - (ny * h * .5));
+
+		var plane = parentROI.getImagePlane();
 
 		for (int yi = 0; yi < ny; yi++) {
 			for (int xi = 0; xi < nx; xi++) {
 
 				double x = xMin + xi * w - overlap;
 				double y = yMin + yi * h - overlap;
 
-				Rectangle2D boundsTile = new Rectangle2D.Double(x, y, w + overlap*2, h + overlap*2);
+				var rect = ROIs.createRectangleROI(x, y, w + overlap*2, h + overlap*2, plane);
+				var boundsTile = rect.getGeometry();
 
 				//				double x = xMin + xi * w;
 				//				double y = yMin + yi * h;
@@ -553,18 +557,13 @@ public static Collection<? extends ROI> computeTiledROIs(ROI parentROI, Immutabl
 				//					logger.info(boundsTile);
 				ROI pathROI = null;
 				if (area.contains(boundsTile))
-					pathROI = ROIs.createRectangleROI(boundsTile.getX(), boundsTile.getY(), boundsTile.getWidth(), boundsTile.getHeight(), parentROI.getImagePlane());
-				else if (pathArea instanceof RectangleROI) {
-					Rectangle2D bounds2 = boundsTile.createIntersection(bounds);
-					pathROI = ROIs.createRectangleROI(bounds2.getX(), bounds2.getY(), bounds2.getWidth(), bounds2.getHeight(), parentROI.getImagePlane());
-				}
+					pathROI = rect;
 				else {
 					if (!area.intersects(boundsTile))
 						continue;
-					Area areaTemp = new Area(boundsTile);
-					areaTemp.intersect(area);
+					Geometry areaTemp = boundsTile.intersection(area);
 					if (!areaTemp.isEmpty())
-						pathROI = ROIs.createAreaROI(areaTemp, parentROI.getImagePlane());					
+						pathROI = GeometryTools.geometryToROI(areaTemp, plane);
 				}
 				if (pathROI != null)
 					pathROIs.add(pathROI);

qupath-gui-fx/src/main/java/qupath/lib/gui/QuPathGUI.java

@@ -178,10 +178,7 @@
 import javafx.stage.Window;
 import javafx.util.Duration;
 import jfxtras.scene.menu.CirclePopupMenu;
-import qupath.lib.algorithms.CoherenceFeaturePlugin;
-import qupath.lib.algorithms.HaralickFeaturesPlugin;
 import qupath.lib.algorithms.IntensityFeaturesPlugin;
-import qupath.lib.algorithms.LocalBinaryPatternsPlugin;
 import qupath.lib.algorithms.TilerPlugin;
 import qupath.lib.common.GeneralTools;
 import qupath.lib.common.ThreadTools;
@@ -4642,16 +4639,19 @@ public void setProject(final Project<BufferedImage> project) {
 		}
 
 		// Check if we want to save the current image; we could still veto the project change at this point
-		var viewer = getViewer();
-		var imageData = viewer.getImageData();
-		if (imageData != null) {
-			ProjectImageEntry<BufferedImage> entry = getProjectImageEntry(imageData);
-//			if (entry != null) {
-				if (!checkSaveChanges(imageData))
-					return;
-				getViewer().setImageData(null);
-//			} else
-//				ProjectImportImagesCommand.addSingleImageToProject(project, imageData.getServer(), null);
+		for (var viewer : getViewers()) {
+			if (viewer == null || !viewer.hasServer())
+				continue;
+			var imageData = viewer.getImageData();
+			if (imageData != null) {
+				ProjectImageEntry<BufferedImage> entry = getProjectImageEntry(imageData);
+	//			if (entry != null) {
+					if (!checkSaveChanges(imageData))
+						return;
+					viewer.setImageData(null);
+	//			} else
+	//				ProjectImportImagesCommand.addSingleImageToProject(project, imageData.getServer(), null);
+			}
 		}
 
 		// Confirm the URIs for the new project