-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[opengl] add TI_ENABLE_OPENGL env var to disable OpenGL #962
Conversation
Thanks! I'm at a meeting but I'll take a look in the afternoon! Please don't wait for me - take care and get enough sleep :-) |
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.
Looks good except for a few places to discuss.
taichi/util/environ_config.h
Outdated
|
||
TLANG_NAMESPACE_BEGIN | ||
|
||
static int get_environ_config(const std::string &name, int default_value = 0) |
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.
Does it make sense to make an interface like template <typename T> get_environ_config(const std::string &name, const T &default_value) { ... }
, and then specialize it for int
?
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.
I guess use override functions instead of T
is better idea, since the func stoi
and stof
.
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.
You can always use different conversion functions in different specializations. Or you can simply use if constexpr
.
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.
Thanks! LGTM now. Let's have this released in v0.6.4 so that people using virtual machines can have a try :-)
Related issue = #958 (comment)
[Click here for the format server]