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

Add OpenColorIO 2.3 compatibility #2294

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

Conversation

tobim
Copy link

@tobim tobim commented Oct 25, 2023

The signature of the getTexture function changed and the calling code needs to be adapted.

resolves #2284

The signature of the `getTexture` function changed and the calling
code needs to be adapted.

Signed-off-by: Tobias Mayer <tobim@fastmail.fm>
Copy link

@paulkocialkowski paulkocialkowski left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! I had the same issue on Arch and this does it :)
See a few comments on the code.

OCIO::Interpolation interpolation = OCIO::INTERP_LINEAR;

shader_desc->getTexture(i, tex_name, sampler_name, width, height, channel, interpolation);
shader_desc->getTexture(i, tex_name, sampler_name, width, height, channel,

Choose a reason for hiding this comment

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

Seems a bit out of line with the coding style to break off the line for the preprocessor conditional.
I'd rather see two instances of the call, but maybe it's just my personal taste ;)

shader_desc->getTexture(i, tex_name, sampler_name, width, height, channel, interpolation);
shader_desc->getTexture(i, tex_name, sampler_name, width, height, channel,
#if OCIO_VERSION_HEX >= 0x02030000
// OCIO::GpuShaderDesc::TextureDimensions

Choose a reason for hiding this comment

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

I don't think it's necessary to mention the type here. It's also out of line with the coding style.

@rdrpenguin04
Copy link

Could this be merged in some form? Running into the recently-closed issue while building from source.

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.

[BUILD] OpenColorIO 2.3.0 demands new argument at GetColorContext
3 participants