Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions src/renderkit/ui/preview_image.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
"""Preview image marshaling helpers for Qt display."""

from dataclasses import dataclass
from typing import Any

import numpy as np
import OpenImageIO as oiio

from renderkit.ui.qt_compat import QImage, QPixmap


@dataclass(frozen=True)
class PreviewImageData:
"""Contiguous uint8 preview pixels that can cross the worker/UI boundary."""

pixels: np.ndarray
width: int
height: int
channels: int


def imagebuf_to_preview_image(buf: oiio.ImageBuf) -> PreviewImageData:
"""Convert an ImageBuf into contiguous uint8 RGB/RGBA preview data."""
image = buf.get_pixels(oiio.FLOAT)
if image is None or image.size == 0:
raise ValueError("Failed to extract preview pixels.")

spec = buf.spec()
if image.ndim == 1:
image = image.reshape((spec.height, spec.width, spec.nchannels))
Comment on lines +29 to +30

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check for tests that exercise the 1D reshape path

rg -n 'ndim.*==.*1' --type=py tests/

Repository: Ahmed-Hindy/renderkit

Length of output: 286


🏁 Script executed:

# First, let's understand the preview_image.py file and what function contains lines 29-30
cat -n src/renderkit/ui/preview_image.py | head -40

Repository: Ahmed-Hindy/renderkit

Length of output: 1508


🏁 Script executed:

# Check if there are tests specifically for preview_image.py
fd -e py tests/ | xargs grep -l "preview_image"

Repository: Ahmed-Hindy/renderkit

Length of output: 397


🏁 Script executed:

# Look at the test files with ndim==1 to see if they test preview_image
cat -n tests/test_integration_real_files.py | sed -n '85,135p'

Repository: Ahmed-Hindy/renderkit

Length of output: 2491


🏁 Script executed:

# Search for tests that import or use imagebuf_to_preview_image
rg -n "imagebuf_to_preview_image" tests/

Repository: Ahmed-Hindy/renderkit

Length of output: 815


🏁 Script executed:

# Check if there are any tests for preview_image module
rg -n "from.*preview_image|import.*preview_image" tests/

Repository: Ahmed-Hindy/renderkit

Length of output: 134


🏁 Script executed:

# Look at test_color_space.py around line 40
cat -n tests/test_color_space.py | sed -n '30,50p'

Repository: Ahmed-Hindy/renderkit

Length of output: 830


🏁 Script executed:

# Look at the existing preview_image tests to see if they cover 1D arrays
cat -n tests/test_ui_widgets.py | sed -n '95,180p'

Repository: Ahmed-Hindy/renderkit

Length of output: 3431


Add test coverage for 1D array reshape in imagebuf_to_preview_image().

The 1D-to-3D reshape logic (lines 29–30) is not exercised by the existing test suite. Current tests for imagebuf_to_preview_image() only pass 3D pixel arrays, leaving this code path untested. The team's use of the same reshape pattern in other modules (e.g., _buf_to_array() in test_color_space.py) confirms this is a real edge case. Add a test case where buf.get_pixels() returns a 1D array to ensure the reshape works correctly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/renderkit/ui/preview_image.py` around lines 29 - 30, Add a test case for
the `imagebuf_to_preview_image()` function that covers the 1D-to-3D reshape code
path. Create a test where `buf.get_pixels()` returns a 1D array with a length
equal to height × width × nchannels, then verify that the function correctly
reshapes it into a 3D array with dimensions (spec.height, spec.width,
spec.nchannels). This will exercise the currently untested reshape logic at
lines 29-30 and ensure it handles the edge case correctly.


if image.ndim != 3:
raise ValueError(f"Unsupported preview image dimensions: {image.ndim}")

height, width, channels = image.shape
if channels not in (3, 4):
raise ValueError(f"Unsupported image channels: {channels}")

if image.dtype != np.uint8:
image_f32 = image.astype(np.float32, copy=False)
image = np.clip(image_f32, 0.0, 1.0)
image = (image * np.float32(255.0)).astype(np.uint8)

image = np.ascontiguousarray(image)
return PreviewImageData(
pixels=image,
width=width,
height=height,
channels=channels,
)


def preview_image_to_qimage(data: PreviewImageData) -> QImage:
"""Create a QImage from preview image data."""
q_format = _qimage_format(data.channels)
bytes_per_line = data.width * data.channels
image = QImage(
data.pixels.data,
data.width,
data.height,
bytes_per_line,
q_format,
)
return image.copy()


