Community
Participate
Working Groups
Please add a new requestLayout() method to Control, which does essentially the same thing as: widget.getShell().layout(new Control[] {widget}, SWT.DEFER); ...but without the performance problems mentioned by bug 103863, comment 32. We should be marking the dirty widgets so that SWT does not need to perform an inefficient exhaustive search through the widget hierarchy. Further, we should add javadoc for all other mechanisms which trigger layouts to direct callers to use this new method. Justification: The snippet, above, is currently the most efficient and reliable method of triggering layouts in SWT, but most developers are unaware of this. Doing a search through the Eclipse code base, I see that most callers are still trying to guess the root widget affected by their change and invoke Composite.layout(), which is the worst-possible implementation. By making the most-efficient implementation also the easiest to use and documenting it better, it should make it easier to clean this up.
New Gerrit change created: https://git.eclipse.org/r/60410
Arun, would you be so kind as to take a look at the attached patch and let me know what you think?
New Gerrit change created: https://git.eclipse.org/r/60614
Seems like gerrit doesn't like to have the main SWT package changed in the same commit as the new examples, so I broke this into two changes.
SWT team can this be reviewed? We expect more efficient rendering in Platform UI once his method can be used.
Arun, please have a look at the patch.
The current patch adds the new method and adds javadoc to the Composite methods which should be discouraged for performance reasons. I haven't deprecated them since there is no need to move clients off of them if their code performs well enough already. The point of the new Javadoc is just to make clients aware of the best practices. If anyone thinks we should go farther and deprecate Composite.layout(...) and its variants, let me know. This patch does not address the performance problems mentioned by bug 103863, comment 32. According to my benchmark attached bug 103863, comment 23, the runtime cost of this problem is tiny even for very large widget hierarchies... so we can live with it for now. It's certainly worth addressing in a follow-up, but there's no need to hold up the new API in order to address it.
IMHO we should decide after adaption of this new method in the Eclipse top level project. If the performance gain is significant, I'm +1 for deprecation to guide the adaptors in doing the right thing.
(In reply to Stefan Xenos from comment #4) > Seems like gerrit doesn't like to have the main SWT package changed in the > same commit as the new examples, so I broke this into two changes. Stefan, did you get a chance to evaluate my comment on gerrit? I would like this pushed in M4 so that the new API is available for adoption in M4...
> Stefan, did you get a chance to evaluate my comment on gerrit? I replied in gerrit, but the quick version is that invalidating the parent rather than the shell would create layout bugs.
Gerrit change https://git.eclipse.org/r/60410 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=80b7ef93a510bf6f046e456e854dbcbe84fa03e6
I added a small entry for this to https://www.eclipse.org/eclipse/news/4.6/M4/#Platform-Dev please review and correct or update.
Gerrit change https://git.eclipse.org/r/60614 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=debeeaf0a81df9fb9de7fe496e9662df748e3589
http://download.eclipse.org/eclipse/downloads/drops4/I20151207-2000/testresults/html/org.eclipse.swt.tests_macosx.cocoa.x86_64_8.0.html Re-opening, there is a JUnit test failure 'test_requestLayoutL' in today's I-build, share the stack-trace: Widget has the wrong parent java.lang.IllegalArgumentException: Widget has the wrong parent at org.eclipse.swt.SWT.error(SWT.java:4502) at org.eclipse.swt.SWT.error(SWT.java:4436) at org.eclipse.swt.SWT.error(SWT.java:4407) at org.eclipse.swt.widgets.Widget.error(Widget.java:799) at org.eclipse.swt.widgets.Composite.layout(Composite.java:864) at org.eclipse.swt.widgets.Control.requestLayout(Control.java:2828) at org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_Control.test_requestLayoutL(Test_org_eclipse_swt_widgets_Control.java:373) at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:743) at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:351) at org.eclipse.test.CoreTestApplication.runTests(CoreTestApplication.java:36) at org.eclipse.test.CoreTestApplication.run(CoreTestApplication.java:32) at org.eclipse.equinox.internal.app.EclipseAppContainer.callMethodWithException(EclipseAppContainer.java:587) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:198) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:388) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:243) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:670) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:609) at org.eclipse.equinox.launcher.Main.run(Main.java:1516) at org.eclipse.equinox.launcher.Main.main(Main.java:1489) at org.eclipse.core.launcher.Main.main(Main.java:34)
Here's the gtk version: Widget has the wrong parent java.lang.IllegalArgumentException: Widget has the wrong parent at org.eclipse.swt.SWT.error(SWT.java:4502) at org.eclipse.swt.SWT.error(SWT.java:4436) at org.eclipse.swt.SWT.error(SWT.java:4407) at org.eclipse.swt.widgets.Widget.error(Widget.java:482) at org.eclipse.swt.widgets.Composite.layout(Composite.java:1184) at org.eclipse.swt.widgets.Control.requestLayout(Control.java:3773) at org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_Control.test_requestLayoutL(Test_org_eclipse_swt_widgets_Control.java:373) at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:743) at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:351) at org.eclipse.test.CoreTestApplication.runTests(CoreTestApplication.java:36) at org.eclipse.test.CoreTestApplication.run(CoreTestApplication.java:32) at org.eclipse.equinox.internal.app.EclipseAppContainer.callMethodWithException(EclipseAppContainer.java:587) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:198) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:388) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:243) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:670) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:609) at org.eclipse.equinox.launcher.Main.run(Main.java:1516) at org.eclipse.equinox.launcher.Main.main(Main.java:1489) at org.eclipse.core.launcher.Main.main(Main.java:34) The exception is testing whether the receiver of #layout(Control[], int) is an ancestor of its arguments. Since we invoked it like this: getShell ().layout (new Control[] {this}, SWT.DEFER); ...it means that there existed a control which was not a descendant of its getShell(). My best guess is that the control *was* the shell.
Confirmed that requestLayout is failing in the case where it is invoked on a Shell. I'll attach a fix.
New Gerrit change created: https://git.eclipse.org/r/62265
Arun, I've attached a fix for the test failures. Could I trouble you for a code inspection?
Gerrit change https://git.eclipse.org/r/62265 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=5d56ae6dbe33b23b3e8d1ac5cb166329b2607c28
(In reply to Stefan Xenos from comment #18) > Arun, I've attached a fix for the test failures. Could I trouble you for a > code inspection? Thanks for the fix Stefan!
