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

Revit_Core_Engine: pull of windows from link fixed #1281

Merged
merged 2 commits into from
Nov 29, 2022

Conversation

pawelbaran
Copy link
Member

Issues addressed by this PR

Closes #1280

Test files

See #1280

Changelog

Additional comments

From the two issues raised in the original issue:
Null reference was an easy fix, occurred when the solid was entirely outside of the half space, which makes perfect sense.

Generating ids error originates from an edge case where the element had its geometry defined both as instance geometry and element geometry (from what I understand, instance geometry is the geometry created based on the family definition, while the element geometry is what it actually is in the model - see Revit API docs for more details):

  • usually walls have only element geometry, for some weird reason some of them also have instance one:
    image
    ...from what I understand, instance geometry should be ignored in this case because it does not really exist in the model
  • FamilyInstance should only have instance geometry, but who knows what will Revit come up with...
  • I have found elements that have both element and instance geometry (see railing below), but apparently the element geometry is invalid then
    image

...so I drew a conclusion that the procedure should be as follows:

  • if the element has a valid element geometry, return only this geometry
  • otherwise return instance geometry

I made the above assumptions after longer investigation, but I cannot fully guarantee that it is valid for all cases. Therefore, I strongly recommend wider testing using the standard testing procedures before this gets merged @enarhi.

@pawelbaran pawelbaran added the type:bug Error or unexpected behaviour label Nov 24, 2022
@pawelbaran pawelbaran requested a review from enarhi November 24, 2022 20:39
@pawelbaran pawelbaran self-assigned this Nov 24, 2022
@pawelbaran
Copy link
Member Author

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 24, 2022

@pawelbaran to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer

There are 35 requests in the queue ahead of you.

Copy link
Member

@enarhi enarhi left a comment

Choose a reason for hiding this comment

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

LGTM. Tested with the original test file and the pull bug is fixed. Also tested Pulling Physical Panel Locations and Pulling FacadeEElements from the Revit test procedures, and both worked as expected. @pawelbaran please flag if other testprocedures may be required based on your changes, otherwise this one is good to go from my end.

@pawelbaran
Copy link
Member Author

@BHoMBot check copyright-compliance
@BHoMBot check dataset-compliance
@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 29, 2022

@pawelbaran to confirm, the following actions are now queued:

  • check copyright-compliance
  • check dataset-compliance
  • check ready-to-merge

@pawelbaran pawelbaran merged commit 3c0ec50 into main Nov 29, 2022
@pawelbaran pawelbaran deleted the Revit_Toolkit-#1280-RevitWindowPullFromLinkBug branch November 29, 2022 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Error or unexpected behaviour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows Sometimes Fail When Pulling from Link
2 participants