This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Summary: | Rewrite TreeTableView | ||
---|---|---|---|
Product: | platform | Reporter: | Jiri Rechtacek <jrechtacek> |
Component: | Outline&TreeTable | Assignee: | Stanislav Aubrecht <saubrecht> |
Status: | RESOLVED FIXED | ||
Severity: | blocker | CC: | anebuzelsky, davidjon, dstrupl, hmichel, ivan, lebedkov, lhasik, matteodg, phrebejk, tboudreau, tor |
Priority: | P2 | Keywords: | PLAN, UI, UMBRELLA |
Version: | 3.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | TASK | Exception Reporter: | |
Bug Depends on: | 22998, 24892, 33186, 34181, 19266, 22822, 22996, 24893, 27696, 28561, 28962, 29031, 29534, 29612, 29717, 29855, 30539, 31016, 31501, 31621, 32567, 34169, 34170, 34171, 34172, 34173, 34175, 34177, 34178, 34182, 34184, 34185, 35836, 37802, 38434, 133929 | ||
Bug Blocks: | 43660, 136655, 136660, 136663, 136664, 36781, 37100, 38478, 39160, 39523, 40130, 41579, 41612, 44856, 76522, 108861, 136653, 136654, 136656, 136658, 136659, 136661, 136662 |
Description
Jiri Rechtacek
2003-04-28 16:10:25 UTC
UI specification (issue 31621) should be done first. TreeTableView and connected classes must be re-engineered and simplified a lot to make it maintainable and ready for next development. Because the changes in nodes/visualizer are supposed to performance reasons (reduce count of logic layer between a user data and its visualization, i.e. with using Looks API) thus TTV re-engineering should wait for new concept to avoid to resources waste. Also, a deep work on TTV have to wait to UI spec. TTV re-engineering is not supposed to start in a short time, the noted tasks block it for now. The most buggy areas in TreeTableView. 1) selections - there is spaghetti code of posting/reposting event from TreeSelectionModel to ListSelectionModel, multiply transformation mouse events. It must be cleaned a lot to make it maintainable. Issues: issue 28962, issue 29534, issue 29855, issue 34182, issue 34184 2) focus movement/keyboard control - A11Y is broken in TTV, it's unusable move focus around all table cells and edit them. Issues: issue 19266, issue 28561, issue 31501, issue 33641, issue 34169, issue 34170 3) performance - TTV generates <flown> of events with a impact to performance and responsiveness. Issues: issue 32567, issue 32462 4) UI layout. Issues: issue 22996, issue 29612, issue 30539, issue 34172, issue 34173, issue 34175, issue 34178 Tim's comment from 32462: "I was digging through the TTV code the other day, to try to solve that strange selection problem when the renderer was changed. It's pretty terrifying *and* heavyweight (embedding a JTree as a cell renderer and doing strange tricks to keep the other columns in sync). FWIW, it could probably be written infinitely more simply as a JTable with its own handling for expanding/closing/ indenting nodes - similar to the new PropertySheet." FWIW, here is how it could be approached, since the property sheet does a lot of the necessary stuff: 1. Recycle PropertySetModel - it really supplies a list of FeatureDescriptors, which is also parent class of nodes. 2. Recycle SheetTableModel - it's a tree model driven by the PropertySetModel, which fires events when the model changes (things are expanded/collapsed) 3. Add a method to get the nesting depth from the revised PropertySetModel. When painting, just set the border/insets of the renderer to some multiple of the nesting depth. 4. Recycle SheetTable - just about all of it is pretty similar, including code to insert editors. Treat edit requests as expansion requests if they occur in column 0. The only bummer is that we can't reuse the rendering/editing stuff that is package private to the propertysheet package - the code would be almost exactly the same as the value editing code for PropertySheet. Maybe we can find a way - I could expose a very neutral PropertyRenderer class that just paints a property using PropertySheet's renderers, and maybe something that supplies an editor also using that infrastructure. Hi, I wrote my own TreeTable implementation based on the code from the Swing connection web site. It's here and may be used to reimplement TreeTable. http://tasklist.netbeans.org/source/browse/tasklist/compiler/src/org/netbeans/modules/tasklist/compiler/treetable/ --Tim Interesting. I'm not sure the Swing Connection article takes the right approach for scalability/reliability, though - IMO using a table/tree as a cell renderer is a sort of strange hack that happens to work if you translate events back and forth accurately. At the same time, doing it right involves a heroic amount of effort - you basically need to write a TableModel backed by a TreeModel, and implement all of the expanded path management code from JTree in that model. There are some helper classes (FixedHeightLayoutCache, et. al.) that could possibly help, but it would be a pretty massive effort - I played with prototyping such a thing for a few hours this weekend to explore the problem space, and I figure it would be a solid month of work to get everything solid in such a beast. > ------- Additional Comments From Tim Boudreau 2003-10-13 00:22 PDT > > ------- > > Interesting. I'm not sure the Swing Connection article takes the > right approach for scalability/reliability, though - IMO using a > table/tree as a cell renderer is a sort of strange hack that happens > to work if you translate events back and forth accurately. As far as I know the current implementation uses another "hack". It places 2 components in a JPanel: a tree and a table. > > At the same time, doing it right involves a heroic amount of effort - > you basically need to write a TableModel backed by a TreeModel, and > implement all of the expanded path management code from JTree in that > model. There are some helper classes (FixedHeightLayoutCache, et. > al.) that could possibly help, but it would be a pretty massive > effort > - I played with prototyping such a thing for a few hours this weekend > to explore the problem space, and I figure it would be a solid month > of work to get everything solid in such a beast. > Well, in fact I already use my reimplementation in tasklist/compiler. --Tim Yes, the current implementation is rather nightmarish (note the number of bugs that can't be fixed without rewriting it). By all means, we should try yours - anything would be an improvement. The main places I can imagine strange corner cases are the same ones that have been huge headaches rewriting the property sheet - focus management and event synchronization (the latter is only a really big problem if the look and feel takes a "throw it on the event queue and pray" approach [this is how you get combo boxes that try to show their popups after they're offscreen & such]). Jirka, it's your baby, it's up to you...I'd welcome some change - my rewrite of property panel works well, but all of the focus and event gobbledygook going on in there makes the result less reliable than it could be when actually put into use. Hi Tim, thanks a lot for your concern about TreeTableView, I'm interested I'll read over your TTV implementation. Tim B. right marked current TTV as my nightmare, really needs improve (but small correction: it wasn't not my own child but was adopted ;-) I currently don't work on TreeTableView improvement I'm assigned to other work, code of TTV should be solve in timeframe investigating the explorer's views. I'll concern about it as soon as my time allows. Thanks and regards, -jiri Taking this issue, as I now own TreeTableView. Some notes, mainly for myself, or whoever ends up rewriting TTV: A good approach would be to share some pieces of infrastructure between TTV and the property sheet - both are tables that show properties; the list of properties is a separate model that drives the table model, so it wouldn't be much work to have TTV simply alter that model to change the contents of the TTV properties columns as nodes are expanded. More or less, we have a bunch of code already dedicated to showing properties, their inline editors, etc. in a table, managing the assorted focus issues, etc., which should be reused in both places. General things to do: - Move SheetTable, et. al. from org.openide.explorer.propertysheet to org.netbeans.modules.openide.explorer, so the necessary classes can be public without being part of the APIs, and TTV and the property sheet can share them. - TreeTable should subclass BaseTable (right now it duplicates a lot of code there), which is a generic table that does the row selection, painting logic, ensuring combos get opened on the first click, etc. - Refactor the property sheet a bit so that the properties come from the columns rather than directly from the model, and modify its table model to use strings fetched from the properties in the second column as values for the first column (right now getValue() on either column returns a Property or PropertySet, and renderers for one column display the name, and the other the value). This will make it possible to plug in a treemodel or table model that supplies nodes as the first column contents. This will also get us a rewrite of ListTableView (cousin of TTV and completely broken now) nearly for free. Making this task a P1 for Promo-D - I don't think we should let it slide much longer. FYI, there is now the beginnings of a TTV replacement in contrib/ttv (JTable based, and trying to be very simple straightforward in use and implementation). There's a simple file tree test app which is works there as well. It's basically just a JTable that takes a combined TableModel/TreeModel, a couple support classes, and model implementation and a convenience sub-model that makes it really easy to implement the part that fetches values for the non-tree columns based on the value in the tree column. General to-do's: - Large model support doesn't work yet - there are a bunch of places you have to explicitly tell FixedHeightLayoutCache that something has changed, and not all of them are implemented - There's an interface for providing display name/tooltip/icon, but the renderer doesn't use it yet - Updating the layout when the model data changes - Haven't tested editing support yet, but it should already work - comes for free with the JTable - Some extraneous things you can do with a JTable like expanding all nodes at once and so forth - we'll do those if there's a genuine need. TaskList folks, what I'd like to do re your tree table implementation is to continue with both for the time being, then test the performance and scalability of both approaches. I've described elsewhere what I dislike about starting with a JTable and doing strange things to make it behave like a table. But my taste in engineering approach doesn't matter here - what will work better, perform better and be more scalable and maintainable are the important things. Hi Tim (Tim Boudreau), let me know if I can help with my TTV-implementation. Hi Tim, did you already decided which approach you'll take? Nope, I haven't touched it in a few weeks - just concentrating on fixing bugs for 3.6 until it's out the door, then I'll have some cycles to come back to it. Tree table view rewrite will not make it into promotion D. Created a branch: cvs -d $HACKEDCVSROOT rtag -b -r BLD200404291800 ttv_rewrite openide nbbuild contrib Depending on the amount of changes requested in the HTML renderer before integration on Monday, I'll try to get at least the beginnings of something that compiles, displays and doesn't throw violent exceptions everywhere. No promises. Bookkeeping - ignore: I've factored the entire implementation of the propertysheet except public classes out and moved it into org.netbeans.modules.openide.explorer, so the new tree table will be able to use the same cell editor and renderer as the property sheet. As we may want to integrate this into the promotion D codeline (so any property sheet fixes for promotion D will be applied to the sources in their new location, making further work on the branch more convenient), I've tagged this: cvs -d$HACKEDCVSROOT rtag -r ttv_rewrite propsheet_refactoring openide_nowww A merge -j BLD200404291800 -j propsheet_refactoring will bring these changes into the main codeline. TBD if we'll do that now or wait until August; my preference is for now. AFAIK, between David and my work in contrib/ttv, this is effectively done. Does anybody plan to replace usages of TTV in NB with the new version? I'd imagine the debugger could benefit considerably. I would love to see bug #108861 fixed in the near future. Unfortunately it depends on this TreeTable rewrite and while this issue is flagged with P1 it, in turn, depends on other issues which are flagged P3/P4 and have been open for over five years. Shouldn't the priority of the blocking issues also be upped to P1? Fine with me. The rewrite is done and is quite usable in contrib/ttv and is already used by Nokia and Boeing in their platform apps for some time. Someone just has to deprecate TreeTableView and start switching existing components over to use OutlineView in contrib/ttv instead. The APIs are similar and it should be trivial. done 5ca2a45fd6a1 what a magic! Fixing issues in already finished release :) Changing the TM to correct M1 Amen. When I wrote the original version of contrib/ttv, I thought I would integrate it in the spring of 2005, then I changed jobs, then David did a lot of work to make it better. Now Standa has finally finished integrated it. Never thought it would take five years. All hail Standa! Integrated into 'main-golden', available in NB_Trunk_Production #234 build Changeset: http://hg.netbeans.org/main/rev/5ca2a45fd6a1 User: S. Aubrecht <saubrecht@netbeans.org> Log: #33281 - Outline & OutlineView There is a god, and his name is Standa. Thank you. [JG01] Missing Javadoc for OV.setProperties. [JG02] There is no apparent replacement for setRootVisible(false). Blocks issue #136664. [JG03] There is no apparent replacement for setDefaultActionAllowed(false). Not sure if it is necessary or not. [JG02] use getOutline().setRootVisible(false) |