Bug 510232 - [Forms] Switch from synchronous layout() calls to requestLayout() in forms
Summary: [Forms] Switch from synchronous layout() calls to requestLayout() in forms
Status: REOPENED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.7   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Ralf Petter CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks: 276660 533089
  Show dependency tree
 
Reported: 2017-01-11 02:54 EST by Lars Vogel CLA
Modified: 2018-04-04 11:49 EDT (History)
6 users (show)

See Also:
daniel_megert: pmc_approved-

Attachments
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 Lars Vogel CLA 2017-01-11 02:54:23 EST 
We should evaluate if layout() calls in forms can be replaced by requestLayout() calls.
Comment 1 Lars Vogel CLA 2017-01-11 03:00:08 EST 
Ralf, something for you?
Comment 2 Ralf Petter CLA 2017-01-11 04:22:24 EST 
Yes i will investigate this and provide a patch. Is it possible to change the "Assigned To" field on the bugs i am working? This will it make more easier to track the bugs i am working on.
Comment 3 Lars Vogel CLA 2017-01-11 04:43:31 EST 
(In reply to Ralf Petter from comment #2)
> Is it possible to change the "Assigned To" field on the bugs i am working? 

I requested bug triage rights for you from webmaster.
Comment 4 Eclipse Genie CLA 2017-04-16 04:50:51 EDT 
New Gerrit change created: https://git.eclipse.org/r/95105
Comment 5 Stefan Xenos CLA 2017-05-21 09:48:40 EDT 
I suspect this will fix bug 276660.
Comment 6 Mickael Istria CLA 2017-09-12 03:55:39 EDT 
This is an important behavior change. We need PMC advice before decidign how versions should evolve with such change.
Comment 7 Dani Megert CLA 2017-10-03 10:48:42 EDT 
Composite#layout(boolean) already mentions that clients better call Control#requestLayout(). Changing the behavior/implementation of #layout itself is a no go.
Comment 8 Stefan Xenos CLA 2017-10-17 02:31:59 EDT 
Calls to super.layout() from within the implementation of layout() (or presumably from layoutIfNecessary) should stay as-is since they're part of the implementation of layout(). Switching them to requestLayout() would cause problems (and possibly an infinite layout loop).

Calls to layout() from within non-layout-related methods (like setText()) should be changed to requestLayout().

I assume the former issue is why Dani rejected the patch...?
Comment 9 Mickael Istria CLA 2017-10-17 02:50:26 EDT 
(In reply to Stefan Xenos from comment #8)
> Calls to super.layout() from within the implementation of layout() (or
> presumably from layoutIfNecessary) should stay as-is since they're part of
> the implementation of layout(). Switching them to requestLayout() would
> cause problems (and possibly an infinite layout loop).
> Calls to layout() from within non-layout-related methods (like setText())
> should be changed to requestLayout().
> I assume the former issue is why Dani rejected the patch...?

Yes, see last comment on Gerrit.
Comment 10 Lars Vogel CLA 2017-11-30 19:07:46 EST 
Adding Karsten, as he is also interested in performance work.
Comment 11 Dani Megert CLA 2018-01-31 08:48:18 EST 
Please set a target milestone again when you really plan to work on it.
Comment 12 Mickael Istria CLA 2018-03-14 12:06:30 EDT 
Let's target it for 4.8.M7 as we're very close to a good patch to merge.
Comment 14 Dani Megert CLA 2018-04-04 11:49:36 EDT 
This caused a regressions, see bug 533089.

Reverted the change with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=6bf8aa42b55821e82e17a5aeda9294dc6d3d208e

Such changes are always risky.