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

Update OpenImageIO 2.5.7.0, Imath 3.1.11 and OpenEXR 3.2.3 to latest versions on Windows. #944

Closed
wants to merge 10 commits into from

Conversation

TodicaIonut
Copy link
Contributor

Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. Additionally, make sure you've done all of these things:

PR Description

What type of PR is this? (Check one of the boxes below)

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (non-breaking change which does not add functionality nor fixes a bug but improves Natron in some way)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • My change requires a change to the documentation
    • I have updated the documentation accordingly

What does this pull request do?

Upgrade to OpenImageIO 2.5

Have you tested your changes (if applicable)? If so, how?

Created a Windows release build with these new library versions and it appears to work fine.

Copy link
Collaborator

@acolwell acolwell left a comment

Choose a reason for hiding this comment

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

This is causing the "Build pacman repo" action to fail. It looks to be caused by the oiio-2.4.13.0-thread-shutdown.patch. You should be able to remove this patch since you are updating to 2.5.7, but you need to make sure that code in the openfx-io plugin still calls default_thread_pool_shutdown() when the plugin is unloaded. You can take a look at NatronGitHub/openfx-io@b194c3c to see the relevant code in openfx-io.

@TodicaIonut TodicaIonut changed the title Update OpenImageIO 2.5.7.0 to latest versions on Windows. Update OpenImageIO 2.5.7.0 and OpenEXR 3.2.2 to latest versions on Windows. Feb 13, 2024
@TodicaIonut TodicaIonut changed the title Update OpenImageIO 2.5.7.0 and OpenEXR 3.2.2 to latest versions on Windows. Update OpenImageIO 2.5.7.0 and OpenEXR 3.2.3 to latest versions on Windows. Feb 14, 2024
@TodicaIonut
Copy link
Contributor Author

OpenEXR 3.2.3 (February 21, 2024)

@TodicaIonut TodicaIonut marked this pull request as draft February 14, 2024 14:31
@TodicaIonut TodicaIonut requested a review from acolwell February 14, 2024 16:38
Copy link
Collaborator

@acolwell acolwell left a comment

Choose a reason for hiding this comment

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

oiio-2.4.13.0-thread-shutdown.patch should be removed since it isn't being used anymore.

I do have a few questions though:
What is the motivation for this change?
Why did you pick 2.5.7 instead of the latest 2.5.8 release?
What testing have you done to verify that this has not broken anything?
What have you done to explicitly verify that the thread shutdown logic is still being called?
Why did you change the title to OpenEXR 3.2.3 when that release doesn't even exist yet and isn't being used here?

I must admit I have concerns about updating this non-trivial dependency without making sure there is extensive and thorough testing of many of the file types covered by this library. I do want to get us on a 2.5.x version, but we need to be careful and deliberate when doing so in my opinion.

@TodicaIonut TodicaIonut marked this pull request as ready for review March 7, 2024 16:15
@TodicaIonut TodicaIonut requested a review from acolwell March 7, 2024 16:25
@acolwell
Copy link
Collaborator

acolwell commented Mar 7, 2024

Please answer my questions. I am unlikely to approve this until you do.

@TodicaIonut
Copy link
Contributor Author

Update:

  • OpenImageIO 2.5.7.0
  • Imath 3.1.11
  • OpenEXR 3.2.3

Removed:

  • mingw-w64-openexr25 is no longer available
  • oiio-2.4.13.0-thread-shutdown.patch be removed

going to merge

@TodicaIonut TodicaIonut changed the title Update OpenImageIO 2.5.7.0 and OpenEXR 3.2.3 to latest versions on Windows. Update OpenImageIO 2.5.7.0, Imath 3.1.11 and OpenEXR 3.2.3 to latest versions on Windows. Mar 8, 2024
@@ -33,22 +33,19 @@ source=(${_realname}-${pkgver}.tar.gz::https://github.com/OpenImageIO/oiio/archi
oiio-2.1.10-boost.diff
oiio-2.0.8-invalidatespec.patch
oiio-2.2.14-libraw.diff
oiio-2.4.13.0-thread-shutdown.patch # Remove when updating to a future 2.5 release that has these changes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks to be caused by the oiio-2.4.13.0-thread-shutdown.patch. You should be able to remove this patch since you are updating to 2.5.7, but you need to make sure that code in the openfx-io plugin still calls default_thread_pool_shutdown() when the plugin is unloaded.

done updating.

@TodicaIonut TodicaIonut deleted the patch-2 branch March 11, 2024 09:25
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.

2 participants