Use CMake visibility flags#1411
Merged
hodoulp merged 5 commits intoAcademySoftwareFoundation:masterfrom Jun 15, 2021
Merged
Conversation
Signed-off-by: Rémi Achard <remiachard@gmail.com>
Signed-off-by: Rémi Achard <remiachard@gmail.com>
lgritz
reviewed
Jun 13, 2021
lgritz
approved these changes
Jun 13, 2021
Collaborator
lgritz
left a comment
There was a problem hiding this comment.
This is a good improvement that (along with the related PR I made) aligns OCIO with best practices for symbol hiding. Please do also backport this to RB-2.0! Many of us would be relieved to get a 2.0.2 that does all the symbol hiding properly.
Collaborator
|
I don't know the right solution for macos, but I would certainly like to find out. What I tried in the other PR works for Linux, but that linker option just didn't exist on Mac. |
michdolan
approved these changes
Jun 14, 2021
Signed-off-by: Rémi Achard <remiachard@gmail.com>
Signed-off-by: Rémi Achard <remiachard@gmail.com>
hodoulp
approved these changes
Jun 15, 2021
hodoulp
added a commit
to autodesk-forks/OpenColorIO
that referenced
this pull request
Jun 28, 2021
* Update install doc minimal version numbers Signed-off-by: Rémi Achard <remiachard@gmail.com> * Use CMake visibility presets variables Signed-off-by: Rémi Achard <remiachard@gmail.com> * Fix python package case Signed-off-by: Rémi Achard <remiachard@gmail.com> * Allow advanced user to override symbol visibility Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com>
hodoulp
added a commit
that referenced
this pull request
Jun 28, 2021
* Update install doc minimal version numbers Signed-off-by: Rémi Achard <remiachard@gmail.com> * Use CMake visibility presets variables Signed-off-by: Rémi Achard <remiachard@gmail.com> * Fix python package case Signed-off-by: Rémi Achard <remiachard@gmail.com> * Allow advanced user to override symbol visibility Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com>
doug-walker
added a commit
that referenced
this pull request
Jul 6, 2021
* Adsk Contrib - Improve file rules for v1 configs Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Emergency GPU build fix (#1391) Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Fix some bugs found by Maya and SonarCloud (#1403) * Adsk Contrib - Fix some bugs found by Maya and SonarCloud Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix a Windows warning Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * After Effects and Photoshop plug-in updates (#1373) * Set Mac OS deployment target to 10.10 Signed-off-by: Brendan Bolles <brendan@fnordware.com> * More 10.10 targets Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Ditch ADD_EXTRA_BUILTINS Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Getting strange callback when AE effect is pasted Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Use NDEBUG in AE project Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Disable radio buttons with no config Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Update version Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Move to AE 2021 SDK, set PF_OutFlag2_MUTABLE_RENDER_SEQUENCE_DATA_SLOWER Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Define OCIO_DEPRECATED in After Effects builds Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Verify change from color space menu Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Handle situation where color space has a '/' in it Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Windows family separator fix Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Invert everything Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Invert everything Windows Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Disable more controls Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Incorporate invert into LUT export Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Get rid of fullPaths stuff Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Fix changed config settings retention Signed-off-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Add OpenColorIOConfig.cmake generation (#1397) * Initial OpenColorIO Config CMake implementation Signed-off-by: Rémi Achard <remiachard@gmail.com> * Remove macro, improve documentation Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Use CMake visibility flags (#1411) * Update install doc minimal version numbers Signed-off-by: Rémi Achard <remiachard@gmail.com> * Use CMake visibility presets variables Signed-off-by: Rémi Achard <remiachard@gmail.com> * Fix python package case Signed-off-by: Rémi Achard <remiachard@gmail.com> * Allow advanced user to override symbol visibility Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix Windows build Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Update include/OpenColorIO/OpenColorIO.h Co-authored-by: Michael Dolan <michdolan@gmail.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Update src/OpenColorIO/FileRules.cpp Co-authored-by: Michael Dolan <michdolan@gmail.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Hide dependency symbol visibility (#1409) (#1416) When creating libOpenColorIO.so, we lacked the linker commands that hide symbol visibility from the dependent libraries, which is necessary to prevent OCIO from exporting the symbols from Expat and the other dependencies that OCIO needs to use internally but is not trying intentionally to expose via its API. Failing to do this can result in symbol clashes and all sorts of subtle errors if OCIO is used in an app that also uses and is linked against a potentially different version of Expat (or any of the other deps). Signed-off-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Improve DX11 & Cg fragment shader language support (#1406) * Adsk Contrib - Improve DX11 support Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Improve Cg support Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Improve GPU comments Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix unit test failures Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> Co-authored-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Michael Dolan <michdolan@gmail.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: doug-walker <43830961+doug-walker@users.noreply.github.com>
doug-walker
added a commit
that referenced
this pull request
Jul 13, 2021
* Update install doc minimal version numbers Signed-off-by: Rémi Achard <remiachard@gmail.com> * Use CMake visibility presets variables Signed-off-by: Rémi Achard <remiachard@gmail.com> * Fix python package case Signed-off-by: Rémi Achard <remiachard@gmail.com> * Allow advanced user to override symbol visibility Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Co-authored-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: doug-walker <43830961+doug-walker@users.noreply.github.com>
hodoulp
added a commit
that referenced
this pull request
Aug 2, 2021
* Adsk Contrib - Improve file rules for v1 configs Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Emergency GPU build fix (#1391) Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Fix some bugs found by Maya and SonarCloud (#1403) * Adsk Contrib - Fix some bugs found by Maya and SonarCloud Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix a Windows warning Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * After Effects and Photoshop plug-in updates (#1373) * Set Mac OS deployment target to 10.10 Signed-off-by: Brendan Bolles <brendan@fnordware.com> * More 10.10 targets Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Ditch ADD_EXTRA_BUILTINS Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Getting strange callback when AE effect is pasted Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Use NDEBUG in AE project Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Disable radio buttons with no config Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Update version Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Move to AE 2021 SDK, set PF_OutFlag2_MUTABLE_RENDER_SEQUENCE_DATA_SLOWER Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Define OCIO_DEPRECATED in After Effects builds Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Verify change from color space menu Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Handle situation where color space has a '/' in it Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Windows family separator fix Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Invert everything Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Invert everything Windows Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Disable more controls Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Incorporate invert into LUT export Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Get rid of fullPaths stuff Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Fix changed config settings retention Signed-off-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Add OpenColorIOConfig.cmake generation (#1397) * Initial OpenColorIO Config CMake implementation Signed-off-by: Rémi Achard <remiachard@gmail.com> * Remove macro, improve documentation Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Use CMake visibility flags (#1411) * Update install doc minimal version numbers Signed-off-by: Rémi Achard <remiachard@gmail.com> * Use CMake visibility presets variables Signed-off-by: Rémi Achard <remiachard@gmail.com> * Fix python package case Signed-off-by: Rémi Achard <remiachard@gmail.com> * Allow advanced user to override symbol visibility Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix Windows build Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Update include/OpenColorIO/OpenColorIO.h Co-authored-by: Michael Dolan <michdolan@gmail.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Update src/OpenColorIO/FileRules.cpp Co-authored-by: Michael Dolan <michdolan@gmail.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Hide dependency symbol visibility (#1409) (#1416) When creating libOpenColorIO.so, we lacked the linker commands that hide symbol visibility from the dependent libraries, which is necessary to prevent OCIO from exporting the symbols from Expat and the other dependencies that OCIO needs to use internally but is not trying intentionally to expose via its API. Failing to do this can result in symbol clashes and all sorts of subtle errors if OCIO is used in an app that also uses and is linked against a potentially different version of Expat (or any of the other deps). Signed-off-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Improve DX11 & Cg fragment shader language support (#1406) * Adsk Contrib - Improve DX11 support Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Improve Cg support Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Improve GPU comments Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix unit test failures Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> Co-authored-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Michael Dolan <michdolan@gmail.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: doug-walker <43830961+doug-walker@users.noreply.github.com>
hodoulp
added a commit
that referenced
this pull request
Aug 12, 2021
…t for v1 (#1445) * Adsk Contrib - Improve File Rules support for v1 configs (#1417) * Adsk Contrib - Improve file rules for v1 configs Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Emergency GPU build fix (#1391) Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Fix some bugs found by Maya and SonarCloud (#1403) * Adsk Contrib - Fix some bugs found by Maya and SonarCloud Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix a Windows warning Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * After Effects and Photoshop plug-in updates (#1373) * Set Mac OS deployment target to 10.10 Signed-off-by: Brendan Bolles <brendan@fnordware.com> * More 10.10 targets Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Ditch ADD_EXTRA_BUILTINS Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Getting strange callback when AE effect is pasted Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Use NDEBUG in AE project Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Disable radio buttons with no config Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Update version Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Move to AE 2021 SDK, set PF_OutFlag2_MUTABLE_RENDER_SEQUENCE_DATA_SLOWER Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Define OCIO_DEPRECATED in After Effects builds Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Verify change from color space menu Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Handle situation where color space has a '/' in it Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Windows family separator fix Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Invert everything Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Invert everything Windows Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Disable more controls Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Incorporate invert into LUT export Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Get rid of fullPaths stuff Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Fix changed config settings retention Signed-off-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Add OpenColorIOConfig.cmake generation (#1397) * Initial OpenColorIO Config CMake implementation Signed-off-by: Rémi Achard <remiachard@gmail.com> * Remove macro, improve documentation Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Use CMake visibility flags (#1411) * Update install doc minimal version numbers Signed-off-by: Rémi Achard <remiachard@gmail.com> * Use CMake visibility presets variables Signed-off-by: Rémi Achard <remiachard@gmail.com> * Fix python package case Signed-off-by: Rémi Achard <remiachard@gmail.com> * Allow advanced user to override symbol visibility Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix Windows build Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Update include/OpenColorIO/OpenColorIO.h Co-authored-by: Michael Dolan <michdolan@gmail.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Update src/OpenColorIO/FileRules.cpp Co-authored-by: Michael Dolan <michdolan@gmail.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Hide dependency symbol visibility (#1409) (#1416) When creating libOpenColorIO.so, we lacked the linker commands that hide symbol visibility from the dependent libraries, which is necessary to prevent OCIO from exporting the symbols from Expat and the other dependencies that OCIO needs to use internally but is not trying intentionally to expose via its API. Failing to do this can result in symbol clashes and all sorts of subtle errors if OCIO is used in an app that also uses and is linked against a potentially different version of Expat (or any of the other deps). Signed-off-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Improve DX11 & Cg fragment shader language support (#1406) * Adsk Contrib - Improve DX11 support Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Improve Cg support Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Improve GPU comments Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix unit test failures Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> Co-authored-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Michael Dolan <michdolan@gmail.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: doug-walker <43830961+doug-walker@users.noreply.github.com> * Add missing file change Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> Co-authored-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Michael Dolan <michdolan@gmail.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: doug-walker <43830961+doug-walker@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following recent discussions around symbol visibility, we noticed that OpenColorIO only seem to use
-fvisibility-inlines-hiddenbut not-fvisibility=hiddenon the core library. This PR add it and replace the hard coded compiler flags with CMake utility variables to enable that behaviour by default overall.Here is some tests I did on macOS to illustrate the impact:
Notice that Expat symbols are still leaked, I don't know yet how to fix that on macOS.