Skip to content

Fix image#669

Closed
paul-nechifor wants to merge 17 commits intodevfrom
fix-iage
Closed

Fix image#669
paul-nechifor wants to merge 17 commits intodevfrom
fix-iage

Conversation

@paul-nechifor
Copy link
Contributor

No description provided.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Comment on lines +263 to +273
def to_cpu(self) -> "Image":
if isinstance(self._impl, NumpyImage):
return self.copy()
return Image(
NumpyImage(
np.asarray(self._impl.to_opencv()),
self._impl.format,
self._impl.frame_id,
self._impl.ts,
)
)

Choose a reason for hiding this comment

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

P1 Badge Preserve alpha channels when moving CUDA images to CPU

The new to_cpu path copies GPU-backed images by calling self._impl.to_opencv() and wraps the result in a NumpyImage while keeping the original format. For BGRA/RGBA inputs, CudaImage.to_opencv() converts to BGR (drops the alpha channel), yet the returned NumpyImage still advertises BGRA/RGBA. Any later call that assumes four channels (e.g. to_opencv() or to_bgr()) will now fail or use corrupted data because the array only has three channels. This regression did not exist before, when the class carried the raw numpy array unchanged. to_cpu should copy the underlying data without forcing a BGR conversion so that channel counts remain consistent with format.

Useful? React with 👍 / 👎.

Copy link
Contributor

Choose a reason for hiding this comment

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

@paul-nechifor is this useful

Copy link
Contributor

Choose a reason for hiding this comment

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

@spomichter this is a helper function. It can be, if you'd like to have it included.

Comment on lines 117 to +123
@classmethod
def from_numpy(
cls, np_image: np.ndarray, format: ImageFormat = ImageFormat.BGR, **kwargs
cls,
np_image: np.ndarray,
format: ImageFormat = ImageFormat.RGB,
to_cuda: bool = False,
**kwargs,

Choose a reason for hiding this comment

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

P1 Badge from_numpy now defaults to RGB, breaking OpenCV/BGR callers

The helper now defaults format to ImageFormat.RGB instead of ImageFormat.BGR. Existing code that relied on the previous default (and passed OpenCV images without an explicit format) will silently interpret BGR data as RGB, producing swapped color channels throughout the pipeline. Nothing in the commit signals a deliberate API change, and the surrounding codebase and tests historically assumed BGR as the implicit format. The default should remain BGR or the call sites must all be updated to pass format explicitly.

Useful? React with 👍 / 👎.

Copy link
Contributor

Choose a reason for hiding this comment

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

Super useful. @spomichter, this is why you're seeing color channels flipped.

@spomichter spomichter changed the base branch from dev to matt-image-upgrades October 9, 2025 05:15
@spomichter spomichter changed the base branch from matt-image-upgrades to dev October 9, 2025 20:28
@spomichter
Copy link
Contributor

This branch has been totally refactored and rewritten #677

@spomichter spomichter closed this Oct 12, 2025
@spomichter spomichter deleted the fix-iage branch October 12, 2025 12:35
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