Skip to content

Commit 604759f

Browse files
committed
Striped GPU uploads for large images
Implement striped/row-wise GPU uploads across Vulkan, OpenGL, and Metal to avoid monolithic source buffers and fix large-image crashes. Add LoadedImage helpers (imiv_loaded_image.*) to validate layout and convert to ImageBuf, and a tiling utility (imiv_tiling.*) to plan and pad row-stripe uploads and copy rows into aligned buffers. Update Vulkan upload path to use dynamic storage-buffer offsets, adjust descriptor types to dynamic storage buffers, and respect device limits (with IMIV_VULKAN_MAX_STORAGE_BUFFER_RANGE_OVERRIDE). Update OpenGL and Metal to perform tiled glTexSubImage2D / per-stripe MTLBuffer uploads and add environment overrides for chunk size (IMIV_OPENGL_MAX_UPLOAD_CHUNK_BYTES_OVERRIDE, IMIV_METAL_MAX_UPLOAD_CHUNK_BYTES_OVERRIDE). Wire in logging, resource/debug labels, and tests/tools: add a focused imiv_large_image_switch_regression.py and CTest entries for Vulkan/OpenGL/Metal, plus documentation updates describing the new striped upload behavior and test instructions. Signed-off-by: Vlad <shaamaan@gmail.com>
1 parent fcaa493 commit 604759f

17 files changed

+1171
-166
lines changed

src/doc/imiv_dev.rst

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -866,6 +866,33 @@ Recommendation for future work:
866866
That separation is the important design guardrail: image storage strategy for
867867
huge files should change independently from per-view preview/export semantics.
868868

869+
The first implementation step toward that model is now in place for
870+
Vulkan/OpenGL/Metal:
871+
872+
* `src/imiv/imiv_vulkan_texture.cpp` no longer binds the whole raw source
873+
payload as one storage-buffer descriptor range;
874+
* the Vulkan upload path now pads row pitch to a device-safe alignment,
875+
dispatches the compute upload in row stripes, and binds those stripes with
876+
dynamic storage-buffer offsets;
877+
* `src/imiv/imiv_renderer_opengl.cpp` now allocates the source texture first
878+
and uploads large images in row stripes via `glTexSubImage2D`, using the
879+
same stripe planner to avoid one monolithic upload call; and
880+
* `src/imiv/imiv_renderer_metal.mm` now keeps the existing compute
881+
normalization shader but dispatches it in row stripes, feeding it one
882+
stripe-sized `MTLBuffer` at a time instead of one monolithic source buffer;
883+
* GPU normalization stays intact, including the current Vulkan RGB-to-RGBA
884+
compute conversion path, the current Metal compute conversion path, and the
885+
current OpenGL native-channel upload path.
886+
887+
This is intentionally still a partial step:
888+
889+
* the viewer still loads full source pixels into `LoadedImage`;
890+
* visible-region tile caching is still future work.
891+
892+
So the immediate large-image Vulkan crash is addressed, OpenGL and Metal now
893+
use the same striped-upload seam, and the broader cross-backend tile/proxy
894+
architecture remains the next major renderer task.
895+
869896
OCIO integration
870897
----------------
871898

src/doc/imiv_tests.rst

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,11 @@ Recent focused GUI regressions in `src/imiv/tools/` include:
244244
* `imiv_save_window_ocio_regression.py`
245245
GUI-driven `Export As...` OCIO export, including view-baked display/view
246246
validation against `oiiotool --ociodisplay`.
247+
* `imiv_large_image_switch_regression.py`
248+
GPU-backend large-image queue switching. It currently has focused `ctest`
249+
entries for Vulkan, OpenGL, and Metal where those backends are enabled,
250+
forcing the striped upload path and verifying that next/previous image
251+
navigation does not regress on large multi-image sessions.
247252

248253

249254
Direct Dear ImGui Test Engine usage
@@ -433,6 +438,10 @@ Run the focused per-view recipe regression::
433438

434439
ctest --test-dir build_u -V -R '^imiv_view_recipe_regression$'
435440

441+
Run the focused Vulkan/OpenGL/Metal large-image switch regressions::
442+
443+
ctest --test-dir build_u -V -R '^imiv_large_image_switch_regression_(vulkan|opengl|metal)$'
444+
436445
Run the backend-preference regression::
437446

