-
Notifications
You must be signed in to change notification settings - Fork 25
Speed up controllers #682
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
Speed up controllers #682
Conversation
|
@macumber could you add a quick walkthrough of your code via review + some quick text explaining why removing the mutex is the right choice please? |
| bool m_itemsDraggable; | ||
| bool m_itemsRemoveable; | ||
| OSItemType m_type; | ||
| bool m_dirty; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m_dirty wasn't actually controlling anything here, all that refresh was doing was to clear m_dirty
| delete m_reportItemsMutex; | ||
| } | ||
|
|
||
| void ConstructionObjectVectorController::reportItemsLater() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only a few OSVectorController derived classes were specialized to have the reportItemsLater slot which schedules a call to OSVectorController::reportItems. This PR pulls that functionality into the OSVectorController. Now reportItems will schedule a call to a reportItemsImpl method that will run once so we don't report (and redraw) the same objects multipe times per execute loop.
| openstudio::IddObjectType m_iddObjectType; | ||
| model::Model m_model; | ||
| bool m_showLocalBCL; | ||
| bool m_isLibrary; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m_showLocalBCL meant to show LocalBCL objects, basically just meant that this was a library controller, I thought m_isLibrary was more clear.
jmarrec
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I follow everything but I trust your judgment.
Fixes #681 by moving optimizations in certain controllers to all controllers