-
Notifications
You must be signed in to change notification settings - Fork 607
Refactor AppOptions and add larger-than-RAM feature flag
#12256
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
Conversation
|
Web viewer built successfully.
View image diff on kitdiff. Note: This comment is updated whenever you push a commit. |
abey79
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.
lgtm (though this minor snapshot change is odd)
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.
The new snapshot has different highlight on the top right "show left panel" icon. Not sure why or if it even matters.
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.
Maybe it was below the threshold of error on main? 😬
Only visible in debug builds.
I couldn't resist grouping all the video settings into its own struct. This unfortunately means the video settings will reset to default in the next Rerun release. But having a flat structure of everything just isn't scalable.
In order to be able to read the "larger-than-ram" setting in time, I also had to re-arrange the startup code for the viewer slightly.
Let's hold off merging until we cut the 0.28 release branch.