Bug 291383 - CCombo creates too many shells
Summary: CCombo creates too many shells
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 3.6   Edit
Hardware: PC All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Platform-SWT-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-05 12:27 EDT by Kevin Barnes CLA
Modified: 2019-09-06 15:32 EDT (History)
3 users (show)

See Also:


Attachments
CCombo share shells (3.98 KB, patch)
2009-10-05 12:29 EDT, Kevin Barnes CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin Barnes CLA 2009-10-05 12:27:37 EDT
Every time a CCombo is created, it creates a Shell. The shell is only shown for a short time, and only one can be visible at a time. A better approach would be to have CCombo's share a Shell, or create/dispose the Shell as needed.
Comment 1 Kevin Barnes CLA 2009-10-05 12:29:04 EDT
Created attachment 148799 [details]
CCombo share shells

This patch creates one Shell for all CCombos in a Shell to reuse.
Comment 2 Elias Volanakis CLA 2010-07-09 12:42:45 EDT
Sad to see this lingering for almost a year. Now the patch is stale. Would be great to see this in 3.7.
Comment 3 Felipe Heidrich CLA 2010-07-15 12:31:49 EDT
Kevin, why wasn't your patch ever released ?


Elias, is this behaviour causing problems for you ?
Comment 4 Kevin Barnes CLA 2010-07-15 16:54:23 EDT
Felipe, Seemed risky and I wasn't sure that it solved any problem. Creating a new window was/is expensive, but the memory is reclaimed when the window is destroyed. Other performance improvements were found and no one argued that this one was needed.
Comment 5 Elias Volanakis CLA 2010-07-15 17:22:33 EDT
From inspecting the code I see that each CCombo creates a non-visible Shell for the popup immediately (in the constructor). This seems a waste, since out of many CCombos, only a few will ever be dropped down (i.e. need the Shell). Those Shells are not reclaimed until the CCombo is disposed or reparented.

I think it would be best for me to create a demo app with a couple hundred CCombos and see what difference it makes with a profiler. (if it's bad, I'm willing to contribute a patch since we use CCombo a lot)

Do you think it's a good next step?
Comment 6 Kevin Barnes CLA 2010-07-16 10:04:31 EDT
I won't comment on what the next step should be since I'm no longer involved in SWT, but if I were still involved, I would be more interested in improving Combo to meet the needs of CCombo users (because CCombo sucks on mac).
Comment 7 Felipe Heidrich CLA 2010-07-20 09:31:14 EDT
Thanks Kevin, I agree with you.

If the patch is not fixing anything then it should not go in.
I'm closing this as wont fix.

Elias,

If you find the time, try to benchmark a your app with and without the patch. I believe you won't see any difference. Creating a demo app with thousands of ccombo is probably not worth (too artificial).

Kevin, why CCombo sucks on Cocoa ? Where are the real problems ?
Comment 8 Scott Kovatch CLA 2010-07-20 12:28:01 EDT
(In reply to comment #7)

> Kevin, why CCombo sucks on Cocoa ? Where are the real problems ?

I can answer that. The overall problem is that a CCombo looks nothing like the native Combo, so it's more obvious that it's a Java-based control.

Compare CCombo from the CustomControlExample vs. Combo in ControlExample.

-- CCombo is smaller than Combo
-- CCombo's drop down arrow doesn't have blue background.
-- CCombo has no focus ring around the control when focused
-- CCombo popup window doesn't have a shadow
-- CCombo's popup window becomes active/focused, so the menu bar of the parent window clears
-- clicking outside of a CCombo doesn't hide the drop down menu.
-- no visible difference in a READ_ONLY vs. editable CCombo. It looks like the text should be edible but clicking on it doesn't do anything. The text shouldn't be selectable, nor should it have a selection.

Maybe we should start a new bug. I see that CCombo isn't designed to look or behave like the native Combo box, but as written it seems to be a replacement for the Win32 Combo.
Comment 9 Eclipse Webmaster CLA 2019-09-06 15:32:40 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.