Community
Participate
Working Groups
GTK: boolean runDeferredLayouts () { if (layoutDeferredCount != 0) { Composite[] temp = layoutDeferred; int count = layoutDeferredCount; layoutDeferred = null; layoutDeferredCount = 0; for (int i = 0; i < count; i++) { Composite comp = temp[i]; if (!comp.isDisposed()) comp.setLayoutDeferred (false); } update (); return true; } return false; } Win32: boolean runDeferredLayouts () { if (layoutDeferredCount != 0) { Composite[] temp = layoutDeferred; int count = layoutDeferredCount; layoutDeferred = null; layoutDeferredCount = 0; for (int i = 0; i < count; i++) { Composite comp = temp[i]; if (!comp.isDisposed()) comp.setLayoutDeferred (false); } return true; } return false; } As Paul discovered the update call is missing in win32.
New Gerrit change created: https://git.eclipse.org/r/163837
I don't see an update() call on MAC as well: boolean runDeferredLayouts () { if (layoutDeferredCount != 0) { Composite[] temp = layoutDeferred; int count = layoutDeferredCount; layoutDeferred = null; layoutDeferredCount = 0; for (int i = 0; i < count; i++) { Composite comp = temp[i]; if (!comp.isDisposed()) comp.setLayoutDeferred (false); } return true; } return false; } Quick queries: 1. Do we know a breaking use-case which works on GTK and not on Win32(or may be MAC as well) ? 2. To be consistent we should update MAC code as well.
(In reply to Niraj Modi from comment #2) > 1. Do we know a breaking use-case which works on GTK and not on Win32(or may > be MAC as well) ? For example, we have extra code here for windows. https://git.eclipse.org/r/#/c/163834/3/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/SashLayout.java A requestLayout without update() does not update the UI under windows in this case.
(In reply to Lars Vogel from comment #3) > (In reply to Niraj Modi from comment #2) > > > 1. Do we know a breaking use-case which works on GTK and not on Win32(or may > > be MAC as well) ? > > For example, we have extra code here for windows. > > https://git.eclipse.org/r/#/c/163834/3/bundles/org.eclipse.e4.ui.workbench. > renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/SashLayout.java > > A requestLayout without update() does not update the UI under windows in > this case. Niraj, could you check this example?
(In reply to Lars Vogel from comment #3) > (In reply to Niraj Modi from comment #2) > > > 1. Do we know a breaking use-case which works on GTK and not on Win32(or may > > be MAC as well) ? Ok, for MAC I see bug 564073 > For example, we have extra code here for windows. > > https://git.eclipse.org/r/#/c/163834/3/bundles/org.eclipse.e4.ui.workbench. > renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/SashLayout.java > > A requestLayout without update() does not update the UI under windows in > this case. Searched for possible calls to display.runDeferredLayouts () and update() method combination in SWT. SWT's Tracker.open() method looks interesting, where I can see some event processing between display.runDeferredLayouts () @line 586 and update() line 608. So, clubbing display.update() with display.runDeferredLayouts() may have some unknown side-effects. Also with this change there exists a possibility of SWT client code calling update() twice(specifically for SWT client who are used to calling update() separately).
(In reply to Niraj Modi from comment #5) > Searched for possible calls to display.runDeferredLayouts () and update() > method combination in SWT. > SWT's Tracker.open() method looks interesting, where I can see some event > processing between display.runDeferredLayouts () @line 586 and update() line > 608. > > So, clubbing display.update() with display.runDeferredLayouts() may have > some unknown side-effects. I'm sorry but I do not understand. Currently we have to call it in platform directly, so any side-effect will be already there. > Also with this change there exists a possibility of SWT client code calling > update() twice(specifically for SWT client who are used to calling update() > separately). In this case the clients might be working around the issue that we do currently not call update() like we have to do in platform. So fixing this in SWT would allow them (including platform UI) to remove these workaround. And I think calling update() twice does not do harm (except the double call).
(In reply to Lars Vogel from comment #6) > (In reply to Niraj Modi from comment #5) > > Searched for possible calls to display.runDeferredLayouts () and update() > > method combination in SWT. > > SWT's Tracker.open() method looks interesting, where I can see some event > > processing between display.runDeferredLayouts () @line 586 and update() line > > 608. > > > > So, clubbing display.update() with display.runDeferredLayouts() may have > > some unknown side-effects. > > I'm sorry but I do not understand. Currently we have to call it in platform > directly, so any side-effect will be already there. To summarize on above: Unlike platform example(comment 3) in SWT's Windows Tracker.open() method: - display.update() method call is very much conditional - Also there's some event processing as well in between display.runDeferredLayouts () and display.update() method calls. We have contrasting examples here, which suggests not mandate to call update (); in Display#runDeferredLayouts on SWT Windows.
Resetting target, please re-target as required.
Mass move out to 4.18
Looks like this change is not necessary.
