cuslide2: add pyramidal OME-TIFF semantics and nvImageCodec 0.9 readiness #1122
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,4 +17,4 @@ cmake_version: | |
| - ">=4.0" | ||
|
|
||
| nvimgcodec_version: | ||
| - ">=0.9.0,<0.10.0" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,22 @@ if (NOT TARGET deps::nvimgcodec) | |
| set(NVIMGCODEC_LIB_PATH "") | ||
| set(NVIMGCODEC_INCLUDE_PATH "") | ||
|
|
||
| set(NVIMGCODEC_SONAME_CANDIDATES | ||
| "libnvimgcodec.so.0.9.0" | ||
| "libnvimgcodec.so.0.8.0" | ||
|
There was a problem hiding this comment. Similar question as above. Also maybe the cmake structure can be simplified so nvimagecodec search is done in a single place? But this is a question for a future pull request
Contributor
Author
There was a problem hiding this comment. Same as on the other cmake file — On consolidating the two |
||
| "libnvimgcodec.so.0" | ||
| "libnvimgcodec.so") | ||
|
|
||
| function(_cucim_find_nvimgcodec_library out_var root_dir) | ||
| foreach(_soname IN LISTS NVIMGCODEC_SONAME_CANDIDATES) | ||
| if(EXISTS "${root_dir}/${_soname}") | ||
| set(${out_var} "${root_dir}/${_soname}" PARENT_SCOPE) | ||
| return() | ||
| endif() | ||
| endforeach() | ||
| set(${out_var} "" PARENT_SCOPE) | ||
| endfunction() | ||
|
|
||
| # Try find_package first (works in conda and system installations) | ||
| find_package(nvimgcodec QUIET) | ||
|
|
||
|
|
@@ -23,11 +39,7 @@ if (NOT TARGET deps::nvimgcodec) | |
| set(CONDA_NATIVE_ROOT "$ENV{CONDA_PREFIX}") | ||
| if(EXISTS "${CONDA_NATIVE_ROOT}/include/nvimgcodec.h") | ||
| set(NVIMGCODEC_INCLUDE_PATH "${CONDA_NATIVE_ROOT}/include/") | ||
| _cucim_find_nvimgcodec_library(NVIMGCODEC_LIB_PATH "${CONDA_NATIVE_ROOT}/lib") | ||
| endif() | ||
|
|
||
| # Fallback: try Python site-packages in conda environment | ||
|
|
@@ -36,16 +48,10 @@ if (NOT TARGET deps::nvimgcodec) | |
| set(CONDA_PYTHON_ROOT "$ENV{CONDA_PREFIX}/lib/python${PY_VER}/site-packages/nvidia/nvimgcodec") | ||
| if(EXISTS "${CONDA_PYTHON_ROOT}/include/nvimgcodec.h") | ||
| set(NVIMGCODEC_INCLUDE_PATH "${CONDA_PYTHON_ROOT}/include/") | ||
| # Check conda package layout first, then pip-style layout. | ||
| _cucim_find_nvimgcodec_library(NVIMGCODEC_LIB_PATH "${CONDA_PYTHON_ROOT}/lib") | ||
| if(NOT NVIMGCODEC_LIB_PATH) | ||
| _cucim_find_nvimgcodec_library(NVIMGCODEC_LIB_PATH "${CONDA_PYTHON_ROOT}") | ||
| endif() | ||
| if(NVIMGCODEC_LIB_PATH) | ||
| break() | ||
|
|
@@ -80,14 +86,9 @@ if (NOT TARGET deps::nvimgcodec) | |
| set(NVIMGCODEC_PYTHON_ROOT "${PYTHON_USER_SITE_PACKAGES}/nvidia/nvimgcodec") | ||
| if(EXISTS "${NVIMGCODEC_PYTHON_ROOT}/include/nvimgcodec.h") | ||
| set(NVIMGCODEC_INCLUDE_PATH "${NVIMGCODEC_PYTHON_ROOT}/include/") | ||
| _cucim_find_nvimgcodec_library(NVIMGCODEC_LIB_PATH "${NVIMGCODEC_PYTHON_ROOT}/lib") | ||
| if(NOT NVIMGCODEC_LIB_PATH) | ||
| _cucim_find_nvimgcodec_library(NVIMGCODEC_LIB_PATH "${NVIMGCODEC_PYTHON_ROOT}") | ||
| endif() | ||
| endif() | ||
| endif() | ||
|
|
@@ -97,14 +98,9 @@ if (NOT TARGET deps::nvimgcodec) | |
| set(NVIMGCODEC_PYTHON_ROOT "${PYTHON_SITE_PACKAGES}/nvidia/nvimgcodec") | ||
| if(EXISTS "${NVIMGCODEC_PYTHON_ROOT}/include/nvimgcodec.h") | ||
| set(NVIMGCODEC_INCLUDE_PATH "${NVIMGCODEC_PYTHON_ROOT}/include/") | ||
| _cucim_find_nvimgcodec_library(NVIMGCODEC_LIB_PATH "${NVIMGCODEC_PYTHON_ROOT}/lib") | ||
| if(NOT NVIMGCODEC_LIB_PATH) | ||
| _cucim_find_nvimgcodec_library(NVIMGCODEC_LIB_PATH "${NVIMGCODEC_PYTHON_ROOT}") | ||
| endif() | ||
| endif() | ||
| endif() | ||
|
|
@@ -113,14 +109,18 @@ if (NOT TARGET deps::nvimgcodec) | |
|
|
||
| # System-wide installation fallback | ||
| if(NOT NVIMGCODEC_LIB_PATH) | ||
| _cucim_find_nvimgcodec_library(NVIMGCODEC_LIB_PATH "/usr/lib/x86_64-linux-gnu") | ||
| if(NVIMGCODEC_LIB_PATH) | ||
| set(NVIMGCODEC_INCLUDE_PATH "/usr/include/") | ||
| else() | ||
| _cucim_find_nvimgcodec_library(NVIMGCODEC_LIB_PATH "/usr/lib/aarch64-linux-gnu") | ||
| endif() | ||
| if(NVIMGCODEC_LIB_PATH) | ||
| set(NVIMGCODEC_INCLUDE_PATH "/usr/include/") | ||
| else() | ||
| _cucim_find_nvimgcodec_library(NVIMGCODEC_LIB_PATH "/usr/lib64") | ||
| endif() | ||
| if(NVIMGCODEC_LIB_PATH) | ||
| set(NVIMGCODEC_INCLUDE_PATH "/usr/include/") | ||
| endif() | ||
| endif() | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the conda yaml file you set requirements >= 0.9 for nvimagecodec. Why do we need 0.8 here then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, that 0.8.0 line shouldn't be there — it's left over from when this branch still supported 0.8. Everything else already asks for 0.9:
dependencies.yaml, the conda environment files,conda_build_config.yamlandpyproject.tomlare all>=0.9.0,<0.10.0, and the vendored headers sayNVIMGCODEC_VER_MINOR 9. Dropped it from this file and the cuslide2 copy.Digging into it though, removing that line isn't quite enough. The list still ends with
libnvimgcodec.so.0andlibnvimgcodec.so, and those symlinks point at whatever major-0 build happens to be installed — so on a machine with only 0.8 we'd still pick it up without noticing. Combined with the two things you spotted elsewhere (limit_imagesgone in 0.9, no compile-time version checks), that would blow up at runtime instead of failing the build, which is a much worse way to find out.So I added a check at configure time: it reads
NVIMGCODEC_VER_MAJOR/MINORout of thenvimgcodec_version.hwe discover, errors below 0.9, warns if it's outside the pinned range, and skips quietly when the header isn't there so wheel and system installs keep working. Thefind_packagebranch now looks atnvimgcodec_VERSIONtoo — it wasn't checking anything at all before.On consolidating the two
nvimgcodec.cmakefiles: fair point, and the check I just added is now duplicated as well. I'd rather do that separately than grow this PR — the two files get picked up through differentsuperbuild_depend()search paths (cuslide2 viaSUPERBUILD_ADDITIONAL_DEPS_DIRS, the other viaCMAKE_SUPERBUILD_DEPS_ROOT_DIR, which points at the cuslide plugin's cmake dir and has no nvimgcodec entry), so merging them takes a little care not to bbreak standalone plugin builds.