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

PR: Implement Support for Remaining DisplayP3 Colourspaces, and Other Next Release Features. #100

Merged

Conversation

KelSolaar
Copy link
Contributor

@KelSolaar KelSolaar commented Jun 18, 2023

This PR implements support for the remaining DisplayP3 colourspaces and many other features.

Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
@KelSolaar
Copy link
Contributor Author

@doug-walker: I opened a PR (ampas/aces-dev#136) to add support for DisplayP3 on the aces-dev side and updated the spreadsheets accordingly. We are missing a Builtin on the OCIO side which is alright but it begs the question as to whether the Builtin should be in https://github.com/AcademySoftwareFoundation/OpenColorIO/blob/main/src/OpenColorIO/transforms/builtins/ACES.cpp or not given the PR is not merged at all, and might not be in time.

@KelSolaar KelSolaar changed the title PR: Implement Support for Remaining DisplayP3 Colourspaces PR: Implement Support for Remaining DisplayP3 Colourspaces, and Other Next Release Features. Jul 26, 2023
@KelSolaar
Copy link
Contributor Author

PR was merged, we can move on! A few notes:

  • We will use this PR for most of the new release features as it is simple.
  • Because the next release of OCIO and the future Pypi wheels depend on the config generated by this PR but this PR cannot require non-existing wheels, we have decided to manually build the configs for the next release of OCIO rather than doing it on CI. This will be done once the wheels are available.

Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
@KelSolaar KelSolaar force-pushed the feature/displayp3 branch 2 times, most recently from d900e5c to 12eabed Compare July 28, 2023 10:27
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
@KelSolaar KelSolaar marked this pull request as ready for review August 12, 2023 02:31
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
@KelSolaar KelSolaar force-pushed the feature/displayp3 branch 4 times, most recently from 011234a to f05b3b8 Compare August 19, 2023 21:53
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
@KelSolaar KelSolaar force-pushed the feature/displayp3 branch 4 times, most recently from 69f8d8d to 4c1dd57 Compare September 2, 2023 08:49
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
@KelSolaar
Copy link
Contributor Author

Ok this is ready, I verified that the artefacts generated are producing the same files than the builtin configs in 2.3!

@michdolan and @doug-walker for VIS!

Signed-off-by: Thomas Mansencal <thomas.mansencal@gmail.com>
Copy link
Contributor

@doug-walker doug-walker left a comment

Choose a reason for hiding this comment

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

LGTM, thank you Thomas!

@@ -3,7 +3,7 @@
<InputDescriptor>ARRI LogC4</InputDescriptor>
<OutputDescriptor>ACES2065-1</OutputDescriptor>
<Info>
<ACEStransformID>urn:ampas:aces:transformId:v1.5:IDT.ARRI.LogC4.a1.v1</ACEStransformID>
<ACEStransformID>urn:ampas:aces:transformId:v1.5:IDT.ARRI.ARRI-LogC4.a1.v1</ACEStransformID>
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that we are modifying the ID submitted by Sean of ARRI, should we do anything to indicate this was to make it agree with what is in aces-dev? Should we get Sean to approve it? Or maybe it's sufficiently obvious that it's just a bug fix and no comment is necessary. Either way is fine with me.

Copy link
Contributor Author

@KelSolaar KelSolaar Sep 12, 2023

Choose a reason for hiding this comment

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

Will add a comment, thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add a comment in the release notes as it gives better visibility.

@KelSolaar KelSolaar merged commit 10a851a into AcademySoftwareFoundation:main Sep 16, 2023
9 checks passed
@KelSolaar KelSolaar deleted the feature/displayp3 branch September 16, 2023 21:05
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.

None yet

2 participants