Tree widget: Use provided "classification -> category" relationship name to get categories

[dlew72]

Package
@itwin/tree-widget-react

Describe the bug
When toggling visibility of nodes in the classification tree, the visibility handler targets the categories of the elements under a classification. This doesn't work with the complicated data structure of OpenSite+. We have input and output elements, and only the input elements are classified. These elements all have different categories. Additionally, some of the output elements share categories between classifications.

To Reproduce
In OpenSite+, create two parking lot classifications, and add a parking lot to each. Clicking hide next to Classification-1 will cause some elements from Parking Lot 1 to disappear, and some elements from Parking Lot 2 to disappear. Ideally, all elements from Parking Lot 1 would disappear, and none of the elements from Parking Lot 2 would disappear.

Expected behavior
OpenSite+ already has logic to calculate and hide necessary output elements when the user tries to hide an input element (listener on the neverDrawn set). So all we need from this is for each classified element under a given classification to be hidden rather than it's categories.

Desktop (please complete the applicable information):

• OS: [e.g. iOS]
  OS: Windows
  Electron 36.9.1
  iTwin.js 5.6.0

[grigasp]

Due to historic reasons, the Classifications tree uses categories to control classifications' visibility. Sounds like we should switch to using elements directly.

However, my concern with that is whether display system will cope with possibly thousands (maybe even tens or hundreds of thousands) of elements in the always / never displayed sets. @iTwin/itwinjs-core-display, any insights?

[eringram]

I'm not super familiar with that part of the code, but based on initial reading:

• Calling FeatureSymbology.setNeverDrawnSet / FeatureSymbology.setAlwaysDrawnSet calls copyIdSetToUint32Set, which iterates over all of the IDs to be copied, so could be slow if the ID set is very large
• The performance of rebuilding the feature overrides texture seems to depend on the number of features per tile not the number of elements, so wouldn't be affected by very large always/never drawn sets
• However if both of these need to be done frequently, like the visibility is toggled often, it might have performance impacts?

If this is a priority to fix, it might be useful to do an initial experiment where we test always/never drawn sets with thousands of elements and see where the bottlenecks are. cc @markschlosseratbentley if you have any other thoughts on this

[markschlosseratbentley]

I agree with @eringram's assessment to consider doing a simple experiment to see if performance is up to expectation based on the above-described system architecture.

It would also be worthwhile to consider if the data itself could be grouped in an organization more amenable to filtering visibility based solely on categories.

[grigasp]

• Calling FeatureSymbology.setNeverDrawnSet / FeatureSymbology.setAlwaysDrawnSet calls copyIdSetToUint32Set, which iterates over all of the IDs to be copied, so could be slow if the ID set is very large

To make it clear, we're calling Viewport.setNeverDrawn and Viewport.setAlwaysDrawn.

I agree with @eringram's assessment to consider doing a simple experiment to see if performance is up to expectation based on the above-described system architecture.

I'll let @dlew72 comment on priority, but it's for OS+, so I guess we need a solution sooner rather than later.

It would also be worthwhile to consider if the data itself could be grouped in an organization more amenable to filtering visibility based solely on categories.

@diegoalexdiaz, adding you since I know you're familiar with the relationships between elements / categories / classifications in OS+'s iModels. To use categories instead of elements directly, we need them to match this criteria:

• Elements are related with at most one classification.
• A category is not used for elements related to different classifications.
• Authoring elements and driven elements belong to the same category.

[diegoalexdiaz]

I talked to @dlew72 offline to refresh my memory on this topic vs. how it currently works.

In summary:

• OS+ currently associates Input (authoring) and some output (plan-projection) elements with one classification.
• Each classification is associated with one or more categories via relationships that provide the app with their context.
• Each category is associated with only one classification.
• Some output elements (physical/3D) use categories that the OS+ team considers "legacy": created early-on during the development of the product, which are not associated to any classification.

The display system is optimized around models & subcategories, thus I think the best course of action here is to address the last bullet point: categories used by physical/full-3D elements should be associated to classifications in the same manner as the other kinds of elements. That way, controlling the visibility of a classification can be directly translated into turning a set of categories on/off - which will perform and scale much better than an element-driven visibility approach.

[grigasp]

@dlew72, just to confirm - the agreement is that the fix is on your side, correct? If yes, I'd like to close this issue.

[dlew72]

I support using categories as the visibility driver for this use case, and a lot of the work is on our side, but there's one caveat that I'd like some clarification on. Since any one classification is usually related to multiple categories, we would need a way to supply a query to get from a classification id to a list of category ids. Is that functionality exposed in the tree today?

It has to do with our data structure, since the input Authoring element is the one that gets classified, but it has a throwaway category and no graphics. We would need to hide the multiple categories of our plan and physical output elements. We have a relationship in our schema to describe this classification-->category(s) link, we'd just need exposure in the tree to use it.

[grigasp]

We have a relationship in our schema to describe this classification-->category(s) link, we'd just need exposure in the tree to use it.

I guess there are a couple of options:

1. We take the relationship name and take care of finding the categories based on classification id. Less work for you.
2. We take a callback like getCategoryIds(classificationIds: Id64Arg): Promise<Id64Array> and it's up to you to implement it. This is more work for you as you'd probably want categories to be cached, etc. But it would allow you to reuse the result for other uses, if that's needed.

Which one do you prefer? Or maybe have other ideas?

[dlew72]

Option 1 would be best for us. Thanks a ton

[grigasp]

@JonasDov, could you take this? I think the relationship name prop should be optional and we should fall back to existing behavior if it's not provided.