-
Notifications
You must be signed in to change notification settings - Fork 617
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
consolidate project configurations to one place #1473
Conversation
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 like you're getting an error compiling src/bin/exrinfo/main.c
Ah, thanks for noticing the tools, I have to add a couple more tweaks to this PR. Yes, the bounds check fix accidentally crept in to the PR. If I rebase on latest that should resolve it. |
Signed-off-by: Nick Porcino <meshula@hotmail.com>
I removed the spurious compression fix. Now looking into why ImathConfig.h is not available to the tools when they are built |
Signed-off-by: Nick Porcino <meshula@hotmail.com>
@cary-ilm it would probably be clearer to simplify our cmake system again to use vanilla cmake instructions rather than an arguably ergonomic macro. I don't know that the result would be improved though, in terms of people seeing the dependencies that make sense. If it were a supported case that it's possible to build Iex without having to also build IlmThread without having to build OpenEXR, it might take extensive finagling to really enable that. |
Signed-off-by: Nick Porcino <meshula@hotmail.com>
f0cca7f
to
1a69aed
Compare
Signed-off-by: Nick Porcino <meshula@hotmail.com>
Signed-off-by: Nick Porcino <meshula@hotmail.com>
@cary-ilm I think this is good to go ~ I can't access the "1 change requested", hopefully it's something resolved during a previous push. |
Still not exactly what I had in mind, but it works for now at the very least. I'll submit an alternative shortly for review, I may be missing something. Thanks! |
I'm doing a build that is purely OpenEXRCore. We include <IlmThreadConfig.h> and others from various places in the code base. This change consolidates the OpenEXR C++ configuration incudes into openexr_conf.h so that it's possible to elide them in one spot instead of touching many files. This also ensures that the same preprocessor defines are set consistently across all the source files.
Also, this change adds a missing header guard include, and guards a macro that might be set by some c standard libraries, generating a macro redefinition warning.