438447
ctest --test-dir build_u -V -R '^imiv_backend_preferences_regression$'
@@ -468,6 +477,12 @@ When adding new UI or UX behavior, the most durable workflow is usually:
468477
`imiv_gui_test_run.py` or the existing focused regression scripts before
469478
inventing a new runner.
470479
6. Add backend coverage intentionally.
480+
The large-image GPU regressions use
481+
`IMIV_VULKAN_MAX_STORAGE_BUFFER_RANGE_OVERRIDE` and
482+
`IMIV_OPENGL_MAX_UPLOAD_CHUNK_BYTES_OVERRIDE` and
483+
`IMIV_METAL_MAX_UPLOAD_CHUNK_BYTES_OVERRIDE` internally so the striped
484+
upload paths are exercised deterministically even on devices with larger
485+
native limits.
471486
If a feature is backend-sensitive, decide whether it needs a backend-wide
472487
verification entry, a focused regression, or both.
473488

src/imiv/CMakeLists.txt

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,13 +169,15 @@ set (_imiv_shared_sources
169169
imiv_file_dialog.cpp
170170
imiv_frame.cpp
171171
imiv_image_view.cpp
172+
imiv_loaded_image.cpp
172173
imiv_menu.cpp
173174
imiv_navigation.cpp
174175
imiv_ocio.cpp
175176
imiv_overlays.cpp
176177
imiv_renderer.cpp
177178
imiv_shader_compile.cpp
178179
imiv_style.cpp
180+
imiv_tiling.cpp
179181
imiv_upload_types.cpp
180182
imiv_ui.cpp
181183
imiv_viewer.cpp
@@ -1574,6 +1576,60 @@ if (TARGET imiv
15741576
LABELS "imiv;imiv_sampling;gui"
15751577
TIMEOUT 180)
15761578

1579+
if (_imiv_enabled_vulkan)
1580+
add_test (
1581+
NAME imiv_large_image_switch_regression_vulkan
1582+
COMMAND
1583+
"${Python3_EXECUTABLE}"
1584+
"${CMAKE_CURRENT_SOURCE_DIR}/tools/imiv_large_image_switch_regression.py"
1585+
--bin "$<TARGET_FILE:imiv>"
1586+
--cwd "$<TARGET_FILE_DIR:imiv>"
1587+
--backend vulkan
1588+
--oiiotool "$<TARGET_FILE:oiiotool>"
1589+
--env-script "${CMAKE_BINARY_DIR}/imiv_env.sh"
1590+
--out-dir "${CMAKE_BINARY_DIR}/imiv_captures/large_image_switch_regression_vulkan")
1591+
set_tests_properties (
1592+
imiv_large_image_switch_regression_vulkan PROPERTIES
1593+
LABELS "imiv;gui;imiv_vulkan;imiv_large"
1594+
TIMEOUT 300)
1595+
endif ()
1596+
1597+
if (_imiv_enabled_opengl)
1598+
add_test (
1599+
NAME imiv_large_image_switch_regression_opengl
1600+
COMMAND
1601+
"${Python3_EXECUTABLE}"
1602+
"${CMAKE_CURRENT_SOURCE_DIR}/tools/imiv_large_image_switch_regression.py"
1603+
--bin "$<TARGET_FILE:imiv>"
1604+
--cwd "$<TARGET_FILE_DIR:imiv>"
1605+
--backend opengl
1606+
--oiiotool "$<TARGET_FILE:oiiotool>"
1607+
--env-script "${CMAKE_BINARY_DIR}/imiv_env.sh"
1608+
--out-dir "${CMAKE_BINARY_DIR}/imiv_captures/large_image_switch_regression_opengl")
1609+
set_tests_properties (
1610+
imiv_large_image_switch_regression_opengl PROPERTIES
1611+
LABELS "imiv;gui;imiv_opengl;imiv_large"
1612+
TIMEOUT 300)
1613+
endif ()
1614+
1615+
if (_imiv_enabled_metal)
1616+
add_test (
1617+
NAME imiv_large_image_switch_regression_metal
1618+
COMMAND
1619+
"${Python3_EXECUTABLE}"
1620+
"${CMAKE_CURRENT_SOURCE_DIR}/tools/imiv_large_image_switch_regression.py"
1621+
--bin "$<TARGET_FILE:imiv>"
1622+
--cwd "$<TARGET_FILE_DIR:imiv>"
1623+
--backend metal
1624+
--oiiotool "$<TARGET_FILE:oiiotool>"
1625+
--env-script "${CMAKE_BINARY_DIR}/imiv_env.sh"
1626+
--out-dir "${CMAKE_BINARY_DIR}/imiv_captures/large_image_switch_regression_metal")
1627+
set_tests_properties (
1628+
imiv_large_image_switch_regression_metal PROPERTIES
1629+
LABELS "imiv;gui;imiv_metal;imiv_large"
1630+
TIMEOUT 300)
1631+
endif ()
1632+
15771633
if (_imiv_enabled_backend_count GREATER 1)
15781634
add_test (
15791635
NAME imiv_backend_preferences_regression

src/imiv/README.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,12 @@ Focused per-view recipe regression:
157157
ctest --test-dir build_u -V -R '^imiv_view_recipe_regression$'
158158
```
159159

160+
Focused Vulkan/OpenGL/Metal large-image switch regressions:
161+
162+
```bash
163+
ctest --test-dir build_u -V -R '^imiv_large_image_switch_regression_(vulkan|opengl|metal)$'
164+
```
165+
160166
Focused Save Selection export regression:
161167

162168
```bash

src/imiv/imiv_actions.cpp

Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "imiv_actions.h"
66

77
#include "imiv_file_dialog.h"
8+
#include "imiv_loaded_image.h"
89
#include "imiv_ocio.h"
910
#include "imiv_ui.h"
1011

@@ -410,59 +411,6 @@ open_dialog_default_path(const ViewerState& viewer,
410411
return stem + "_window" + ext;
411412
}
412413

413-
bool
414-
imagebuf_from_loaded_image(const LoadedImage& image, ImageBuf& out,
415-
std::string& error_message)
416-
{
417-
error_message.clear();
418-
const TypeDesc format = upload_data_type_to_typedesc(image.type);
419-
if (format == TypeUnknown) {
420-
error_message = "unsupported source pixel type";
421-
return false;
422-
}
423-
if (image.width <= 0 || image.height <= 0 || image.nchannels <= 0) {
424-
error_message = "no valid image is loaded";
425-
return false;
426-
}
427-
428-
const size_t width = static_cast<size_t>(image.width);
429-
const size_t height = static_cast<size_t>(image.height);
430-
const size_t channels = static_cast<size_t>(image.nchannels);
431-
const size_t min_row_pitch = width * channels * image.channel_bytes;
432-
if (image.row_pitch_bytes < min_row_pitch) {
433-
error_message = "image row pitch is invalid";
434-
return false;
435-
}
436-
const size_t required_bytes = image.row_pitch_bytes * height;
437-
if (image.pixels.size() < required_bytes) {
438-
error_message = "image pixel buffer is incomplete";
439-
return false;
440-
}
441-
442-
ImageSpec spec(image.width, image.height, image.nchannels, format);
443-
if (image.channel_names.size() == channels)
444-
spec.channelnames = image.channel_names;
445-
spec.attribute("Orientation", image.orientation);
446-
if (!image.metadata_color_space.empty())
447-
spec.attribute("oiio:ColorSpace", image.metadata_color_space);
448-
449-
out.reset(spec);
450-
const std::byte* begin = reinterpret_cast<const std::byte*>(
451-
image.pixels.data());
452-
const cspan<std::byte> byte_span(begin, image.pixels.size());
453-
const stride_t xstride = static_cast<stride_t>(image.nchannels
454-
* image.channel_bytes);
455-
const stride_t ystride = static_cast<stride_t>(image.row_pitch_bytes);
456-
if (!out.set_pixels(ROI::All(), format, byte_span, begin, xstride,
457-
ystride, AutoStride)) {
458-
error_message = out.geterror();
459-
if (error_message.empty())
460-
error_message = "failed to copy source pixels into ImageBuf";
461-
return false;
462-
}
463-
return true;
464-
}
465-
466414
bool
467415
build_selection_source_image(const LoadedImage& image,
468416
const ViewerState& viewer, ImageBuf& result,

src/imiv/imiv_loaded_image.cpp

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
// Copyright Contributors to the OpenImageIO project.
2+
// SPDX-License-Identifier: Apache-2.0
3+
// https://github.com/AcademySoftwareFoundation/OpenImageIO
4+
5+
#include "imiv_loaded_image.h"
6+
7+
#include <cstddef>
8+
#include <string>
9+
10+
using namespace OIIO;
11+
12+
namespace Imiv {
13+
14+
bool
15+
describe_loaded_image_layout(const LoadedImage& image,
16+
LoadedImageLayout& layout,
17+
std::string& error_message)
18+
{
19+
error_message.clear();
20+
layout = LoadedImageLayout();
21+
22+
if (image.width <= 0 || image.height <= 0 || image.nchannels <= 0
23+
|| image.channel_bytes == 0) {
24+
error_message = "invalid source image dimensions";
25+
return false;
26+
}
27+
28+
const size_t width = static_cast<size_t>(image.width);
29+
const size_t height = static_cast<size_t>(image.height);
30+
const size_t channels = static_cast<size_t>(image.nchannels);
31+
const size_t pixel_stride = channels * image.channel_bytes;
32+
const size_t min_row_pitch = width * pixel_stride;
33+
if (pixel_stride == 0 || image.row_pitch_bytes < min_row_pitch) {
34+
error_message = "invalid source row pitch";
35+
return false;
36+
}
37+
38+
const size_t required_bytes = image.row_pitch_bytes * height;
39+
if (image.pixels.size() < required_bytes) {
40+
error_message = "source pixel buffer is smaller than declared stride";
41+
return false;
42+
}
43+
44+
layout.pixel_stride_bytes = pixel_stride;
45+
layout.min_row_pitch_bytes = min_row_pitch;
46+
layout.required_bytes = required_bytes;
47+
return true;
48+
}
49+
50+
bool
51+
loaded_image_pixel_pointer(const LoadedImage& image, int x, int y,
52+
const unsigned char*& out_pixel,
53+
LoadedImageLayout* out_layout,
54+
std::string* error_message)
55+
{
56+
out_pixel = nullptr;
57+
LoadedImageLayout layout;
58+
std::string local_error;
59+
if (!describe_loaded_image_layout(image, layout, local_error)) {
60+
if (error_message != nullptr)
61+
*error_message = local_error;
62+
return false;
63+
}
64+
if (x < 0 || y < 0 || x >= image.width || y >= image.height) {
65+
if (error_message != nullptr)
66+
*error_message = "requested pixel is outside the loaded image";
67+
return false;
68+
}
69+
70+
const size_t row_start = static_cast<size_t>(y) * image.row_pitch_bytes;
71+
const size_t pixel_start = static_cast<size_t>(x) * layout.pixel_stride_bytes;
72+
const size_t offset = row_start + pixel_start;
73+
if (offset + layout.pixel_stride_bytes > image.pixels.size()) {
74+
if (error_message != nullptr)
75+
*error_message = "requested pixel lies outside the loaded buffer";
76+
return false;
77+
}
78+
79+
out_pixel = image.pixels.data() + offset;
80+
if (out_layout != nullptr)
81+
*out_layout = layout;
82+
if (error_message != nullptr)
83+
error_message->clear();
84+
return true;
85+
}
86+
87+
bool
88+
imagebuf_from_loaded_image(const LoadedImage& image, ImageBuf& out,
89+
std::string& error_message)
90+
{
91+
error_message.clear();
92+
const TypeDesc format = upload_data_type_to_typedesc(image.type);
93+
if (format == TypeUnknown) {
94+
error_message = "unsupported source pixel type";
95+
return false;
96+
}
97+
98+
LoadedImageLayout layout;
99+
if (!describe_loaded_image_layout(image, layout, error_message))
100+
return false;
101+
102+
ImageSpec spec(image.width, image.height, image.nchannels, format);
103+
if (image.channel_names.size() == static_cast<size_t>(image.nchannels))
104+
spec.channelnames = image.channel_names;
105+
spec.attribute("Orientation", image.orientation);
106+
if (!image.metadata_color_space.empty())
107+
spec.attribute("oiio:ColorSpace", image.metadata_color_space);
108+
109+
out.reset(spec);
110+
const std::byte* begin = reinterpret_cast<const std::byte*>(image.pixels.data());
111+
const cspan<std::byte> byte_span(begin, layout.required_bytes);
112+
const stride_t xstride = static_cast<stride_t>(layout.pixel_stride_bytes);
113+
const stride_t ystride = static_cast<stride_t>(image.row_pitch_bytes);
114+
if (!out.set_pixels(ROI::All(), format, byte_span, begin, xstride, ystride,
115+
AutoStride)) {
116+
error_message = out.geterror();
117+
if (error_message.empty())
118+
error_message = "failed to copy source pixels into ImageBuf";
119+
return false;
120+
}
121+
return true;
122+
}
123+
124+
} // namespace Imiv

src/imiv/imiv_loaded_image.h

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright Contributors to the OpenImageIO project.
2+
// SPDX-License-Identifier: Apache-2.0
3+
// https://github.com/AcademySoftwareFoundation/OpenImageIO
4+
5+
#pragma once
6+
7+
#include "imiv_types.h"
8+
9+
#include <cstddef>
10+
#include <string>
11+
12+
#include <OpenImageIO/imagebuf.h>
13+
14+
namespace Imiv {
15+
16+
struct LoadedImageLayout {
17+
size_t pixel_stride_bytes = 0;
18+
size_t min_row_pitch_bytes = 0;
19+
size_t required_bytes = 0;
20+
};
21+
22+
bool
23+
describe_loaded_image_layout(const LoadedImage& image,
24+
LoadedImageLayout& layout,
25+
std::string& error_message);
26+
27+
bool
28+
loaded_image_pixel_pointer(const LoadedImage& image, int x, int y,
29+
const unsigned char*& out_pixel,
30+
LoadedImageLayout* out_layout = nullptr,
31+
std::string* error_message = nullptr);
32+
33+
bool
34+
imagebuf_from_loaded_image(const LoadedImage& image, OIIO::ImageBuf& out,
35+
std::string& error_message);
36+
37+
} // namespace Imiv

0 commit comments

Comments
 (0)