-
Notifications
You must be signed in to change notification settings - Fork 373
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
osltoy - Feature Request: custom #include search paths #1876
osltoy - Feature Request: custom #include search paths #1876
Conversation
Signed-off-by: Maxwell Iverson <[email protected]>
…nclude search paths passed to the compiler Signed-off-by: Maxwell Iverson <[email protected]>
Signed-off-by: Maxwell Iverson <[email protected]>
/easycla |
src/osltoy/osltoyapp.cpp
Outdated
while (!m_lines.empty()) | ||
pop_line(); | ||
|
||
|
||
m_maxIndexWithContent | ||
= (int)paths.size() | ||
- 1; // ok that this is -1 if the paths are empty | ||
|
||
|
||
|
||
auto initialLineCount = required_lines(); | ||
|
||
m_lines.reserve(initialLineCount); |
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.
As a style nit, we tend to only put blank lines within a function body where the extra whitespace is clarifying -- for example, if you have several code statements that are doing a well defined task A, followed immediately by several statements that do a separate task B, then a single blank line between them can help understanding of the conceptual grouping of the lines into tasks. But if the space doesn't especially clarify the algorithm, you may as well omit it and benefit from the function being compact. Also, even when whitespace makes it more clear, a single line suffices.
(I'm referring to WITHIN a function body. I also like a single line between function definitions of a class definition. But any freestanding functions, my personal preference is 3 lines between functions.)
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.
Gotcha! That makes perfect sense; I definitely have a bad habit of inserting extra line breaks while working on something for my own legibility and subsequently forgetting to remove them later on. I'll fix that now.
…n deleted previously. Signed-off-by: Maxwell Iverson <[email protected]>
Signed-off-by: Maxwell Iverson <[email protected]>
Noting that when the automated test suite ran on Friday evening for this pull request, the icx build got caught in an infinite loop at some point while building the BVH. Not that it's particularly relevant to this, but I wanted to point it out as it appears to be another case of #1874 cropping up |
The icx thing is a known problem, we're looking into it |
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 much nicer now, thanks.
Merging!
Description
For Dev Days
#1862
Addressing a osltoy feature request, I've added the ability to specify custom search paths.
The changes consist of the following:
-I <path>
to specify search paths (OIIO::ArgParse
unfortunately can't mirrorOSLCompiler
's-Ipath
syntax)OSLCompiler
options alongside a flag indicating whether or not they should be regenerated before compilation. The only option currently able to be specified is-Ipath
, but this should ease any future extension.Tests
I did not add any automated tests for this feature, as is consistent with the rest of osltoy. I did, however, do some manual testing to make sure the functionality works as expected.
Shaders that failed to
#include
some file instead succeeded when the parent directory of that file was added to the list of search paths via both the command line and the GUI component. Respectively, when said search path is then removed viathe GUI component, any subsequent compilation attempts fail again. The GUI component otherwise behaves as expected.
Checklist:
already run clang-format v17 before submitting, I definitely will look at
the CI test that runs clang-format and fix anything that it highlights as
being nonconforming.