iTwin / itwinjs-core

Monorepo for iTwin.js Library
https://www.itwinjs.org
MIT License
606 stars 210 forks source link

ViewState should participate in Viewport's zoomToElements/zoomToElementProps #6416

Open a-gagnon opened 8 months ago

a-gagnon commented 8 months ago

Problem: There is no easy way to override Viewport.zoomToElements and Viewport.zoomToElementProps to supply custom ranges to zoom to.

Context: I have specialized views (eg. subclasses of SpatialViewState) in which I display alternate geometry for some elements using Decorators. By that, I mean the pickId that passed to the GraphicBuilder is a persisted elementId.

I'd like to be able to supply range of the alternate geometry instead of the one that comes from the imodel. The approach used in Viewport's zoomToElements and zoomToElementProps always fetch the placement properties from the iModel to compute a range. It assumes the element will be displayed "as persisted" in the iModel which is incorrect for my use-case.

The whole UI system makes it difficult to subclass ScreenViewport and override the zoomTo methods for my needs. It seems like that would be easier if the function was moved inside the ViewState. I could easily override the logic in a subclass and use the right range to perform the zoomTo operation.

Thoughts?

Thank you! Alex

pmconne commented 8 months ago

Link to your relevant code please.

a-gagnon commented 8 months ago

@pmconne You can refer to the following sandbox for an example.

I cut a lot of corners coming up with a working sandbox example, but the idea is SpecializedViewState display no geometry except what comes from the ElementDecorator. This decorator takes the id of an existing element, uses it as a pickId to create line graphics from (0,0,0) to (10, 10, 0) in the bottom view.

I've added a small button that calls zoomToElements on every viewport at once.

pmconne commented 8 months ago

SpecializedViewState display no geometry except what comes from the ElementDecorator

Would you agree that subclassing SpatialViewState to achieve this is a pretty clumsy solution (albeit, potentially necessary due to what the current API offers)? Scene-based viewer API should address this need when it becomes available.

a-gagnon commented 7 months ago

Would you agree that subclassing SpatialViewState to achieve this is a pretty clumsy solution (albeit, potentially necessary due to what the current API offers)? Scene-based viewer API should address this need when it becomes available.

Yeah, I fully agree. Any ETA as to when this new API will become available? In the meantime, is there any working document about ir or alpha APIs?

pmconne commented 7 months ago

I have outlined the proposal verbally with Caleb and Joe. Actual implementation only just started yesterday. I want to tinker with the code for a bit before producing and sharing some more formal documentation - within the next week or two. We aim to have a beta API and prototype by the end of March. I will keep you in the loop as things progress.