def preview_image_to_pixmap(data: PreviewImageData) -> QPixmap:
"""Create a QPixmap from preview image data."""
return QPixmap.fromImage(preview_image_to_qimage(data))


def _qimage_format(channels: int) -> Any:
if channels == 3:
name = "Format_RGB888"
elif channels == 4:
name = "Format_RGBA8888"
else:
raise ValueError(f"Unsupported image channels: {channels}")

enum = getattr(QImage, "Format", None)
if enum is not None:
value = getattr(enum, name, None)
if value is not None:
return value

value = getattr(QImage, name, None)
if value is None:
raise ValueError(f"Qt image format is unavailable: {name}")
return value
68 changes: 9 additions & 59 deletions src/renderkit/ui/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from pathlib import Path
from typing import Any, Optional

import numpy as np
import OpenImageIO as oiio

from renderkit.core.config import BurnInConfig, BurnInElement, ContactSheetConfig
Expand All @@ -18,11 +17,15 @@
)
from renderkit.processing.scaler import ImageScaler
from renderkit.ui.icons import icon_manager
from renderkit.ui.preview_image import (
PreviewImageData,
imagebuf_to_preview_image,
preview_image_to_pixmap,
)
from renderkit.ui.qt_compat import (
QApplication,
QFrame,
QHBoxLayout,
QImage,
QLabel,
QPixmap,
QPoint,
Expand Down Expand Up @@ -97,7 +100,7 @@ def _prepare_buf_for_preview_display(
class PreviewWorker(QThread):
"""Worker thread for loading preview image."""

preview_ready = Signal(QPixmap)
preview_ready = Signal(object)
error = Signal(str)

def __init__(
Expand Down Expand Up @@ -167,60 +170,7 @@ def run(self) -> None:
ColorSpacePreset.NO_CONVERSION,
input_space=None,
)

image = buf.get_pixels(oiio.FLOAT)
if image is None or image.size == 0:
raise ValueError("Failed to extract preview pixels.")
spec = buf.spec()
if image.ndim == 1:
image = image.reshape((spec.height, spec.width, spec.nchannels))

# Convert to uint8
if image.dtype != np.uint8:
image_f32 = image.astype(np.float32, copy=False)
image = np.clip(image_f32, 0.0, 1.0)
image = (image * np.float32(255.0)).astype(np.uint8)

# Convert to QImage
height, width = image.shape[:2]
# Handle different Qt versions - format access differs
# PySide6/PyQt6: QImage.Format.Format_RGB888
# PySide2/PyQt5: QImage.Format_RGB888
try:
# Try PySide6/PyQt6 style
rgb_format = getattr(QImage.Format, "Format_RGB888", None)
rgba_format = getattr(QImage.Format, "Format_RGBA8888", None)
except (AttributeError, TypeError):
# Try PySide2/PyQt5 style
rgb_format = getattr(QImage, "Format_RGB888", None)
rgba_format = getattr(QImage, "Format_RGBA8888", None)

# Fallback to direct attribute access if getattr failed
if rgb_format is None:
try:
rgb_format = QImage.Format.Format_RGB888
except AttributeError:
rgb_format = QImage.Format_RGB888

if rgba_format is None:
try:
rgba_format = QImage.Format.Format_RGBA8888
except AttributeError:
rgba_format = QImage.Format_RGBA8888

if image.shape[2] == 3:
# RGB
q_image = QImage(image.data, width, height, width * 3, rgb_format)
elif image.shape[2] == 4:
# RGBA
q_image = QImage(image.data, width, height, width * 4, rgba_format)
else:
raise ValueError(f"Unsupported image channels: {image.shape[2]}")

# Create pixmap
pixmap = QPixmap.fromImage(q_image)

self.preview_ready.emit(pixmap)
self.preview_ready.emit(imagebuf_to_preview_image(buf))
except (RenderKitError, OSError, RuntimeError, TypeError, ValueError) as e:
logger.exception(
"Preview worker failed. file=%s color_space=%s input_space=%s layer=%s "
Expand Down Expand Up @@ -599,9 +549,9 @@ def _on_worker_finished(self) -> None:
if worker == self.worker:
self.worker = None

def _on_preview_ready(self, pixmap: QPixmap) -> None:
def _on_preview_ready(self, preview_image: PreviewImageData) -> None:
"""Handle preview ready."""
self._original_pixmap = pixmap
self._original_pixmap = preview_image_to_pixmap(preview_image)
self.preview_label.setText("")
self._update_scaled_pixmap()
self.expand_btn.show()
Expand Down
102 changes: 102 additions & 0 deletions tests/test_ui_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@
PreparedFrameBuffer,
prepare_frame_buffer,
)
from renderkit.ui.preview_image import (
PreviewImageData,
imagebuf_to_preview_image,
preview_image_to_pixmap,
)
from renderkit.ui.qt_compat import QPixmap
from renderkit.ui.widgets import (
PreviewWorker,
_prepare_buf_for_preview_display,
Expand Down Expand Up @@ -92,6 +98,100 @@ def test_prepare_buf_for_preview_display_expands_data_channels(channels: int) ->
np.testing.assert_allclose(result_pixels[:, :, 2], pixels[:, :, 0])


def test_imagebuf_to_preview_image_converts_rgb_float_to_uint8() -> None:
"""RGB float preview buffers should become contiguous uint8 payloads."""
if oiio is None:
pytest.skip("OpenImageIO not available")

pixels = np.array(
[
[[0.0, 0.5, 1.0], [1.5, -0.5, 0.25]],
[[0.2, 0.4, 0.6], [0.8, 1.0, 0.0]],
],
dtype=np.float32,
)
spec = oiio.ImageSpec(2, 2, 3, oiio.FLOAT)
buf = oiio.ImageBuf(spec)
assert buf.set_pixels(oiio.ROI(), pixels)

result = imagebuf_to_preview_image(buf)

assert result.width == 2
assert result.height == 2
assert result.channels == 3
assert result.pixels.dtype == np.uint8
assert result.pixels.flags.c_contiguous
np.testing.assert_array_equal(
result.pixels,
np.array(
[
[[0, 127, 255], [255, 0, 63]],
[[51, 102, 153], [204, 255, 0]],
],
dtype=np.uint8,
),
)


def test_imagebuf_to_preview_image_preserves_rgba_channels() -> None:
"""RGBA preview buffers should preserve the alpha channel."""
if oiio is None:
pytest.skip("OpenImageIO not available")

pixels = np.array([[[0.0, 0.25, 0.5, 1.0]]], dtype=np.float32)
spec = oiio.ImageSpec(1, 1, 4, oiio.FLOAT)
buf = oiio.ImageBuf(spec)
assert buf.set_pixels(oiio.ROI(), pixels)

result = imagebuf_to_preview_image(buf)

assert result.channels == 4
np.testing.assert_array_equal(
result.pixels,
np.array([[[0, 63, 127, 255]]], dtype=np.uint8),
)


def test_imagebuf_to_preview_image_rejects_empty_pixels() -> None:
"""Empty OIIO pixel extraction should fail clearly."""

class EmptyBuf:
def get_pixels(self, pixel_type):
return None

with pytest.raises(ValueError, match="Failed to extract preview pixels"):
imagebuf_to_preview_image(EmptyBuf())


def test_imagebuf_to_preview_image_rejects_unsupported_channels() -> None:
"""Only RGB/RGBA buffers should reach Qt preview marshaling."""
if oiio is None:
pytest.skip("OpenImageIO not available")

pixels = np.ones((1, 1, 5), dtype=np.float32)
spec = oiio.ImageSpec(1, 1, 5, oiio.FLOAT)
buf = oiio.ImageBuf(spec)
assert buf.set_pixels(oiio.ROI(), pixels)

with pytest.raises(ValueError, match="Unsupported image channels: 5"):
imagebuf_to_preview_image(buf)


def test_preview_image_to_pixmap_returns_pixmap(qapp) -> None:
"""Preview image data should become a valid GUI-thread pixmap."""
data = PreviewImageData(
pixels=np.full((1, 1, 3), 255, dtype=np.uint8),
width=1,
height=1,
channels=3,
)

pixmap = preview_image_to_pixmap(data)

assert isinstance(pixmap, QPixmap)
assert not pixmap.isNull()


def test_prepare_frame_buffer_expands_data_channels_without_color_conversion() -> None:
"""Shared frame prep should keep data-channel buffers preview/render safe."""
if oiio is None:
Expand Down Expand Up @@ -169,6 +269,8 @@ def fake_prepare_frame_buffer(options):
worker.run()

assert emitted
assert isinstance(emitted[0], PreviewImageData)
assert not isinstance(emitted[0], QPixmap)
assert calls
call = calls[0]
assert call.frame_path == "render.0001.exr"
Expand Down
Loading