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 libtiff v4.4.0 #4953

Open
blowekamp opened this issue Nov 15, 2024 · 16 comments
Open

Update libtiff v4.4.0 #4953

blowekamp opened this issue Nov 15, 2024 · 16 comments
Assignees
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances

Comments

@blowekamp
Copy link
Member

The third-party libtiff is rather old and out dated.

@blowekamp blowekamp added the type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances label Nov 15, 2024
@blowekamp
Copy link
Member Author

@dzenanz The current ITK/Modules/ThirdParty/TIFF does not have a sub-tree. How does one initialize using sub-trees and the UpdateFromUpstream infrastructure?

@dzenanz
Copy link
Member

dzenanz commented Nov 15, 2024

For vanilla initializing subtrees: add a remote of the library you want to add (libtiff), fetch, then merge with main repository.

For starting a new subtree using UpdateFromUpstream infrastructure: I don't know, I have never done that. I have only made changes to existing third-party libaries and their update scripts.

@bradking should be able to advise, or maybe even get it started for us?

@blowekamp
Copy link
Member Author

For reference this was the last update to the library:
54c790a

There are a lot of upstream changes, and not many local changes. So starting with a fresh upstream is reasonable IMHO.

@bradking
Copy link
Member

I'll take a look at importing a new tiff.

@dzenanz
Copy link
Member

dzenanz commented Nov 15, 2024

Linking some related and possibly related issues:
#4777
#2959
#4102

@bradking
Copy link
Member

As a first step, #4957 rebuilds the existing tiff 4.0.3 import with our modern history structure. That will make the update to a newer tiff version much easier.

@hjmjohnson
Copy link
Member

FYI: Initial 10-minute attempt to update to v4.7.0. I don't have time to continue working on this at the moment.

Some file names have changed, so we need to update the updater script.

0001-ENH-Update-to-latest-v4.7.0-libtiff-codebase.patch

@blowekamp
Copy link
Member Author

FYI: Initial 10-minute attempt to update to v4.7.0. I don't have time to continue working on this at the moment.

Some file names have changed, so we need to update the updater script.

0001-ENH-Update-to-latest-v4.7.0-libtiff-codebase.patch

Yes, lots of changes have happened in the last 10 year. They also add CMake instrastructure. I had not had the change to determine if it was usable or not yet.

@bradking
Copy link
Member

For reference, VTK has imported tiff 4.6.0 and uses the upstream's CMake code with some patches.

@blowekamp blowekamp self-assigned this Nov 18, 2024
@blowekamp
Copy link
Member Author

These look like the changes VTK made:
https://gitlab.kitware.com/third-party/tiff/-/compare/v4.6.0...for%2Fvtk-20231025-4.6.0?page=2&straight=true

It appears VTK make their third party changes in a separate repository, then form the sub-tree via a script which is then merged into the main-line of the VTK repo.

@bradking
Copy link
Member

Yes, VTK's approach has the advantage that the changes from upstream are recorded independently from VTK. It has the disadvantage that an extra repository is needed with VTK-specific tags.

With the tiff 4.0.3 re-import done, I'll move on to an actual update in ITK.

@blowekamp
Copy link
Member Author

Yes, VTK's approach has the advantage that the changes from upstream are recorded independently from VTK. It has the disadvantage that an extra repository is needed with VTK-specific tags.

With the tiff 4.0.3 re-import done, I'll move on to an actual update in ITK.

Awesome, I just started to make changes in to libtiff library to get it to configure, but I'll leave it to you!

@bradking
Copy link
Member

@blowekamp I don't know how soon I'll be able to get to it, so if you've made progress, go ahead and continue. I can help structure the history once you have something working.

@blowekamp
Copy link
Member Author

After merge of #4961, there are some build issues when DCMTK in enabled that I am investigating.

@blowekamp
Copy link
Member Author

@bradking It looks like there are some library properties that are not being forwarded to the DCMTK External project:
https://github.com/InsightSoftwareConsortium/ITK/blob/master/Modules/ThirdParty/TIFF/src/itktiff/libtiff/CMakeLists.txt#L125

Should these target properties be replaced in ITK's 3P module CMake code, or the DCMTK configuration be updated to extract these target properties and forward to DCMTK configuration.

https://github.com/InsightSoftwareConsortium/ITK/blob/master/Modules/ThirdParty/DCMTK/CMakeLists.txt#L218

@bradking
Copy link
Member

Modules/ThirdParty/TIFF/CMakeLists.txt's setting of ITKTIFF_INCLUDE_DIRS may need to be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances
Projects
None yet
Development

No branches or pull requests

4 participants