DynamoDS / DynamoRevit

Dynamo Libraries for Revit
https://dynamobim.org
328 stars 184 forks source link

AxonView.ByEyePointTargetandBoundingBox: View Created isnt Bound by Bbox #1449

Open twiceroadsfool opened 7 years ago

twiceroadsfool commented 7 years ago

Dynamo 1.2.1 Revit 2017.1.1 and Revit 2016 R2 UR7 Windows 10 1607

Graph goes:

Get bounding box from {3D} view Section Box (which is showing extents of entire project). Create geometry from that Get all levels (and level above) Create geometry for each "level sliver." Get bounding box for each piece of level sliver geometry Create Axon View for each, with Eye, Point, Target, and BBox. The last step is all thats not working: It creates all the views, but they:

Have no Section Box enabled Are not cropped to the Bounding Box at all (they show the extents of the entire model) Are Locked (Locked Orientation = 1), and not an editable parameter

We expected to see Section Boxed 3D Views, one per level in the model.

Parallax Team-01-Project Prep-Construction-3D Coordination-Create Export Views by Level.zip

For what its worth, during troubleshooting we also tested the PerspectiveView.ByEyePointTargetAndBoundingBox, and it didnt work as expected either.

Am i thinking these nodes do something differently than they are meant to do?

dimven commented 7 years ago

I reported a similar issue a while ago on this node. Maybe we can extend this request and close the old one? https://github.com/DynamoDS/Dynamo/issues/4251

mjkkirschner commented 7 years ago

@kronz this was tracked with https://adsk-oss.myjetbrains.com/youtrack/issue/MAGN-7161 should we bring it over - not sure if it has been yet?

ksobon commented 7 years ago

@kronz @mjkkirschner Guys help me understand what the thinking is with the current nodes. We have three (3) methods that in my opinion should be doing this:

  1. AxonometriView.ByEyePointAndTarget:

This should create a new view from two points and a name.

  1. AxonometricView.ByEyePointTargetAndElement

This should create a new view from two points and a name. Then there is an input for Element and isolateElement boolean flag, which i fail to understand. I think that what it should be doing is isolating elements that are passed to Element input. Why the isolateElement flag? Can't we just check if input is null then flag is false by default, if its not then we want to isolate something. Also, its a single element input. Why not allow a list?

  1. AxonometricView.ByEyePointTargetAndBoundingBox

This should create a new view from two points and a name. Then there are two additional inputs of isolateElement boolean and bounding box. In my understanding the goal here was to use the section box property of the 3d view to set it from a bounding box. Somehow this is trying to set a crop box from a bounding box. It's possible, but not exactly most straightforward for users to understand as its using a 3d bbox to set up a 2d crop box. The issue here, and why it fails lies in order of things. The way AxonometricView class is currently set up it creates the view, sets its orientation and locks it. Only then it tries to set the crop box. I am not sure if that's possible, it sure is not for section box. The view, once locked disables certain functionality, so order is important. Also, just like above I don't see a reason for a isolateElement flag here. It currently does nothing.

Is that more or less what we are looking at here? If that's the case then i took a stab at fixing the bounding box view like this:

image image

I removed the isolateElement input, and instead of a CropBox set SectionBox. That makes more sense here. We can expose ability to set CropBox on a view class for ALL views. That would make more sense than doing it individually for each one like here. Let me know if this makes sense and I can push that into the repo.

Cheers!

kronz commented 7 years ago

@ksobon thanks for giving this some consideration. As far as what our thinking was on this functionality, these are REALLY old nodes that were done while we were still figuring a number of things out, and previous to our being able to supply default values. The isolation fucntions were made with the idea of supporting panel documentation workflows in particular, so that normal views could be generated with the panels in isolation. Your proposal sounds good, and if you have changes that feel more sensible, please issue a pull request, and we'll merge them in after validating them.

For why certain functionalities likeisolate don't seem to do anything . . . I'm pretty sure they used to, I wonder if there are obsolete API functionalities that they call. @ikeough made these initially, Ian can you weigh in on what happened to isolate functions?