Skip to content
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

Adsk Contrib - Improve File Rules support for v1 configs #1417

Conversation

hodoulp
Copy link
Member

@hodoulp hodoulp commented Jun 19, 2021

Signed-off-by: Patrick Hodoul Patrick.Hodoul@autodesk.com

The changes ease the transition from the version 1 Config::parseColorSpaceFromString() method to the File Rules using Config:: getColorSpaceFromFilepath(). When reading version 1 config files, the in-memory config will now have two valid file rules. The first one is FilePathSearchRuleName which evaluates like the former Config::parseColorSpaceFromString(), and the second one is the DefaultRuleName. It means that the integration of the OpenColorIO v2 library can now only use the Config:: getColorSpaceFromFilepath() method for all config versions.

In order to always return a valid color space (i.e. find a color space for the DefaultRuleName rule), the algorithm is:

  1. Use the default role if it exists (i.e. that's the default implementation)
  2. Use the "raw" color space (case insensitive) if it exists & is a 'data' color space
  3. Use the first 'data' color space if one exists
  4. Use the first active color space
  5. finally, fallback to the first color space.

Two enhanced unit tests validate the v1 config read and the v1 in-memory config creation.

Note: The Config::parseColorSpaceFromString() method is now flagged as deprecated.

Please refer to #1398 for the discussion details.

Copy link
Collaborator

@michdolan michdolan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two super minor wording suggestions, but LGTM. Thanks @hodoulp !

include/OpenColorIO/OpenColorIO.h Outdated Show resolved Hide resolved
src/OpenColorIO/FileRules.cpp Outdated Show resolved Hide resolved
hodoulp and others added 12 commits June 28, 2021 11:06
Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com>
Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com>
…twareFoundation#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>
…n#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>
* 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>
* 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>
Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com>
Co-authored-by: Michael Dolan <michdolan@gmail.com>
Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com>
Co-authored-by: Michael Dolan <michdolan@gmail.com>
Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com>
…cademySoftwareFoundation#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>
…ademySoftwareFoundation#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>
Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com>
@hodoulp hodoulp force-pushed the adsk_contrib/improve_v1_support branch from 218d78a to 0b941b3 Compare June 28, 2021 15:06
@hodoulp
Copy link
Member Author

hodoulp commented Jul 5, 2021

Friendly reminder that the pull request can now be merged.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 5, 2021

CLA Signed

The committers are authorized under a signed CLA.

@doug-walker doug-walker merged commit 76ddf44 into AcademySoftwareFoundation:master Jul 6, 2021
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants