Skip to content

Commit bd7c698

Browse files
rose413rose413
authored
api: add OIIO_NODISCARD_ERROR to ImageOutput methods (#5201)
- Annotate all ImageOutput write methods (write_image, write_scanline, write_scanlines, write_tile, write_tiles, write_rectangle, write_deep_scanlines, write_deep_tiles, write_deep_image), as well as open, close, and copy_image, with OIIO_NODISCARD_ERROR. - Also fix the handful of internal callsites (imagebuf.cpp, maketexture.cpp, py_imagebuf.cpp, iconvert.cpp, imagebufalgo_test.cpp, and imagespeed_test.cpp) that were previously ignoring the return value of close(), either by propagating the error or by casting to void where the error path is already being handled. - Reference: #4781 ### Tests - tested against unit_imageinout, unit_imagebuf, unit_imagebufalgo Assisted-by: Claude Sonnet 4.6 --------- Signed-off-by: rose413 <116857309+rose413@users.noreply.github.com>
1 parent 386d459 commit bd7c698

7 files changed

Lines changed: 118 additions & 77 deletions

File tree

‎src/iconvert/iconvert.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,10 @@ convert_file(const std::string& in_filename, const std::string& out_filename)
494494
} while (ok && in->seek_subimage(subimage, miplevel));
495495
}
496496

497-
out->close();
497+
if (!out->close()) {
498+
errorfmt("Error closing \"{}\" : {}", out_filename, out->geterror());
499+
ok = false;
500+
}
498501
in->close();
499502

500503
// Figure out a time for the input file -- either one supplied by

‎src/include/OpenImageIO/imageio.h‎

Lines changed: 59 additions & 44 deletions
Large diffs are not rendered by default.

‎src/libOpenImageIO/imagebuf.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1811,7 +1811,10 @@ ImageBuf::write(string_view _filename, TypeDesc dtype, string_view _fileformat,
18111811
}
18121812
if (!write(out.get(), progress_callback, progress_callback_data))
18131813
return false;
1814-
out->close();
1814+
if (!out->close()) {
1815+
error(out->geterror());
1816+
return false;
1817+
}
18151818
if (progress_callback)
18161819
progress_callback(progress_callback_data, 0);
18171820
return true;

‎src/libOpenImageIO/imagebufalgo_test.cpp‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,7 +1613,8 @@ test_demosaic_algo(const ImageBuf& src_image, const ImageBuf& mosaiced_image,
16131613
std::string ext = type.is_floating_point() ? "exr" : "png";
16141614
std::string path = file_name + "_" + test_name + "." + ext;
16151615
auto imageOutput = ImageOutput::create(ext);
1616-
imageOutput->open(path, demosaiced_image.spec());
1616+
bool ok = imageOutput->open(path, demosaiced_image.spec());
1617+
OIIO_ASSERT(ok);
16171618
demosaiced_image.write(imageOutput.get());
16181619
}
16191620
}
@@ -1650,7 +1651,8 @@ test_demosaic(const DemosaicTestConfig& config, const ImageBuf& src_image,
16501651
std::string path = file_name + "_src." + ext;
16511652

16521653
auto imageOutput = ImageOutput::create(ext);
1653-
imageOutput->open(path, mosaiced_image.spec());
1654+
bool ok = imageOutput->open(path, mosaiced_image.spec());
1655+
OIIO_ASSERT(ok);
16541656
mosaiced_image.write(imageOutput.get());
16551657
}
16561658

@@ -1741,7 +1743,8 @@ test_demosaic()
17411743

17421744
if (write_files) {
17431745
auto imageOutput = OIIO::ImageOutput::create("exr");
1744-
imageOutput->open("source.exr", src_image.spec());
1746+
bool ok = imageOutput->open("source.exr", src_image.spec());
1747+
OIIO_ASSERT(ok);
17451748
src_image.write(imageOutput.get());
17461749
}
17471750

‎src/libOpenImageIO/imagespeed_test.cpp‎

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,8 @@ time_write_image()
187187
OIIO_ASSERT(out);
188188
bool ok = out->open(output_filename, outspec);
189189
OIIO_ASSERT(ok);
190-
out->write_image(bufspec.format, &buffer[0]);
190+
ok = out->write_image(bufspec.format, &buffer[0]);
191+
OIIO_CONTRACT_ASSERT(ok);
191192
}
192193

193194

@@ -202,10 +203,12 @@ time_write_scanline_at_a_time()
202203

203204
size_t pixelsize = outspec.nchannels * sizeof(float);
204205
imagesize_t scanlinesize = outspec.width * pixelsize;
205-
for (int y = 0; y < outspec.height; ++y) {
206-
out->write_scanline(y + outspec.y, outspec.z, bufspec.format,
207-
&buffer[scanlinesize * y]);
206+
bool ok = true;
207+
for (int y = 0; y < outspec.height && ok; ++y) {
208+
ok = out->write_scanline(y + outspec.y, outspec.z, bufspec.format,
209+
&buffer[scanlinesize * y]);
208210
}
211+
OIIO_ASSERT(ok);
209212
}
210213

211214

