Bug 284767

Description Marcel Romijn 2009-07-27 11:18:01 EDT

Build ID: I20090611-1540

We are using Layers added to Layers and we noticed that Layer.containsPoint() returned false for some children where it should return true.
After investigation, it turns out that the change for bug 259700 causes the regression.
In bug 259700, the creation of new Point objects was replaced by re-using a Point.SINGLETON.

See method Layer.containsPoint():
When useLocalCoordinates() returns true, translateFromParent(pt) will change the contents of Point.SINGLETON.
Since containsPoint() is recursive, a child figure can also change the contents of Point.SINGLETON while the parent is iterating over its children.
Therefore, for a second child, pt can have another value than for the first child.
So Point.SINGLETON cannot not be used in this situation.

Looking a bit further, it seems that a similar problem was solved in Figure.findDescendantAtExcluding as described in bug 81725.
Since bug 81725 uses a simple and elegant solution, that could also be used in this situation:

    public boolean containsPoint(int x, int y) {
    	if (isOpaque())
    		return super.containsPoint(x, y);
    	Point pt = Point.SINGLETON;
    	pt.setLocation(x, y);
    	translateFromParent(pt);
+    	x = pt.x;
+    	y = pt.y;
    	for (int i = 0; i < getChildren().size(); i++) {
    		IFigure child = (IFigure)getChildren().get(i);
~   		if (child.containsPoint(x, y))
    			return true;
    	}
    	return false;
    }


Note that the same problem could exist in other locations where statics are reused, like Figure.findMouseEventTargetInDescendantsAt().

Steps To Reproduce:

    public class LayerContainsTest extends TestCase{
 
        static boolean FAIL = true;
 
        public void testContainsPointInLayer() {
            MyLayer layer = new MyLayer();
            layer.setBounds(new Rectangle(0,0, 1000, 1000));
        
            MyLayer layer1 = new MyLayer();
            layer1.setBounds(new Rectangle(20, 20, 50, 50));
            layer.add(layer1);
        
            Figure figureOnLayer1 = new Figure();
            layer1.add(figureOnLayer1);
            figureOnLayer1.setBounds(new Rectangle(10,10, 30, 30));  // So effectively on 30,30
        
            MyLayer layer2 = new MyLayer();
            layer2.setBounds(new Rectangle(50, 50, 50, 50));
            layer.add(layer2);
        
            Figure figureOnLayer2 = new Figure();
            figureOnLayer2.setBounds(new Rectangle(10,10, 30, 30));  // So effectively on 60, 60
            layer2.add(figureOnLayer2);
 
            assertEquals(true, layer.containsPoint(60, 60));  // Is location of figureOnLayer2
        }
 
        public class MyLayer extends Layer {
 
            @Override
            protected boolean useLocalCoordinates() {
                return true;
            }
        
            /* (non-Javadoc)
             * @see org.eclipse.draw2d.Layer#containsPoint(int, int)
             */
         
            @Override
            public boolean containsPoint(int x, int y) {
                if (FAIL)
                    return super.containsPoint(x, y);
                else {
                    return workingContainsPoint(x, y);
                }
            }
        
            private boolean workingContainsPoint(int x, int y) {
                if (isOpaque())
                    return super.containsPoint(x, y);
                Point pt = Point.SINGLETON;
                pt.setLocation(x, y);
                translateFromParent(pt);
                x = pt.x;
                y = pt.y;
                for (int i = 0; i < getChildren().size(); i++) {
                    IFigure child = (IFigure)getChildren().get(i);
                    if (child.containsPoint(x, y))
                        return true;
                }
                return false;
            }
			
        }
    }

When field FAIL is set to true, it will use the existing code and fail.
When field FAIL is set to false, it will use the modified code and succeed.