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

Bug 287564

Summary: Image in ImageFigure rendered on wrong position / GridData modifies PreferredSize
Product: [Tools] GEF Reporter: Heiko Böttger <heiko.boettger>
Component: GEF-Legacy Draw2dAssignee: Alex Boyko <aboyko>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: aboyko, cocoakevin, francisu, mvo
Version: 3.5   
Target Milestone: 3.6.1 (Helios SR1)   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Patch to fix the issue. none

Description Heiko Böttger CLA 2009-08-25 10:33:50 EDT
Build ID: M20090211-1700

Steps To Reproduce:
1. Create a canvas and an ImageFigure with an image smaller than the canvas
2. add a rootfigure with GridLayout to the ViewPort
3. add the ImageFigure to the rootfigure using GridData with heightHint == SWT.DEFAULT (-1) 
4. layout the rootfigure (should be done automatically)

The image is rendered with aligned at the top but should be in the center. 


More information:
The responsible code is found in GridData#computeSize and ImageFigure#getPreferredSize/ImageFigure#paintFigure.

I think there are two bugs. 

a) GridData modifies the preferred size:

<code>

Dimension size = figure.getPreferredSize(widthHint, heightHint);
if (widthHint != -1) size.width = widthHint;
if (heightHint != -1) size.height = heightHint;

</code>

GridData should work on a copy. 

b) I think the ImageFigure shouldn't disable the possibility to overwrite the preferred size by the user. Or easier said, the image size should be stored independent of the preferred size.

A workaround for this is: Creating a subclass witch returns a copy on every call to ImageFigure#getPreferredSize(int,int) when using GridData or use a Label.
Comment 1 Francis Upton IV CLA 2009-08-25 10:36:46 EDT
GridLayout is SWT
Comment 2 Kevin Barnes CLA 2009-08-25 10:56:59 EDT
GridData calls computeSize on the control that it's trying to layout and sizes the control according to its computed size. 
ImageFigure can (should?) over ride computeSize to return its preferred size.

Comment 3 Heiko Böttger CLA 2009-08-25 14:30:03 EDT
Hello, it seems like this bug was redirected to the wrong group. It's not swt its draw2d. And there is a GridLayout (org.eclipse.draw2d.GridLayout) too. 

When I filled in the bug report I wondered where to define that this bug is related to gef/draw2d. 
Comment 4 Kevin Barnes CLA 2009-08-25 14:35:05 EDT
reopening to reassign
Comment 5 Milos Volauf CLA 2010-06-28 05:53:24 EDT
I consider it a major bug that GridData.computeSize method modifies a dimension returned from the figure.

JavaDoc of IFigure#getPreferredSize(int, int) clearly says that the returned dimension must not be modified.
Comment 6 Alex Boyko CLA 2010-06-28 10:08:18 EDT
Yes, we need to call getCopy() on the preferred size dimension. This can be delivered for GEF 3.6.1
Comment 7 Anthony Hunter CLA 2010-08-31 10:11:47 EDT
Created attachment 177837 [details]
Patch to fix the issue.

Hi Alex, I think this is the fix we are looking for here. Can you all confirm?
Comment 8 Alex Boyko CLA 2010-08-31 11:44:59 EDT
Hey Anthony, yes, this is definitely the fix. If you feel like delivering it, please do, because if I don't do this now I'll just forget about it again until someone brings it up again :-) Thanks!
Comment 9 Milos Volauf CLA 2010-08-31 11:51:21 EDT
(In reply to comment #7)
> Created an attachment (id=177837) [details]
> Patch to fix the issue.
> 
> Hi Alex, I think this is the fix we are looking for here. Can you all confirm?

Yes, fix looks OK.
Comment 10 Alex Boyko CLA 2010-08-31 13:37:30 EDT
Delivered the fix