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

2098 Archicad Connector geometry fidelity #2127

Merged

Conversation

jozseflkiss
Copy link
Contributor

#2098

New functions:

  • Import option for parametric or mesh import
  • LibpartImportManager
    • Edge status calculcation (welded, unwelded) based on vertex reusage and polygon directions
    • Transformation matrix introduced, but not currently used
    • Geometry reusage based on hash ids introduced, but not currently used
  • AttributeManager
    • Attribute generation cache
  • MeshModel
    • Speckle id sent to Archicad for content-based identification
    • Custom JSON serialization for Dictionaries for Graphisoft representation (e.g. "firts" and "second" props for pairs)

No Schema change.

Extra minor changes:

  • Beam, column common codes moved to central place
  • All hierarchical elements exported

Description & motivation

Changes:

To-do before merge:

Screenshots:

Validation of changes:

Checklist:

  • My pull request follows the guidelines in the Contributing guide?
  • My pull request does not duplicate any other open Pull Requests for the same update/change?
  • My commits are related to the pull request and do not amend unrelated code or documentation.
  • My code follows a similar style to existing code.
  • [] I have added appropriate tests.
  • I have updated or added relevant documentation.

References

With hard edges only:
image

image

Without edges:
image

@jozseflkiss jozseflkiss requested a review from a team as a code owner February 9, 2023 11:29
var enumerable = meshes as Mesh[] ?? meshes.ToArray();
foreach (var mesh in enumerable)
{
meshModel.vertices.AddRange(mesh.GetPoints().Select(p => Utils.PointToNative(p)));
Copy link
Member

@JR-Morgan JR-Morgan Feb 10, 2023

Choose a reason for hiding this comment

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

Quick comment on performance, minor, but I'd thought I'd share my findings from other connectors.

In other connectors, we have optimised the performance of MeshToNative quite significantly by avoiding using Points and reusing the scaling factor (calling Units.GetConversionFactor once per Mesh)

Creating a Point object for each vertex leads to a lot of heap allocations,
additionally, I see, PointToNative will call Units.GetConversionFactor for each point, which has a small cost, but one that adds up, especially when you consider meshes with high vert count.

Take this with a grain of salt, you may prefer to care about performance later, I'll leave this up to you.

@jozseflkiss jozseflkiss force-pushed the archicad/2098-Archicad-Connector-geometry-fidelity branch from 7594514 to c21e3f9 Compare February 10, 2023 13:19
Copy link
Member

@JR-Morgan JR-Morgan left a comment

Choose a reason for hiding this comment

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

🙌 amazing

@AlanRynne AlanRynne merged commit 2f0f397 into release/2.12 Feb 10, 2023
@AlanRynne AlanRynne deleted the archicad/2098-Archicad-Connector-geometry-fidelity branch February 10, 2023 15:17
@clairekuang clairekuang added archicad enhancement New feature or request labels May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archicad enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants