--- old/modules/graphics/src/main/java/com/sun/javafx/sg/prism/NGRegion.java 2013-11-15 11:04:57.392261390 +0100 +++ new/modules/graphics/src/main/java/com/sun/javafx/sg/prism/NGRegion.java 2013-11-15 11:04:57.264261386 +0100 @@ -1681,9 +1681,15 @@ * then fill that shape without additional transforms. */ private Shape resizeShape(float topOffset, float rightOffset, float bottomOffset, float leftOffset) { + return resizeShape(shape, scaleShape, centerShape, width, height, topOffset, rightOffset, bottomOffset, leftOffset); + } + + public static Shape resizeShape(Shape shape, boolean scale, boolean center, + float width, float height, + float topOffset, float rightOffset, float bottomOffset, float leftOffset) { // The bounds of the shape, before any centering / scaling takes place final RectBounds bounds = shape.getBounds(); - if (scaleShape) { + if (scale) { // First we need to modify the transform to scale the shape so that it will fit // within the insets. SCRATCH_AFFINE.setToIdentity(); @@ -1695,11 +1701,11 @@ SCRATCH_AFFINE.scale(w / bounds.getWidth(), h / bounds.getHeight()); // If we also need to center it, we need to adjust the transform so as to place // the shape in the center of the bounds - if (centerShape) { + if (center) { SCRATCH_AFFINE.translate(-bounds.getMinX(), -bounds.getMinY()); } return SCRATCH_AFFINE.createTransformedShape(shape); - } else if (centerShape) { + } else if (center) { // We are only centering. In this case, what we want is for the // original shape to be centered. If there are offsets (insets) // then we must pre-scale about the center to account for it. --- old/modules/graphics/src/main/java/javafx/scene/layout/Region.java 2013-11-15 11:04:57.788261402 +0100 +++ new/modules/graphics/src/main/java/javafx/scene/layout/Region.java 2013-11-15 11:04:57.672261399 +0100 @@ -66,7 +66,9 @@ import com.sun.javafx.css.converters.SizeConverter; import com.sun.javafx.geom.BaseBounds; import com.sun.javafx.geom.PickRay; +import com.sun.javafx.geom.RectBounds; import com.sun.javafx.geom.Vec2d; +import com.sun.javafx.geom.transform.Affine2D; import com.sun.javafx.geom.transform.BaseTransform; import com.sun.javafx.scene.DirtyBits; import com.sun.javafx.scene.input.PickResultChooser; @@ -867,6 +869,7 @@ if (value != _width) { _width = value; boundingBox = null; + scaledShape = null; impl_layoutBoundsChanged(); impl_geomChanged(); impl_markDirty(DirtyBits.NODE_GEOMETRY); @@ -925,6 +928,8 @@ // is now going to recompute the height eagerly. The cost of excessive and // unnecessary bounds changes, however, is relatively high. boundingBox = null; + // Scaled shape might depend on width/height if it's centered or scaled. + scaledShape = null; // Note: although impl_geomChanged will usually also invalidate the // layout bounds, that is not the case for Regions, and both must // be called separately. @@ -1240,11 +1245,13 @@ } // Update our reference to the old shape _shape = value; + scaledShape = null; } } @Override public void run() { impl_geomChanged(); + scaledShape = null; impl_markDirty(DirtyBits.REGION_SHAPE); } }; @@ -1272,6 +1279,7 @@ } @Override public void invalidated() { impl_geomChanged(); + scaledShape = null; impl_markDirty(DirtyBits.REGION_SHAPE); } }; @@ -1301,6 +1309,7 @@ } @Override public void invalidated() { impl_geomChanged(); + scaledShape = null; impl_markDirty(DirtyBits.REGION_SHAPE); } }; @@ -2457,7 +2466,17 @@ @Override public NGNode impl_createPeer() { return new NGRegion(); } - + + private com.sun.javafx.geom.Shape scaledShape; + + private com.sun.javafx.geom.Shape getShapeScaledAndPositioned() { + if (scaledShape == null) { + scaledShape = NGRegion.resizeShape(_shape.impl_configShape(), isScaleShape(), isCenterShape(), + (float)getWidth(), (float)getHeight(), 0f,0f,0f,0f); + } + return scaledShape; + } + /** * @treatAsPrivate implementation detail * @deprecated This is an internal API that is not intended for use and will be removed in the next version @@ -2477,15 +2496,9 @@ final double maxRadius = Math.min(x2 / 2.0, y2 / 2.0); // First check the shape. Shape could be impacted by scaleShape & positionShape properties. - // This is going to be ugly! The problem is that basically all the scale / position operations - // have to be implemented here in Region, whereas right now they are all implemented in - // NGRegion. Drat. Basically I can't implement this properly until I have a way to get the - // geometry backing an arbitrary FX shape. For example, in this case I need an NGShape peer - // of this shape so that I can resize it as appropriate for these picking tests. - // Lacking that, for now, I will simply check the shape (so that picking works for pie charts) - // Bug is filed as RT-27775. + // We are calling NGRegion static method here because we lack a better place for the code. if (_shape != null) { - return _shape.contains(localX, localY); + return getShapeScaledAndPositioned().contains((float)localX, (float)localY); } // OK, there was no background shape, so I'm going to work on the principle of --- old/modules/graphics/src/test/java/javafx/scene/layout/RegionPickTest.java 2013-11-15 11:04:58.148261414 +0100 +++ new/modules/graphics/src/test/java/javafx/scene/layout/RegionPickTest.java 2013-11-15 11:04:58.044261410 +0100 @@ -27,6 +27,7 @@ import javafx.geometry.Insets; import javafx.scene.paint.Color; +import javafx.scene.shape.Circle; import org.junit.Before; import org.junit.Test; @@ -512,5 +513,84 @@ * * *************************************************************************/ - // TODO implement along with fix for RT-27775 + @Test public void pickingSimpleShape() { + region.setPickOnBounds(false); + region.setScaleShape(false); + region.setCenterShape(false); + region.setBackground(new Background(new BackgroundFill(Color.RED, CornerRadii.EMPTY, Insets.EMPTY))); + + region.setShape(new Circle(50, 50, 30)); + + assertFalse(region.contains(X + 50, Y + 81)); + assertFalse(region.contains(X + 50, Y + 19)); + assertFalse(region.contains(X + 81, Y + 50)); + assertFalse(region.contains(X + 19, Y + 50)); + assertTrue(region.contains(X + 50, Y + 79)); + assertTrue(region.contains(X + 50, Y + 21)); + assertTrue(region.contains(X + 79, Y + 50)); + assertTrue(region.contains(X + 21, Y + 50)); + + } + + @Test public void pickingCenteredShape() { + region.setPickOnBounds(false); + region.setScaleShape(false); + region.setCenterShape(true); + region.setBackground(new Background(new BackgroundFill(Color.RED, CornerRadii.EMPTY, Insets.EMPTY))); + + region.setShape(new Circle(0, 0, 30)); + + assertFalse(region.contains(X + 50, Y + 81)); + assertFalse(region.contains(X + 50, Y + 19)); + assertFalse(region.contains(X + 81, Y + 50)); + assertFalse(region.contains(X + 19, Y + 50)); + assertTrue(region.contains(X + 50, Y + 79)); + assertTrue(region.contains(X + 50, Y + 21)); + assertTrue(region.contains(X + 79, Y + 50)); + assertTrue(region.contains(X + 21, Y + 50)); + + } + + @Test public void pickingScaledShape() { + region.setPickOnBounds(false); + region.setScaleShape(true); + region.setCenterShape(false); + region.setBackground(new Background(new BackgroundFill(Color.RED, CornerRadii.EMPTY, Insets.EMPTY))); + + region.setShape(new Circle(0, 0, 30)); + + assertFalse(region.contains(X, Y + HEIGHT / 2 + 1)); + assertFalse(region.contains(X, Y - HEIGHT / 2 - 1)); + assertFalse(region.contains(X + WIDTH / 2 + 1, Y)); + assertFalse(region.contains(X - WIDTH / 2 - 1, Y)); + + assertTrue(region.contains(X + 1, Y + HEIGHT / 2 - 1)); + assertTrue(region.contains(X + WIDTH / 2 - 1, Y)); + + // Even though the shape is there, these points are outside of region's bounds, so it wouldn't be really picked. + assertFalse(region.contains(X + 1, Y - HEIGHT / 2 + 1)); + assertFalse(region.contains(X - WIDTH / 2 + 1, Y)); + + } + + @Test public void pickingScaledAndCenteredShape() { + region.setPickOnBounds(false); + region.setScaleShape(true); + region.setCenterShape(true); + region.setBackground(new Background(new BackgroundFill(Color.RED, CornerRadii.EMPTY, Insets.EMPTY))); + + region.setShape(new Circle(0, 0, 30)); + + assertFalse(region.contains(X + WIDTH / 2, Y + HEIGHT + 1)); + assertFalse(region.contains(X + WIDTH / 2, Y - 1)); + assertFalse(region.contains(X + WIDTH + 1, Y + HEIGHT / 2)); + assertFalse(region.contains(X - 1, Y + HEIGHT / 2)); + + assertTrue(region.contains(X + WIDTH / 2, Y + HEIGHT - 1)); + assertTrue(region.contains(X + WIDTH / 2, Y + 1)); + assertTrue(region.contains(X + WIDTH - 1, Y + HEIGHT / 2)); + assertTrue(region.contains(X + 1, Y + HEIGHT / 2)); + + } + }