Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

show view range - match view extents #2245

Closed
RevitIRL opened this issue May 8, 2024 · 14 comments
Closed

show view range - match view extents #2245

RevitIRL opened this issue May 8, 2024 · 14 comments
Labels
New Feature New feature request [class->Implemented #{number}: {title}]

Comments

@RevitIRL
Copy link

RevitIRL commented May 8, 2024

Is your feature request related to a problem? Please describe

Trying to show users visually what is in a plan view

Describe the solution you'd like

Color planes should match what's shown in the view (not the extents of Level1)

Describe alternatives you've considered

grin & bear it :)

Additional context

@RevitIRL RevitIRL added the New Feature New feature request [class->Implemented #{number}: {title}] label May 8, 2024
@thumDer
Copy link
Contributor

thumDer commented May 8, 2024

It was intentional to use the level, because calculating a views outline can be tricky if it is not cropped. An alternative could be to use the view's outline if it is croped but fallback to the level's if it is not.

@RevitIRL
Copy link
Author

RevitIRL commented May 8, 2024

I'd prefer that method - and that's what I would expect (using the view's Associated Level if no Crop Region).

from the other thread:
maybe you could get the bounding box of the view instead? [though it would be nice,] If it's not precisely the outline of the crop region, that's fine (i.e. it can be rectangular), but sometimes our level is huge, but the plan view is smaller

@thumDer
Copy link
Contributor

thumDer commented May 8, 2024

The extents of the crop is easy. If there is no crop the extents has to be calculated from the complete model and that is quite expensive.

@thumDer
Copy link
Contributor

thumDer commented May 14, 2024

I've changed the code to use the views extents. Do we need the level's extents as a fallback, or should the UI warn about turning on the crop? I think the different behaviour for uncropped views can be confusing without explanation, and most of the views are cropped anyway.
Thoughts? @RevitIRL @jmcouffin

@jmcouffin
Copy link
Contributor

@thumDer I'd go for the warning IMHO

@thumDer
Copy link
Contributor

thumDer commented May 14, 2024

I've also realized that I would need to combine that pre-check/warning with the 3d view's sectionbox check, because if the 3d view has a section box the script will use its extents for the planes, and the levels were only a fallback. So now the crop will be a fallback, and if there is none the warning will ask the user to turn on the section box OR the crop. Deal?

@jmcouffin
Copy link
Contributor

@thumDer deal
One thing I noticed is that in the Revit sample house (imperial) the plans are offset up by the inverse of the project base point vertical displacement

@thumDer
Copy link
Contributor

thumDer commented May 14, 2024

@jmcouffin Interesting. I don't have the imperial sample house right know, could you send it over to me somehow?

@jmcouffin
Copy link
Contributor

@jmcouffin Interesting. I don't have the imperial sample house right know, could you send it over to me somehow?

https://drive.google.com/file/d/1Vj5GuygVbzUShW91LJtvIp-ACSj20tkc/view?usp=sharing

@thumDer
Copy link
Contributor

thumDer commented May 14, 2024

thanks, already fixed by 5ddd651
we always have our PBP at our internal origin, so never came accross this difference, but nice catch!

@jmcouffin
Copy link
Contributor

shall we close the issue?

@thumDer
Copy link
Contributor

thumDer commented May 14, 2024

That fix I comitted is only for the elevation issue, I still need some time to finalize to crop based extents, only slight adjustments, safeguard mechanisms.

thumDer added a commit that referenced this issue May 14, 2024
plane extents from crop instead of level, closes #2244, closes #2245,
better selection handling,
better doc changed event handling,
API object validity check,
view change necessity check
@thumDer
Copy link
Contributor

thumDer commented May 14, 2024

done. should we close this manually or let it close itself on next release?

@jmcouffin
Copy link
Contributor

Manually, that makes me feel better to see the number of issues going down, that's me liking things tidy🤭

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Feature New feature request [class->Implemented #{number}: {title}]
Projects
None yet
Development

No branches or pull requests

3 participants