Community
Participate
Working Groups
The initial implementation of GalleryTreeViewer (bug 212071) does not attempt to handle a virtual style Gallery. To simplify debugging of the fully-eager case, I removed or disabled a lot of code related to virtual style Trees. This logic should be restored to enable the full functionality of Gallery via the JFace Viewer APIs.
I've done a night's work on this, using TreeViewer as a base and blindly copying all of the code related to virtual table support. Sadly, this requires about 700 lines of completely duplicated code, which more than doubles the length of GalleryTreeViewer from 550 to 1243 LOC. After going through this exercise, I just realized that I copied the code from 3.3 instead of from 3.4, so I have to do it all over again. Tom, do you have any ideas to help us reuse more of this TreeViewer logic for virtual trees? Why did you leave it in TreeViewer instead of pushing it up to AbstractTreeViewer?
Created attachment 108725 [details] Virtual content support for GalleryTreeViewer This is my initial version, based on the TreeViewer code from Eclipse 3.3.2. This needs to be updated to include fixes from 3.4.0, but it is basically usable like this.
Peter, thanks for your hard work on the Gallery viewer. I'll commit the patch ASAP.
I've thought about this a bit more, and there's a subtle difference between the way TreeViewer and TableViewer handle SWT.VIRTUAL widgets that has a big impact. For the Gallery, the most resource-intensive part of the process is usually in the LabelProvider, not the ContentProvider. In theory, it would be possible to use an eager ITreeContentProvider but still fetch and render the Images in virtual style. TableViewer supports this use case by caching the array of contents and satisfying SWT.SetData requests from the cache. TreeViewer and GalleryTreeViewer should be able to do something similar. That way, client code that uses an eager content provider can still gain the performance advantages of loading the images on demand. Tom, should I request this from JFace, or keep it here? In the short term, I've written a decorator object that converts an eager ITreeContentProvider into an ILazyTreeContentProvider. Something similar should be possible for ITreePathContentProvider, but I haven't done this yet. I will post the patch shortly. Because there is no AbstractTreeViewer.replace(), this new class will only work with GalleryTreeViewer for now. For the code duplication problem, I suggest a new class AbstractLazyTreeViewer that would fit between AbstractTreeViewer and TreeViewer. From a quick inspection of the code, the base type Item can be used instead of TreeItem or GalleryItem in all cases except for three method calls: clear() clearAll(boolean) clear(int,boolean) All other cases where the specific subtype is used can be converted to use one of the abstract methods on AbstractTreeViewer, such as getItemCount(Item).
Created attachment 108796 [details] Decorator for eager -> lazy tree content provider
Created attachment 108797 [details] Same as the last one, as a patch file
Created attachment 108958 [details] Bug fixes for last patch Apparently the viewer's IElementComparer is null by default, so I have to check for that case. Also, I fixed a silly but important typo.
Because in time of 3.3 our only customer (Nebula-Grid) had no virtual support and because we had no test case we stayed away from it. Since this situation has changed since then (Gallery adopted us => it really strikes me that you guys adopted our API and Grid also "supports" virtual tables/trees) I think we could start work on this in 3.5. Could you file a bug against JFace where you ask that we move the logic up and CC Boris and me? I can't promise anything because of me work load but I willing to help you guys to maintain as few lines of code as possible.
hello guys, where i am?
This bug does not have a target milestone assigned and is automatically closed as part of the 2.3.0 release cleanup. It could be that this bug is accidentally closed for which we apologize. If this bug is still relevant, please re-open and set a target milestone.
