Cherry picked fixes for proposed 1.1.x release #678
Conversation
…ndation#638) * Fix window-specific issue in GetEnv() While the content of result was correct, its length was set to a wrong size. This is a patch by Patrick Hodoul. * Fix wrong display returned by getDefaultDisplay() Solved by ignoring environment variable if it's an empty string. Ideally, need to test it to be NULL instead to support obscure case when empty string is passed. Unfortunately, this is not possible with the current API, and is unlikely scenario anyway. The issue was caused by wrong logic around parsing display/views override environment variable: if it is missing, it was converted to an empty string. That empty string was considered an override of one element display, with an empty name. Additionally, make it so getDisplay() returns displays in order of either active_displays configuration variable or environment variable override. the issue there was caused by wrong order of arguments to vector intersection. Conflicts: src/OpenColorIO/Platform.cpp src/core/Config.cpp This commit was cherry-picked from 718cc7f. Changes were made for compatibility with the older version of Platform.cpp; mainly reverting "Getenv" to "getenv".
…on#601) This fix adds compatibility with OIIO 1.9+, minor ImageInput/ImageOutput API changes. At a certain point ImageInput::create switched from returning a raw pointer (that later needed to be deleted) to returning a std::unique_ptr.
|
Hitting same CI failures as #677 RB-1.1 CI env might just need some tweaking. AppVeyor is failing as the VS2015 image is missing Boost. The Travis Mac Clang build is failing to "find a suitable distribution" of markupsafe. |
|
Oh, oops, I didn't realize I was changing C++ standard requirements. (C++ 03 compat in 2019, ok...) I don't know how to reconcile that. OIIO 2.x certainly is only C++11 or higher, but a patch release should definitely not force a different compiler. |
|
I wonder if we could extend the use of the OIIO version macro to revert to the C++ 03 friendly constructors when appropriate. That or an explicit opt-in cmake property to choose. |
|
I'll cherry pick #682 into this PR once that is merged into master |
As no C++ version is explicitly mentioned in the makefiles, the gcc default version is C++98 (for gcc version < 6.1) while Vc++ default is latest (i.e. depends on the Visual Studio version). Bottom line, the ‘auto’ is problematic in that context. gcc link: http://gcc.gnu.org/projects/cxx-status.html (bottom of the page) |
We recently tried to change the OCIO code so that it would build against both OIIO 2.0 as well as the older 1.x versions. In doing so, we inadvertently broke OCIO for C++03. This patch still requires C++11 when building against OIIO 2.x (it has to), but is fine with C++03 if using older OIIO. (cherry picked from commit 65a08db)
|
'auto' in this case is a 'unique_ptr', so in either case, C++11 is the minimum. With the latest commit ( 65a08db ), C++11 is only required when building with OIIO_VERSION >= 10903. The default behavior would be to not build at all unless OIIO_VERSION < 10903 were being used, which is consistent with OCIO 1.1. Keeping the OIIO related commits would allow OCIO to be built against the latest OIIO optionally. The builder would have to explicitly specify a newer C++ standard on the commandline with -DCMAKE_CXX_FLAGS, but they would have the option to do so. We could update the build instructions and make this requirement clear. The alternative is to remove these commits from the potential patch release and not support a newer OIIO until OCIO v2, which is acceptable if this is deemed beyond the scope of a patch. |
|
...or we could target a 1.2 release and officially support building with a newer C++ standard, without changing library functionality. |
|
One potential solution to use OIIO newer versions for OCIO v1, would be to explicitly enforce the C++11 constraint. While keeping the current changes, one could add the following code somewhere in the ocioconvert and ociodisplay main files: |
|
I tested the added #error directive with builds against older and newer OIIO. To note, when I did try to build OCIO with OIIO 2.0 and C++03, Both the compiler and OIIO gave clear errors about the incompatibility before OCIO did. |
|
This PR will be merged in 24 hours if there is not additional feedback. |
4 participants
This PR merges the two fixes from the master branch proposed for a new 1.1.x patch.
The changes to the apps (related to OpenImageIO) add a requirement for C++11 that was not previously present, so that impact needs weighing before we decide to merge this. I have successfully built and tested this against the latest OIIO release on Linux, but can also confirm it will not build with an earlier CXX standard. One solution may be to support both paths with some CMake magic.
Commits cherry picked from:
718cc7f
1ff2274
(plus a later correction made to two ImageInput/ImageOutput mismatches)