Select layers with selected items not working

[mukoki]

The command Edit > Selection > Select Layers with Selected Items is not working (no layer is selected). Layers with selected items are identified (SelectionManager works fine), but TreeLayerNamePanel miss to select nodes corresponding to thoses layers. Probably happens in the TreeUtil.

[edeso]

seems to have been broken in OJ 1.16 already. let me have a look.

[edeso]

looks like you broke it with this commit. testing for category breaks the iteration. https://github.com/openjump-gis/openjump/commit/d2ff6f180c0f1d1f54202d3e16e3e9272b10eee0#diff-52082054fc6a59bf222475b6eefb3bd603e15f34b724191ce1b21cc87fa444baR133-R139 removing the condition or just using visit(TreeModel model, Stack path, Visitor visitor) in the calling code fixes the issue.

why did you duplicate these methods

    public static void visit(TreeModel model, Visitor visitor)
    public static void visit(TreeModel model, TreePath path, Visitor visitor)
    private static void visit(TreeModel model, Stack path, Visitor visitor)

anyway? what other TreeComponents are visitCategoriesAndLayerables() methods supposed not to visit?

[mukoki]

Thank you for the hint Ede. You was right. visitCategoriesAndLayerables is supposed to not visit ColorThemingValue, RasterStyleValueIntv and RasterStyleValueRamp, but I missed the root node. Should be fixed.

[edeso]

no problem. still don't like the code duplication though.

would you mind if i move the condition into the Visitor?

[mukoki]

Improve it if you like. Note that the intention was probably to avoid visiting all the tree nodes "in certain cases" (when we know that only layers are concerned). In the general case, there is no reason to skip ColorThemingValue nodes. That said, there is probably a cleaner way to manage it.

[edeso]

hmmm,

looking at the code it seems that instead of looping over the complete tree for each node we are looking for https://github.com/openjump-gis/openjump/blob/1bb5d3f0178dd90e04a4fb77fb965a2f51c59cb2/src/com/vividsolutions/jump/workbench/ui/TreeUtil.java#L172-L186

we should implement a public static TreePath findTreePath**s**() instead that takes a list of nodes and iterates only once over the tree and stops immediately when all are found.

sounds reasonable?

[mukoki]

Hi Ede,Oh, I see, but this is a different problem.Right, if we are looking several layers, we currently have to visit several times the tree and your approach can probably save time.I think the optimization I tried to do is that we know that categories contain layerables which may contain values (e.g. with ColorTheming) and visiting everybody when we know we are looking for a Layer is just wasting time. I can't remember where/when/why I noticed that. It should not be so long except maybe if we have layers with ColorTheming containing thousands of distinct values. It may happen after a bad attribute setting (ex. using a unique id for the color theming), but it may happen and maybe it sometimes freeze the application or something like that.Michaël envoyé : 30 janvier 2022 à 22:21de : edeso @.>à : openjump-gis/openjump @.>cc : Michaël Michaud @.>, Assign @.>objet : Re: [openjump-gis/openjump] Select layers with selected items not working (Issue #42) hmmm,looking at the code it seems that instead of looping over the complete tree for each node we are looking for https://github.com/openjump-gis/openjump/blob/1bb5d3f0178dd90e04a4fb77fb965a2f51c59cb2/src/com/vividsolutions/jump/workbench/ui/TreeUtil.java#L172-L186we should implement a public static TreePath findTreePaths() instead that takes a list of nodes and iterates only once over the tree and stops immediately when all are found.sounds reasonable?—Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were assigned.Message ID: @.***>

[edeso]

hey Mike,

pls check https://github.com/openjump-gis/openjump/commit/ba4953a1e17a3764ca0ebf7b90882603747caac6

i pretty much added both. a find of multiple nodes in on e go plus the ability to filter where to recurse in to.

[mukoki]

Wow, this is a big refactoring,I had a quick look at some plugins using indirectly findLayerTreePath and did not find any regression so far.Hopefully, TreeLayerNamePanel will work more smoothly (it is a component where we still used to have bugs or slowness)Michaël envoyé : 1 février 2022 à 00:52de : edeso @.>à : openjump-gis/openjump @.>cc : Michaël Michaud @.>, Assign @.>objet : Re: [openjump-gis/openjump] Select layers with selected items not working (Issue #42) hey Mike,pls check ba4953ai pretty much added both. a find of multiple nodes in on e go plus the ability to filter where to recurse in to.—Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were assigned.Message ID: @.***>

[edeso]

looking bigger than it actually is. my search for TreeUtil.findTreePath[s] usage btw. just finds the one below in TreeLayerNamePanel. which plugins are you referring to?

NOTE: findLayerTreePath() is removed (because of the doubled code) and replacement is a usage like this https://github.com/openjump-gis/openjump/blob/ba4953a1e17a3764ca0ebf7b90882603747caac6/src/com/vividsolutions/jump/workbench/ui/TreeLayerNamePanel.java#L753-L757