@@ -217,16 +220,17 @@ time_write_64_scanlines_at_a_time()
217220
OIIO_ASSERT(out);
218221
bool ok = out->open(output_filename, outspec);
219222
OIIO_ASSERT(ok);
220-
223+
bool ok = true;
221224
size_t pixelsize = outspec.nchannels * sizeof(float);
222225
imagesize_t scanlinesize = outspec.width * pixelsize;
223-
for (int y = 0; y < outspec.height; y += 64) {
224-
out->write_scanlines(y + outspec.y,
225-
std::min(y + outspec.y + 64,
226-
outspec.y + outspec.height),
227-
outspec.z, bufspec.format,
228-
&buffer[scanlinesize * y]);
226+
for (int y = 0; y < outspec.height && ok; y += 64) {
227+
ok = out->write_scanlines(y + outspec.y,
228+
std::min(y + outspec.y + 64,
229+
outspec.y + outspec.height),
230+
outspec.z, bufspec.format,
231+
&buffer[scanlinesize * y]);
229232
}
233+
OIIO_ASSERT(ok);
230234
}
231235

232236

@@ -242,16 +246,18 @@ time_write_tile_at_a_time()
242246
size_t pixelsize = outspec.nchannels * sizeof(float);
243247
imagesize_t scanlinesize = outspec.width * pixelsize;
244248
imagesize_t planesize = outspec.height * scanlinesize;
245-
for (int z = 0; z < outspec.depth; z += outspec.tile_depth) {
246-
for (int y = 0; y < outspec.height; y += outspec.tile_height) {
247-
for (int x = 0; x < outspec.width; x += outspec.tile_width) {
248-
out->write_tile(x + outspec.x, y + outspec.y, z + outspec.z,
249-
bufspec.format,
250-
&buffer[scanlinesize * y + pixelsize * x],
251-
pixelsize, scanlinesize, planesize);
249+
bool ok = true;
250+
for (int z = 0; z < outspec.depth && ok; z += outspec.tile_depth) {
251+
for (int y = 0; y < outspec.height && ok; y += outspec.tile_height) {
252+
for (int x = 0; x < outspec.width && ok; x += outspec.tile_width) {
253+
ok = out->write_tile(x + outspec.x, y + outspec.y,
254+
z + outspec.z, bufspec.format,
255+
&buffer[scanlinesize * y + pixelsize * x],
256+
pixelsize, scanlinesize, planesize);
252257
}
253258
}
254259
}
260+
OIIO_ASSERT(ok);
255261
}
256262

257263

@@ -266,15 +272,20 @@ time_write_tiles_row_at_a_time()
266272

267273
size_t pixelsize = outspec.nchannels * sizeof(float);
268274
imagesize_t scanlinesize = outspec.width * pixelsize;
269-
for (int z = 0; z < outspec.depth; z += outspec.tile_depth) {
270-
for (int y = 0; y < outspec.height; y += outspec.tile_height) {
271-
out->write_tiles(outspec.x, outspec.x + outspec.width,
272-
y + outspec.y, y + outspec.y + outspec.tile_height,
273-
z + outspec.z, z + outspec.z + outspec.tile_depth,
274-
bufspec.format, &buffer[scanlinesize * y],
275-
pixelsize /*xstride*/, scanlinesize /*ystride*/);
275+
bool ok = true;
276+
for (int z = 0; z < outspec.depth && ok; z += outspec.tile_depth) {
277+
for (int y = 0; y < outspec.height && ok; y += outspec.tile_height) {
278+
ok = out->write_tiles(outspec.x, outspec.x + outspec.width,
279+
y + outspec.y,
280+
y + outspec.y + outspec.tile_height,
281+
z + outspec.z,
282+
z + outspec.z + outspec.tile_depth,
283+
bufspec.format, &buffer[scanlinesize * y],
284+
pixelsize /*xstride*/,
285+
scanlinesize /*ystride*/);
276286
}
277287
}
288+
OIIO_ASSERT(ok);
278289
}
279290

280291

‎src/libOpenImageIO/maketexture.cpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -792,7 +792,9 @@ write_mipmap(ImageBufAlgo::MakeTextureMode mode, std::shared_ptr<ImageBuf>& img,
792792
// ImageBuf::write transfers any errors from the ImageOutput to
793793
// the ImageBuf.
794794
errorfmt("Write failed: {}", img->geterror());
795-
out->close();
795+
if (!out->close()) {
796+
errorfmt("Close failed: {}", out->geterror());
797+
}
796798
return false;
797799
}
798800

@@ -965,7 +967,9 @@ write_mipmap(ImageBufAlgo::MakeTextureMode mode, std::shared_ptr<ImageBuf>& img,
965967
// ImageOutput to the ImageBuf.
966968
errorfmt("Error writing \"{}\" : {}", outputfilename,
967969
small->geterror());
968-
out->close();
970+
if (!out->close()) {
971+
errorfmt("Close failed: {}", out->geterror());
972+
}
969973
return false;
970974
}
971975
double wtime = writetimer();

‎src/python/py_imagebuf.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,9 @@ ImageBuf_repr_png(const ImageBuf& self)
273273
if (!out || !out->open("temp.png", altered_spec))
274274
return py::bytes();
275275
self.write(out.get());
276-
out->close();
276+
if (!out->close()) {
277+
return py::bytes();
278+
}
277279

278280
// Cast to const char* and return as python bytes
279281
const char* char_ptr = reinterpret_cast<const char*>(file_buffer.data());

0 commit comments

Comments
 (0)