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

refactor(etabs): adds singleton converter cache for material and section relationships #514

Conversation

clairekuang
Copy link
Member

Changes the current material/section proxy logic to use a singleton cache in the converter which will store object <> section <> material relationships to be accessed in the connector

adamhathcock and others added 2 commits January 22, 2025 10:44
#511)

* This is a workaround for Revit's order of operations when initializing

* Fix event listening
@clairekuang clairekuang changed the base branch from dev to bjorn/cnx-880-add-sections-and-materials-as-proxies January 22, 2025 13:37
Copy link

codecov bot commented Jan 22, 2025

Codecov Report

Attention: Patch coverage is 51.16279% with 21 lines in your changes missing coverage. Please review.

Project coverage is 14.74%. Comparing base (0e26ee8) to head (d302b15).
Report is 1 commits behind head on bjorn/cnx-880-add-sections-and-materials-as-proxies.

Files with missing lines Patch % Lines
...UI3/Speckle.Connectors.DUI/Bridge/BrowserBridge.cs 0.00% 12 Missing ⚠️
...nectors.DUI/Eventing/EventSubscriptionException.cs 0.00% 3 Missing ⚠️
...I3/Speckle.Connectors.DUI/ContainerRegistration.cs 0.00% 2 Missing ⚠️
...eckle.Connectors.DUI/Eventing/DelegateReference.cs 33.33% 1 Missing and 1 partial ⚠️
...I3/Speckle.Connectors.DUI/Eventing/SpeckleEvent.cs 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@                                   Coverage Diff                                   @@
##           bjorn/cnx-880-add-sections-and-materials-as-proxies     #514      +/-   ##
=======================================================================================
- Coverage                                                15.05%   14.74%   -0.32%     
=======================================================================================
  Files                                                      239      239              
  Lines                                                     4703     4674      -29     
  Branches                                                   578      576       -2     
=======================================================================================
- Hits                                                       708      689      -19     
+ Misses                                                    3964     3954      -10     
  Partials                                                    31       31              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

adamhathcock and others added 4 commits January 22, 2025 15:19
…nt Subscription (#512)

* This is a workaround for Revit's order of operations when initializing

* Fix event listening

* Only allow methods on classes as opposed to anonymous lambdas

* formatting

* fix tests

* weakreference found should remove subscription
@bjoernsteinhagen
Copy link
Contributor

@clairekuang sample commit with the refactor here.

Looking good!

image

@clairekuang
Copy link
Member Author

Ok to merge then!

@bjoernsteinhagen bjoernsteinhagen merged commit e90423a into bjorn/cnx-880-add-sections-and-materials-as-proxies Jan 22, 2025
5 checks passed
@bjoernsteinhagen bjoernsteinhagen deleted the claire/singleton-caching branch January 22, 2025 19:34
bjoernsteinhagen added a commit that referenced this pull request Jan 22, 2025
* HostAppVersion

v21 and v22 following SDK updates

* Working POC for material and section proxies

* Refactoring section unpacking

Refactoring in accordance with PropertiesExtractor example

* Material unpackers

* Notes and documentation

* More explanations

* materialId

- Interim solution for viewer filtering is appending the materialId to assignments for each object
- For FRAME this was easy
- For SHELL not so easy. No GetMaterial method avaiable given a AreaObj sectionName. Implemented lightweight materialCache based on cDatabaseTable. Marked as temporary based on previous discussions

* Explicit dictionary entries

* Repeated property strings as consts

- Fair point for repeated strings in the CsiMaterialPropertyExtractor.cs
- Even more reason to include this across all repeated strings. Categories of properties screaming out for this. Added additionally

* PR review comments

- Dictionary lookups for material and section proxies
- Only create proxies for assigned sections and materials (not pretty)

* refactor(etabs): adds singleton converter cache for material and section relationships (#514)

* This is a workaround for Revit's order of operations when initializing (#511)

* This is a workaround for Revit's order of operations when initializing

* Fix event listening

* adds a singleton cache for material and section relationships to csishared

* updating extraction results and simplifying classes

* Only allow methods on classes as opposed to anonymous lambdas for Event Subscription (#512)

* This is a workaround for Revit's order of operations when initializing

* Fix event listening

* Only allow methods on classes as opposed to anonymous lambdas

* formatting

* fix tests

* weakreference found should remove subscription

* doument model store fix (#516)

* testing commit

---------

Co-authored-by: Adam Hathcock <[email protected]>
Co-authored-by: Björn <[email protected]>

* resolving conflicts, testing and small tweaks

- merged dev into branch
- added "type" parameter to group proxies for sections in order to distinguish between frame sections and shell sections

---------

Co-authored-by: Claire Kuang <[email protected]>
Co-authored-by: Adam Hathcock <[email protected]>
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