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

Alpha channel should be 1 by default, rather than 0 #2033

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pennelee
Copy link
Contributor

@pennelee pennelee commented Sep 2, 2024

Related to issue #1781 .
#1781

For CPU processor, when source has no alpha (RGB) but dest does (RGBA) make it default to 1.
GPU Processors (new and legacy) did not seem to have this issue, alpha was already set to 1. Let me know if this is not the case.

Tested changes with ocioconvert and changed the output to force RGBA, since the default is to use inplace conversion (did not check in those changes).
Tested with all the ocioconvert test cases (basic, --lut, --view, --invertview --namedtransform, --invnamed transform), and various EXR file sizes.

Before the changes alpha would be 0, so image was not viewable. After the changes then images can be viewed and from inspection the alpha channel is now 1.0.

@pennelee
Copy link
Contributor Author

pennelee commented Sep 2, 2024

Hello @doug-walker ,
It looks like there were some CI failures and timeouts, but I'm not clear if they are related to this change I made. Do you have any suggestion on how to resolve the CI issues? Thank you!

@KelSolaar
Copy link
Contributor

@pennelee : The tests are failing here: https://github.com/AcademySoftwareFoundation/OpenColorIO/actions/runs/10661264522/job/29546503311?pr=2033#step:9:8497, the workflow file does not set continue-on-error so it will kill all the jobs.

@doug-walker
Copy link
Collaborator

@pennelee , yes I would definitely expect that you will need to update the unit tests based on your changes. You may even want to add a new unit test, if the existing ones don't cover this behavior sufficiently well.

Were you able to run the tests locally? The Contributor Guide has info about running the tests (ctest -V).

@pennelee
Copy link
Contributor Author

pennelee commented Sep 3, 2024

Thanks @KelSolaar , @doug-walker , I will run the test manually and see what needs to be changed and/or added.

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.

3 participants