api!: add image_span versions of ImageInput methods #4748
Merged
Conversation
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
ImageInput methods that read scanlines, tiles, and image are given new flavors that take image_span, much like the changes of PR 4727 did for ImageOutput. Generally, for each, there are three versions: (1) a low-level base case that takes an image_span<byte> that describes the underlying memory, and an explicit TypeDesc that specifies the data type conversion (including allowing TypeUnknown to indicate keeping channels in their native per-channel format from the file); (2) a version taking an `image_span<T>` that infers the data type conversion from `T (must be a single type for all channels); (3) a version taking a `span<T>` that further implies a contiguous buffer in all dimensions. For now, the default implementations of these new ImageInput methods are just wrappers that call the old pointer-based ones. One by one, over time, we can swap them, changing the format implementations to have a full implementation of the new bounded versions, and make their raw pointer versions call the wrappers. The raw pointer ones will be understood to be "unsafe", merely assuming that the pointers always refer to appropriately-sized memory areas. Meanwhile, the ones using spans and image_spans will, due to assertions in their implementations, make it easier to verify (at least in debug mode), that we never touch memory outside these bounds. Signed-off-by: Larry Gritz <[email protected]>
Collaborator
Author
|
We'll have time for discussion of this at the TSC meeting on Monday, but if there are no concerns there, nor voiced here by Monday or so, I will merge. This is really just applying the same set of changes we already did for ImageOutput, but here analogously to the ImageInput class. So it would be very surprising if somebody thought that one was ok but this one was not. |
jessey-git
reviewed
May 18, 2025
| ImageInput::read_image(int subimage, int miplevel, int chbegin, int chend, | ||
| TypeDesc format, const image_span<std::byte>& data) | ||
| { | ||
| #if 0 |
Contributor
There was a problem hiding this comment.
Did you mean to leave this #if 0 in?
Collaborator
Author
There was a problem hiding this comment.
Most of the new methods I added are (currently) just having the new API call the old one underneah. This is the one where I did a full implementation to make sure it was working as intended, and I wanted (for now) to have a means of switching back and forth between new and old for debugging purposes.
Contributor
|
Generally looks ok I think save for 1 thing I noticed. The |
Signed-off-by: Larry Gritz <[email protected]>
Collaborator
Author
Amended to have an extra check. |
Contributor
|
Except for the clang-format problem though, seems easy enough to address during commit though. |
Signed-off-by: Larry Gritz <[email protected]>
Collaborator
Author
|
updated with formatting fix |
488dfb2
into
AcademySoftwareFoundation:main
29 of 30 checks passed
zachlewis
pushed a commit
to zachlewis/OpenImageIO
that referenced
this pull request
Aug 1, 2025
…oundation#4748) ImageInput methods that read scanlines, tiles, and image are given new flavors that take image_span, much like the changes of PR AcademySoftwareFoundation#4727 did for ImageOutput. Generally, for each, there are three versions: (1) a low-level virtual method base case that takes an `image_span<byte>` that describes the underlying memory, and an explicit TypeDesc that specifies the data type conversion (including allowing TypeUnknown to indicate keeping channels in their native per-channel format from the file); (2) a templated version taking an `image_span<T>` that infers the data type conversion from `T` (must be a single type for all channels); (3) a templated version taking a `span<T>` that further implies a contiguous buffer in all dimensions. For now, the default implementations of these new ImageInput methods are just wrappers that call the old pointer-based ones. One by one, over time, we can swap them, changing the format implementations to have a full implementation of the new bounded versions, and make their raw pointer versions call the wrappers. The raw pointer ones will be understood to be "unsafe", merely assuming that the pointers always refer to appropriately-sized memory areas. Meanwhile, the ones using spans and image_spans will, due to assertions in their implementations, make it easier to verify (at least in debug mode), that we never touch memory outside these bounds. --------- Signed-off-by: Larry Gritz <[email protected]>
zachlewis
pushed a commit
to zachlewis/OpenImageIO
that referenced
this pull request
Sep 1, 2025
…oundation#4748) ImageInput methods that read scanlines, tiles, and image are given new flavors that take image_span, much like the changes of PR AcademySoftwareFoundation#4727 did for ImageOutput. Generally, for each, there are three versions: (1) a low-level virtual method base case that takes an `image_span<byte>` that describes the underlying memory, and an explicit TypeDesc that specifies the data type conversion (including allowing TypeUnknown to indicate keeping channels in their native per-channel format from the file); (2) a templated version taking an `image_span<T>` that infers the data type conversion from `T` (must be a single type for all channels); (3) a templated version taking a `span<T>` that further implies a contiguous buffer in all dimensions. For now, the default implementations of these new ImageInput methods are just wrappers that call the old pointer-based ones. One by one, over time, we can swap them, changing the format implementations to have a full implementation of the new bounded versions, and make their raw pointer versions call the wrappers. The raw pointer ones will be understood to be "unsafe", merely assuming that the pointers always refer to appropriately-sized memory areas. Meanwhile, the ones using spans and image_spans will, due to assertions in their implementations, make it easier to verify (at least in debug mode), that we never touch memory outside these bounds. --------- Signed-off-by: Larry Gritz <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
ImageInput methods that read scanlines, tiles, and image are given new flavors that take image_span, much like the changes of PR #4727 did for ImageOutput.
Generally, for each, there are three versions: (1) a low-level virtual method base case that takes an
image_span<byte>that describes the underlying memory, and an explicit TypeDesc that specifies the data type conversion (including allowing TypeUnknown to indicate keeping channels in their native per-channel format from the file); (2) a templated version taking animage_span<T>that infers the data type conversion fromT(must be a single type for all channels); (3) a templated version taking aspan<T>that further implies a contiguous buffer in all dimensions.For now, the default implementations of these new ImageInput methods are just wrappers that call the old pointer-based ones. One by one, over time, we can swap them, changing the format implementations to have a full implementation of the new bounded versions, and make their raw pointer versions call the wrappers. The raw pointer ones will be understood to be "unsafe", merely assuming that the pointers always refer to appropriately-sized memory areas. Meanwhile, the ones using spans and image_spans will, due to assertions in their implementations, make it easier to verify (at least in debug mode), that we never touch memory outside these bounds.