Bug 96326 - [Preferences] Preferences tree flickers heavily while typing into the filter field
Summary: [Preferences] Preferences tree flickers heavily while typing into the filter ...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: 3.1 RC1   Edit
Assignee: Tod Creasey CLA
QA Contact:
URL:
Whiteboard:
Keywords: polish
Depends on:
Blocks:
 
Reported: 2005-05-23 13:19 EDT by Erich Gamma CLA
Modified: 2005-05-27 10:11 EDT (History)
1 user (show)

See Also:


Attachments
patch to FilteredTree (3.51 KB, patch)
2005-05-23 13:23 EDT, Erich Gamma CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Erich Gamma CLA 2005-05-23 13:19:38 EDT
When you type into the filter dialog you can observe how the tree refreshes
after each keystroke. This results in a lot of flickering and might also result
in bad performance with large preference trees.
Comment 1 Erich Gamma CLA 2005-05-23 13:20:41 EDT
I've attached a patch which refreshes the tree only after a small timeout. Using
this patch I get a much smoother look.
Comment 2 Erich Gamma CLA 2005-05-23 13:23:31 EDT
Created attachment 21591 [details]
patch to FilteredTree

The patch uses a UIJob to delay and filter out unneeded refreshes.
Comment 3 Tod Creasey CLA 2005-05-24 08:02:35 EDT
Erich are you planning to raise this as a polish issue? If so I'll get the
approval process started.
Comment 4 Erich Gamma CLA 2005-05-24 08:33:52 EDT
Tod, yes this one is on my polish wish list, but I haven't brought it up with
the polish team yet. 
Comment 5 Nick Edgar CLA 2005-05-24 10:51:01 EDT
Looks like this will create a separate job for each key press, which may be
expensive.
Comment 6 Erich Gamma CLA 2005-05-24 11:26:29 EDT
Performance concerns aside, I really like the visual behaviour you get from this
patch. 

You are correct that the current implementation creates a job per key stroke.
This can be optimized if really needed. I haven't found a performance problem in
my testing (and it is the same logic that is used in the Open Type dialog which
has been tuned for performance). I like the simplicity of the current
implementation and compared to the previous implementation where the tree was
refreshed after each keystroke it might even be more efficient. 

Anyways, since I really would like to see this happen as a polish item. I'm open
to revisit the patch, just let me know.
Comment 7 Erich Gamma CLA 2005-05-24 12:03:47 EDT
Actually, when a new job is created then an existing one should be cancelled so
it never gets scheduled. 
Comment 8 Nick Edgar CLA 2005-05-24 13:17:49 EDT
I think that would work fine.

+1 for this polish item.
Comment 9 Tod Creasey CLA 2005-05-24 14:10:57 EDT
I have modified how this is done using some of the features jobs give us

First of all as we don't check state until the job is running we could
continually reschedule the same job so I made the UIJob a field and just called
schedule again. This also meant that we didn't have to bother with keeping a
timestamp to check as rescheduling an existing job is a no - op.

Modified patch released for build >20050524
Comment 10 Nick Edgar CLA 2005-05-24 14:26:37 EDT
That will cause more, earlier refreshes than canceling the pending job first.
Comment 11 Tod Creasey CLA 2005-05-24 14:30:44 EDT
Understood but this method guarantees a refresh every 200 ms - do we want to
effectively disable updating during fast typing? If so then we can just cancel
the job and reschedule it.
Comment 12 Nick Edgar CLA 2005-05-24 14:45:52 EDT
I'm actually fine with either, with a slight preference to updating every 200ms
if there's been a change, rather than waiting until the user stops.  Either way
it's an improvement on updating on every keystroke.  Erich, any preference?
Comment 13 Erich Gamma CLA 2005-05-24 16:56:31 EDT
The implementation that is currently implemented in HEAD works well for me and
is better than my original patch
Comment 14 Tod Creasey CLA 2005-05-27 10:11:01 EDT
Verified in 20050526