Skip to content

api(ImageBuf): IB::localpixels_as_[writable_]byte_image_span - #5011

Merged
lgritz merged 2 commits into
AcademySoftwareFoundation:mainfrom
lgritz:lg-bufspan
Jan 28, 2026
Merged

api(ImageBuf): IB::localpixels_as_[writable_]byte_image_span#5011
lgritz merged 2 commits into
AcademySoftwareFoundation:mainfrom
lgritz:lg-bufspan

Conversation

@lgritz

@lgritz lgritz commented Jan 18, 2026

Copy link
Copy Markdown
Collaborator

Utilities to ask the IB for the local pixels as an untyped span of bytes. This is a "safe" alternative to localpixels(), which just returned a single pointer, instead returning an image_span that understands the sizes and strides of the buffer.

Utilities to ask the IB for the local pixels as an untyped span
of bytes. This is a "safe" alternative to `localpixels()`, which
just returned a single pointer, instead returning an image_span
that understands the sizes and strides of the buffer.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
@lgritz

lgritz commented Jan 19, 2026

Copy link
Copy Markdown
Collaborator Author

Note: the bleeding edge test failure is not from this PR, it's due to a new commit in main of pybind11 that is causing things to crash. I have reported it.

@lgritz

lgritz commented Jan 23, 2026

Copy link
Copy Markdown
Collaborator Author

Any comments?

Comment on lines +1372 to +1374
/// Return an `image_span<const std::byte>` giving the extent and layout
/// of "local" pixel memory, if they are fully in RAM and not backed by an
/// ImageCache, or an empty span otherwise.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[consider] It may be clearer to indicate that the return of this can be inferred from ImageBuf::storage. So for example:

Return an .. if ImageBuf::storage() is Storage::MEMORY`, otherwise ...

Apokogies if I botched that ayntax but hopefully the intention comes across

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think I'm going to keep it as-is, because the storage test is actually a little more complicated than that. There are multiple storage() types that have addressable pixels, and multiple (at some point in the future, if not now) that do not. So I think keeping vague "if they are fully in RAM" is the distinction I'm trying to make.

@lgritz
lgritz requested a review from Copilot January 28, 2026 12:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds new utility methods to the ImageBuf class that provide safer access to local pixel data by returning image_span objects instead of raw pointers. This allows callers to work with pixel memory while maintaining information about buffer sizes and strides.

Changes:

  • Added two new methods to return local pixels as byte image spans: one for read-only access and one for writable access
  • The writable variant includes a check to return an empty span for read-only ImageBuf instances

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/libOpenImageIO/imagebuf.cpp Implementation of the two new methods that return image spans wrapping the internal buffer
src/include/OpenImageIO/imagebuf.h Public API declarations with documentation for the new localpixels_as_byte_image_span methods

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/libOpenImageIO/imagebuf.cpp Outdated


image_span<std::byte>
ImageBuf::localpixels_as_writable_byte_image_span() const

Copilot AI Jan 28, 2026

Copy link

Choose a reason for hiding this comment

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

The method is declared as const but returns a writable image_span<std::byte>, which allows modification of the underlying buffer. This violates const-correctness. Consider removing the const qualifier from this method, as it provides mutable access to the ImageBuf's data.

Suggested change
ImageBuf::localpixels_as_writable_byte_image_span() const
ImageBuf::localpixels_as_writable_byte_image_span()

Copilot uses AI. Check for mistakes.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
@lgritz
lgritz merged commit 4f08ac0 into AcademySoftwareFoundation:main Jan 28, 2026
29 of 30 checks passed
@lgritz
lgritz deleted the lg-bufspan branch January 28, 2026 22:10
lgritz added a commit to lgritz/OpenImageIO that referenced this pull request Jan 30, 2026
…SoftwareFoundation#5011)

Utilities to ask the IB for the local pixels as an untyped span of
bytes. This is a "safe" alternative to `localpixels()`, which just
returned a single pointer, instead returning an image_span that
understands the sizes and strides of the buffer.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Feb 24, 2026
…SoftwareFoundation#5011)

Utilities to ask the IB for the local pixels as an untyped span of
bytes. This is a "safe" alternative to `localpixels()`, which just
returned a single pointer, instead returning an image_span that
understands the sizes and strides of the buffer.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 1, 2026
…SoftwareFoundation#5011)

Utilities to ask the IB for the local pixels as an untyped span of
bytes. This is a "safe" alternative to `localpixels()`, which just
returned a single pointer, instead returning an image_span that
understands the sizes and strides of the buffer.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad <shaamaan@gmail.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 2, 2026
…SoftwareFoundation#5011)

Utilities to ask the IB for the local pixels as an untyped span of
bytes. This is a "safe" alternative to `localpixels()`, which just
returned a single pointer, instead returning an image_span that
understands the sizes and strides of the buffer.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 2, 2026
…SoftwareFoundation#5011)

Utilities to ask the IB for the local pixels as an untyped span of
bytes. This is a "safe" alternative to `localpixels()`, which just
returned a single pointer, instead returning an image_span that
understands the sizes and strides of the buffer.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 2, 2026
…SoftwareFoundation#5011)

Utilities to ask the IB for the local pixels as an untyped span of
bytes. This is a "safe" alternative to `localpixels()`, which just
returned a single pointer, instead returning an image_span that
understands the sizes and strides of the buffer.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
Signed-off-by: Vlad <shaamaan@gmail.com>
ssh4net pushed a commit to ssh4net/OpenImageIO that referenced this pull request Apr 2, 2026
…SoftwareFoundation#5011)

Utilities to ask the IB for the local pixels as an untyped span of
bytes. This is a "safe" alternative to `localpixels()`, which just
returned a single pointer, instead returning an image_span that
understands the sizes and strides of the buffer.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
Signed-off-by: Vlad <shaamaan@gmail.com>
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.

3 participants