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

fix(Revit): DirectShape Instances to Speckle Conversion support #3572

Merged
merged 4 commits into from
Jan 7, 2025

Conversation

jsdbroughton
Copy link
Contributor

@jsdbroughton jsdbroughton commented Jul 15, 2024

DirectShape Geometry instances are nested Mesh and Solid within a GeometryInstance objetc. Previous conversion had switch case handlers only for the geometry types, this adds a recursion to cater for nested instances.

@jsdbroughton jsdbroughton marked this pull request as ready for review September 13, 2024 01:36
@jsdbroughton jsdbroughton changed the title For POC purposes - implements a fix - needs investigation fix(Revit): DirectShape Instances to Speckle Conversion support Sep 13, 2024
@jsdbroughton jsdbroughton enabled auto-merge (squash) September 13, 2024 01:51
Copy link
Member

@AlanRynne AlanRynne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, but it opens the door to non-meshes to end up being part of the directShape, which will be ignored when creating the Speckle DirectShape as we have the following validation call 👇🏼

Not sure if every GeometryInstance will fit in this 👇🏼

Screenshot 2024-09-13 at 16 27 24

@jsdbroughton
Copy link
Contributor Author

jsdbroughton commented Jan 2, 2025

Not sure if every GeometryInstance will fit in this 👇🏼

@AlanRynne > Ultimately, as this the nested GeometryInstance handling is recursive if a GeometryInstance, the method will still only return a Mesh or a Solid, never a GeometryInstance itself.

@jsdbroughton
Copy link
Contributor Author

This fix will never make it to v2 i suppose. nevertheless, it might be of interest to whoever owns the v3 Revit @clairekuang @oguzhankoral

@jsdbroughton jsdbroughton requested a review from JR-Morgan January 7, 2025 11:13
@jsdbroughton jsdbroughton merged commit 62009cb into dev Jan 7, 2025
14 of 32 checks passed
@jsdbroughton jsdbroughton deleted the jsdb/revit/directshape-instances branch January 7, 2025 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants