Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 321452

Summary:

org.eclipse.draw2d.AbstractPointListShape.addPoint() call to Figure.erase() causing infinite loop in datatools subclasses

Product:

[Tools] GEF

Reporter:

Rekha <nrekha>

Component:

GEF-Legacy Draw2d

Assignee:

Anthony Hunter <ahunter.eclipse>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

aboyko, ahunter.eclipse, hudsonr, lj, lozanoj

Version:

unspecified

Target Milestone:

3.6.1 (Helios SR1)

Hardware:

OS:

Windows XP

Whiteboard:

Attachments:

Description	Flags
Patch to fix the issue.	none

Description Rekha

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

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

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

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

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

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

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

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.