Community
Participate
Working Groups
Created attachment 252671 [details] Snippet to reproduce the issue In GTK2 (and windows) if you call getSize on a control that has been set to not be visible the getSize method still returns non-zero values but with GTK3 it returns (0,0) if the control is not visible. This inconsistent behavior is causing a test to fail under PlatformUI, namely UIDialogsAuto#testShowView. Find attached a snippet to reproduce the issue.
From https://lwn.net/Articles/544395/ : Invisible widgets now return a size of 0x0.
(In reply to Alexander Kurtakov from comment #1) > From https://lwn.net/Articles/544395/ : > Invisible widgets now return a size of 0x0. Calling getSize() on a control will return the width and height of topHandle for that control, which is the swtFixed container/object for that particular control. In Gtk3, widgets that are not visible will return a size of 0x0. For widgets (buttons, labels, etc.) this will be 1x1 (with x and y coordinates of -1). In the case of swtFixed, it appears to be 0x0. Gtk2 returns the size allocated to the widget regardless of whether it is visible or not.
(In reply to Eric Williams from comment #2) > In Gtk3, widgets that are not visible will return a size of 0x0. For widgets > (buttons, labels, etc.) this will be 1x1 (with x and y coordinates of -1). > In the case of swtFixed, it appears to be 0x0. I should clarify: native widgets (which are not visible) return a size of 1x1 and coordinates of -1. SWT widgets have swtFixed containers/objects, therefore calling getSize() on an SWT widget returns 0x0, since the call uses the GtkAllocation of the swtFixed container/object.
New Gerrit change created: https://git.eclipse.org/r/59050
New Gerrit change created: https://git.eclipse.org/r/61651
*** Bug 481851 has been marked as a duplicate of this bug. ***
Can this be reviewed for M4? It breaks our e4 tools, see Bug 481851.
Gerrit change https://git.eclipse.org/r/61651 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=595a640d4be4bdf5bb2cf6e41d4d21bca7dd1ffa
Hi Snjezana, Looking at the code, it seems that calling setVisible(true) and setVisible(false) will cause unnecessary SWT.Show and SWT.Hide events to be sent every time computeSize() is called. There could be other side-effects too of calling setVisible() in computeSize(). Can you please verify?
(In reply to Lakshmi Shanmugam from comment #9) > Hi Snjezana, > Looking at the code, it seems that calling setVisible(true) and > setVisible(false) will cause unnecessary SWT.Show and SWT.Hide events to be > sent every time computeSize() is called. There could be other side-effects > too of calling setVisible() in computeSize(). Can you please verify? SWT.Show and SWT.Hide events will be called for GTK3 (sometimes for GTK2) when a control is invisible.
I haven't looked into details but using gtk_widget_show/hide should be better option than doing the full SWT cycle setVisible.
(In reply to Alexander Kurtakov from comment #11) > I haven't looked into details but using gtk_widget_show/hide should be > better option than doing the full SWT cycle setVisible. +1. We should try to use gtk_widget_show/hide () instead of setVisible () to achieve the same effect.
(In reply to Snjezana Peco from comment #10) > (In reply to Lakshmi Shanmugam from comment #9) > > Hi Snjezana, > > Looking at the code, it seems that calling setVisible(true) and > > setVisible(false) will cause unnecessary SWT.Show and SWT.Hide events to be > > sent every time computeSize() is called. There could be other side-effects > > too of calling setVisible() in computeSize(). Can you please verify? > > SWT.Show and SWT.Hide events will be called for GTK3 (sometimes for GTK2) > when a control is invisible. Thanks for verifying, Sravan. I'm not sure, but there could be other side-effects too since setVisible() does other things. (In reply to Arun Thondapu from comment #12) > (In reply to Alexander Kurtakov from comment #11) > > I haven't looked into details but using gtk_widget_show/hide should be > > better option than doing the full SWT cycle setVisible. > > +1. We should try to use gtk_widget_show/hide () instead of setVisible () to > achieve the same effect. But, we can't use it for Custom Controls as they have common code for all platforms. IMHO it's better to revert the patch for M4 and find a fix that works for both native and custom controls for M5.
> Thanks for verifying, Sravan. Sorry, I meant to say - Thanks for verifying, Snjezana.
(In reply to Lakshmi Shanmugam from comment #13) > > +1. We should try to use gtk_widget_show/hide () instead of setVisible () to > > achieve the same effect. > > But, we can't use it for Custom Controls as they have common code for all > platforms. I don't think this change is even necessary for the custom controls, it should be enough to make the change in the native controls alone AFAICS, because the computeSize() methods in the custom controls anyway use the computeSize() methods from the native controls internally... > IMHO it's better to revert the patch for M4 and find a fix that works for > both native and custom controls for M5. It would have been good to have this bug fixed in M4 as it affects UI scenarios like the one mentioned in bug 481851, but I agree that we should find the optimal solution to avoid the side-effects of using setVisible() here. Reverted the patch via http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=166a902e3280d3067b5a24bba11e2aac7e51a264 and moved to M5.
New Gerrit change created: https://git.eclipse.org/r/62495
https://git.eclipse.org/r/62495 uses gtk_widget_show/gtk_widget_hide to fix the issue. The patch fixes the issue described in bug 481851 and https://issues.jboss.org/browse/JBIDE-21072. Calling gtk_widget_show/gtk_widget_hide for Shell locks Eclipse. As to the CCombo, CLabel, CoolBar and CTable class, I haven't changed them because there aren't any known issues related to these classes. If we face an issue related to them, we will be able to fix it using the setVisible method.
(In reply to Arun Thondapu from comment #15) > It would have been good to have this bug fixed in M4 as it affects UI > scenarios like the one mentioned in bug 481851, but I agree that we should > find the optimal solution to avoid the side-effects of using setVisible() > here. Bug 483848 is one example of an undesirable side-effect caused by toggling visibility using setVisible(). I'm not sure if the patch using gtk_widget_show/gtk_widget_hide would exhibit this or other similar problems, needs to be tested to confirm...
(In reply to Arun Thondapu from comment #18) > > Bug 483848 is one example of an undesirable side-effect caused by toggling > visibility using setVisible(). > > I'm not sure if the patch using gtk_widget_show/gtk_widget_hide would > exhibit this or other similar problems, needs to be tested to confirm... In my opinion, the issue described in bug 483848 isn't related either to this bug or to any patches I have created within it. I can reproduce it on Mars.1, Neon M3, the current SWT master with or without these patches using GTK >= 3.14. It is easier to reproduce it on a slower machine. Try to start several Eclipse instances. Bug 483848 isn't fixed, but it is a duplicate of bug 473369. I think Alexander is right. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=473369#c1
With the latest code also the ui test testShowView(org.eclipse.ui.tests.dialogs.UIDialogsAuto) fails. This test actually test the get_allocation using hidden buttons.
moving to M6 as review is not complete
Is it possible to have a look at the problem for M6. There are some issues with the attached gerrit Patch
Moving to M7 now as it is still not clear whether the patch is fixing the real problem or not...
the patch still does not fix the main issue so moving out of 4.6. since it only test failure I am reducing the severity
I haven't read into this bug too deeply, but I've worked on Visibility issues in: Bug 487757 - ScrolledComposite is empty if it is hidden and then made visible via setVisible(true|false) I added a check that shows a widget in setBounds() if it's suppose to be shown on SWT side but hasn't been shown on GTK side. It might be a bit relevant to this bug.
Moving to 4.8.
Created attachment 268852 [details] Some jUnits to demonstrate issue I've been investigating this business. I've made some observations: - The root of the problem seems to be gtk caching that was introduced in 3.8 - On 3.6, the attached jUnits pass. On 3.8, some fail. Things to note: - It doesn't seem that visibility alone impacts getBounds(), but rather: * setBounds() on an invisible widget seems to now only 'mark' the widget for resizing * actual resizing seems to be done in idle time * only once actual resizing is done in idle time by gtk, does gtk report correct widget sizes for get_allocation/getBounds(). Evidence: - getBounds() will return correct coordinates for invisible widget if setBounds() was called *before* setVisible(false). (see test1) - getbounds() will return correct coordinates for invisible widget if setBounds() was called after setVisible() AND if you open shell & call readAndDispatch() multiple times in between (presumably allowing size-cache to be rebuild in indle cycles). (see hackaround). Possible solutions: 1) Figure out how force gtk to compute sizes, such that we could force compute sizes before returning getBounds()/getAllocate. 2) On SWT side, cache sizes for widgets by setBounds() until gtk computes actual sizes. I.e, if gtk gives us 0,0 but we have a different cached size, then return cached sizes until gtk has updated it's sizes internally. I will investigate if 1) is technically possible by consulting with gtk devel team. If not, I'll investigate a solution for option 2.
New Gerrit change created: https://git.eclipse.org/r/99981
Gerrit change https://git.eclipse.org/r/99981 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=f8912f28f32f1bd171ad64e60219c48b30b9060a
New Gerrit change created: https://git.eclipse.org/r/99982
Gerrit change https://git.eclipse.org/r/99982 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=77ab5c37809c62b8a07a9b6cfede6907c0d6b5c0
Patch that fixes the issue was merged. Tomorrow's builds should contain the fix. I need to backport this patch if it proves to work well.
Bugfix verified. See: Bug 497705 – Unhandled event loop > StackOverflow in Perspective Awaiting Oxygen release & opening of backport branch for backport.
New Gerrit change created: https://git.eclipse.org/r/101659
Gerrit change https://git.eclipse.org/r/101659 was merged to [R4_7_maintenance]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=f8420da7dfe950beb767545b62c7bec3a3a3977b
Fix backported to 4.7.1
