Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 321452 - org.eclipse.draw2d.AbstractPointListShape.addPoint() call to Figure.erase() causing infinite loop in datatools subclasses
Summary: org.eclipse.draw2d.AbstractPointListShape.addPoint() call to Figure.erase() c...
Status: RESOLVED FIXED
Alias: None
Product: GEF
Classification: Tools
Component: GEF-Legacy Draw2d (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.6.1 (Helios SR1)   Edit
Assignee: Anthony Hunter CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-01 05:31 EDT by Rekha CLA
Modified: 2010-09-08 11:06 EDT (History)
5 users (show)

See Also:

Attachments
Patch to fix the issue. (914 bytes, patch)
2010-09-07 15:18 EDT, Anthony Hunter CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Rekha CLA 2010-08-01 05:31:36 EDT 
Build Identifier: 3.6 - I20100507

We are trying to have API compatibility between our Eclipse 3.4 code and Eclipse 3.6 code. 

org.eclipse.draw2d.AbstractPointListShape.addPoint() added call to  Figure.erase()  -->  creating an infinite loop in all our subclasses (provide anchors decoration) -> ***Decoration.getBounds()
In the method DataToolsDecoration.getPoints() we call the inherited method addPoint() which used to be implemented in Polyline.java, but now is "outsourced" to the newly introduced AbstractPointListShape (from which Polyline 
now inherits). The new method AbstractPointListShape.addPoint() now calls Figure.erase() which was not the case in Polyline.addPoint()


Reproducible: Always
Comment 1 Anthony Hunter CLA 2010-09-02 15:16:14 EDT 
In this I think we could consider removing erase() from org.eclipse.draw2d.AbstractPointListShape.addPoint(). But this code has been in GEF now since 2008 and we have not had any other complaints.
Comment 2 Alex Boyko CLA 2010-09-02 15:40:10 EDT 
Any chance you could just adopt this behavior change? 
Modifications of a list of points are reflected on the diagram right away (i.e. painting included)... there is logic in it... if shape's bounds change (#setBounds(Rectangle) call) the same happens for a rectangular shape. There is a chance other clients depend on this behavior... as Anthony mentioned, this change has been around for 2 years already.
Comment 3 Randy Hudson CLA 2010-09-02 17:07:03 EDT 
How can adding a point ever cause the pointlist's bounds to become smaller?  The call to erase() is bogus and was never needed.  Why not just remove it?
Comment 4 Alex Boyko CLA 2010-09-02 17:12:13 EDT 
The bounds can't get smaller, they can get larger. Yes erase() is bogus, because there is a repaint().
Comment 5 Anthony Hunter CLA 2010-09-03 15:11:14 EDT 
I will remove the bogus erase() from org.eclipse.draw2d.AbstractPointListShape.addPoint() in 3.6.1. I will attach a patch for everyone to review.
Comment 6 Anthony Hunter CLA 2010-09-07 15:18:06 EDT 
Created attachment 178354 [details]
Patch to fix the issue.

Can you review the patch?
Comment 7 Anthony Hunter CLA 2010-09-08 11:06:11 EDT 
(In reply to comment #6)
> Can you review the patch?

Based on Randy's comment 3 and Alex's comment 4, we are good to remove erase().

I want to commit NOW so I can build RC3.

Committed to HEAD and R3_6_maintenance for Helios SR1.