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

Bug 371084

Summary: org.eclipse.draw2d.GridLayout should extend org.eclipse.draw2d.AbstractHintLayout instead of org.eclipse.draw2d.AbstractLayout
Product: [Tools] GEF Reporter: François POYER <francois.poyer>
Component: Website / WikiAssignee: Alexander Nyßen <nyssen>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: nyssen
Version: 3.7.1   
Target Milestone: 3.8.0 (Juno)   
Hardware: PC   
OS: Windows XP   
Whiteboard:

Description François POYER CLA 2012-02-09 09:19:18 EST
Currently GridLayout extends AbstractLayout, while it DOES use the given hHint and wHint parameters in its #calculatePreferredSize method, result of which is cached by AbstractLayout in the #getPreferredSize method (leading to erroneous value returned by later calls).

ex: <code>
Figure container = new Figure();
GridLayout gridLayout = new GridLayout(1, true);
gridLayout.marginHeight = 0;
gridLayout.marginWidth = 0;
gridLayout.horizontalSpacing = 0;
gridLayout.verticalSpacing = 0;
container.setLayoutManager(gridLayout);
Figure child = new Figure();
GridData gridData = new GridData(SWT.FILL, SWT.CENTER, true, true);
container.add(child, gridData);

Dimension d1 = container.getPreferredSize(600, -1);
// d1.w = 600 <= OK
Dimension d2 = container.getPreferredSize(-1, -1);
// d2.w = 600 <= KO!

Figure container2 = new Figure();
GridLayout gridLayout2 = new GridLayout(1, true);
gridLayout2.marginHeight = 0;
gridLayout2.marginWidth = 0;
gridLayout2.horizontalSpacing = 0;
gridLayout2.verticalSpacing = 0;
container2.setLayoutManager(gridLayout2);
Figure child2 = new Figure();
GridData gridData2 = new GridData(SWT.FILL, SWT.CENTER, true, true);
container2.add(child2, gridData2);
Dimension d3 = container2.getPreferredSize(-1, -1);
// d3.w = 0 <= OK!
Dimension d4 = container2.getPreferredSize(600, -1);
// d4.w = 0 <= KO!
</code>

Making GridLayout extend AbstractHintLayout would fix this bug.
Comment 1 Alexander Nyßen CLA 2012-02-25 16:42:21 EST
Made GridLayout extend AbstractHintLayout instead of
AbstractLayout so that preferred size cache is correctly flushed in case
size hints change.

Pushed changes to master (3.8M6).
Comment 2 Andreas Scharf CLA 2012-05-30 12:42:58 EDT
Fixing this bug introduced https://bugs.eclipse.org/bugs/show_bug.cgi?id=381075. As I stated in #381075 I'm not sure if extending from AbstractHintLayout is really the problem but it introduced (or revealed..) another bug.
Comment 3 Alexander Nyßen CLA 2012-05-31 07:30:38 EDT
(In reply to comment #2)
> Fixing this bug introduced
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=381075. As I stated in #381075
> I'm not sure if extending from AbstractHintLayout is really the problem but it
> introduced (or revealed..) another bug.

I think it will probably be "revealed". I will take a look into #381075 and comment there.