Conversation
SimonRit
left a comment
There was a problem hiding this comment.
Thanks. There seems to be issue reported by the CI, can you address them? E.g., https://github.com/RTKConsortium/ITKCudaCommon/pull/89/checks#step:5:731. Regarding your question, yes, I think it can be wrapped for all types wrapped for itk::CudaImage. We could possibly implement a macro to simplify this a bit.
5bf90a4 to
5c4e247
Compare
0fe4158 to
0ea19ae
Compare
f35b97e to
e03c858
Compare
|
I covered most of the possible combinations of types, but I couldn't copy the same types as the base itk filters because there is a lot of conditional wrapping, I prefered keep it simple. |
SimonRit
left a comment
There was a problem hiding this comment.
The PR requires some clarifications in my opinion
| unique(to_types "D;UC;UL;${ITKM_IT};${WRAP_ITK_SCALAR}") | ||
| UNIQUE(vector_types "${WRAP_ITK_VECTOR_REAL};${WRAP_ITK_COV_VECTOR_REAL}") | ||
|
|
||
| cuda_wrap_image_filter_combinations("${from_types}" "${to_types}" "" "") |
There was a problem hiding this comment.
Is this necessary? Which filter requires no parent? I think it's better to avoid this parent option and to make it mandatory
| cuda_wrap_image_filter_combinations("${from_types}" "${to_types}" "" "") | ||
| cuda_wrap_image_filter_combinations("${from_types}" "${to_types}" "IP" "itk::InPlaceImageFilter") | ||
|
|
||
| cuda_wrap_vector_combinations("${vector_types}" "" "") |
| unique(to_types "D;UC;UL;${ITKM_IT};${WRAP_ITK_SCALAR}") | ||
| UNIQUE(vector_types "${WRAP_ITK_VECTOR_REAL};${WRAP_ITK_COV_VECTOR_REAL}") | ||
|
|
||
| cuda_wrap_image_filter_combinations("${from_types}" "${to_types}" "IP") |
| cuda_wrap_image_filter_combinations("${from_types}" "${to_types}" "IP") | ||
| cuda_wrap_image_filter_combinations("${from_types}" "${to_types}" "IP" "itk::InPlaceImageFilter") | ||
|
|
||
| cuda_wrap_vector_combinations("${vector_types}" "IP") |
| foreach(d ${ITK_WRAP_IMAGE_DIMS}) | ||
| foreach(t ${types}) | ||
| itk_wrap_template("CI${ITKM_${t}}${d}" "itk::CudaImage<${ITKT_${t}}, ${d}>") | ||
| itk_wrap_template("I${ITKM_${t}}${d}" "itk::Image<${ITKT_${t}}, ${d}>") |
There was a problem hiding this comment.
Isn't it already wrapped by ITK?
| foreach(d ${ITK_WRAP_IMAGE_DIMS}) | ||
| foreach(vt ${vector_types}) | ||
| itk_wrap_template("CI${ITKM_${vt}${c}}${d}" "itk::CudaImage<${ITKT_${vt}${c}}, ${d}>") | ||
| itk_wrap_template("I${ITKM_${vt}${c}}${d}" "itk::Image<${ITKT_${vt}${c}}, ${d}>") |
| macro(cuda_wrap_image_filter_combinations from_types to_types prefix) | ||
| set(parent ${ARGN}) | ||
| foreach(d ${ITK_WRAP_IMAGE_DIMS}) | ||
| foreach(from ${from_types}) |
There was a problem hiding this comment.
Do we really combinations from different types as input and output?
Close #69
I am not sure if wrapping all those types is really necessary.