From 9ea62ff3cb83e8f6627eb1e6123a233b53c3c286 Mon Sep 17 00:00:00 2001 From: Steve Hollasch Date: Wed, 31 Dec 2025 15:20:55 -0800 Subject: [PATCH 1/2] Scope lifetime of rtw_image::fdata to load func The linear floating-point image data stored in the `rtw_image::fdata` member variable had a lifetime from image load to the class destruction. We don't need the original floating-point data after the image has been loaded. This change moves the floating-point data buffer inside the load() function. It is released before the load function returns. Resolves #1723 --- CHANGELOG.md | 3 ++- books/RayTracingTheNextWeek.html | 14 +++++++------- src/TheNextWeek/rtw_stb_image.h | 14 +++++++------- src/TheRestOfYourLife/rtw_stb_image.h | 14 +++++++------- 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7bb82d56..caec66c5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Change Log / Ray Tracing in One Weekend ### The Next Week - Fix -- Remove premature source line for call to `get_sphere_uv` (#1701) + - Change -- Improve lifetime of float image data `fdata` in `rtw_image` class (#1723) ### The Rest of Your Life @@ -19,7 +20,7 @@ Change Log / Ray Tracing in One Weekend ### Common - Fix -- Fixed some dangling references to `random_in_unit_sphere()` (#1637) - Fix -- Clarify `uniform_real_distribution` usage for `random_double()` (#1680) - - Update -- CMake minimum required version max now at 4.0.0. + - Change -- CMake minimum required version max now at 4.0.0. ### In One Weekend - Fix -- Fix equation for refracted rays of non-unit length (#1644) diff --git a/books/RayTracingTheNextWeek.html b/books/RayTracingTheNextWeek.html index cda2e6e87..6a52fb5fc 100644 --- a/books/RayTracingTheNextWeek.html +++ b/books/RayTracingTheNextWeek.html @@ -1715,7 +1715,6 @@ ~rtw_image() { delete[] bdata; - STBI_FREE(fdata); } bool load(const std::string& filename) { @@ -1726,16 +1725,18 @@ // below, for the full height of the image. auto n = bytes_per_pixel; // Dummy out parameter: original components per pixel - fdata = stbi_loadf(filename.c_str(), &image_width, &image_height, &n, bytes_per_pixel); + float *fdata = stbi_loadf(filename.c_str(), &image_width, &image_height, &n, bytes_per_pixel); if (fdata == nullptr) return false; bytes_per_scanline = image_width * bytes_per_pixel; - convert_to_bytes(); + convert_to_bytes(fdata); + + STBI_FREE(fdata); return true; } - int width() const { return (fdata == nullptr) ? 0 : image_width; } - int height() const { return (fdata == nullptr) ? 0 : image_height; } + int width() const { return image_width; } + int height() const { return image_height; } const unsigned char* pixel_data(int x, int y) const { // Return the address of the three RGB bytes of the pixel at x,y. If there is no image @@ -1751,7 +1752,6 @@ private: const int bytes_per_pixel = 3; - float *fdata = nullptr; // Linear floating point pixel data unsigned char *bdata = nullptr; // Linear 8-bit pixel data int image_width = 0; // Loaded image width int image_height = 0; // Loaded image height @@ -1772,7 +1772,7 @@ return static_cast(256.0 * value); } - void convert_to_bytes() { + void convert_to_bytes(float *fdata) { // Convert the linear floating point pixel data to bytes, storing the resulting byte // data in the `bdata` member. diff --git a/src/TheNextWeek/rtw_stb_image.h b/src/TheNextWeek/rtw_stb_image.h index 06fa19b24..d92b06cfb 100644 --- a/src/TheNextWeek/rtw_stb_image.h +++ b/src/TheNextWeek/rtw_stb_image.h @@ -53,7 +53,6 @@ class rtw_image { ~rtw_image() { delete[] bdata; - STBI_FREE(fdata); } bool load(const std::string& filename) { @@ -64,16 +63,18 @@ class rtw_image { // below, for the full height of the image. auto n = bytes_per_pixel; // Dummy out parameter: original components per pixel - fdata = stbi_loadf(filename.c_str(), &image_width, &image_height, &n, bytes_per_pixel); + float *fdata = stbi_loadf(filename.c_str(), &image_width, &image_height, &n, bytes_per_pixel); if (fdata == nullptr) return false; bytes_per_scanline = image_width * bytes_per_pixel; - convert_to_bytes(); + convert_to_bytes(fdata); + + STBI_FREE(fdata); return true; } - int width() const { return (fdata == nullptr) ? 0 : image_width; } - int height() const { return (fdata == nullptr) ? 0 : image_height; } + int width() const { return image_width; } + int height() const { return image_height; } const unsigned char* pixel_data(int x, int y) const { // Return the address of the three RGB bytes of the pixel at x,y. If there is no image @@ -89,7 +90,6 @@ class rtw_image { private: const int bytes_per_pixel = 3; - float *fdata = nullptr; // Linear floating point pixel data unsigned char *bdata = nullptr; // Linear 8-bit pixel data int image_width = 0; // Loaded image width int image_height = 0; // Loaded image height @@ -110,7 +110,7 @@ class rtw_image { return static_cast(256.0 * value); } - void convert_to_bytes() { + void convert_to_bytes(float *fdata) { // Convert the linear floating point pixel data to bytes, storing the resulting byte // data in the `bdata` member. diff --git a/src/TheRestOfYourLife/rtw_stb_image.h b/src/TheRestOfYourLife/rtw_stb_image.h index 06fa19b24..d92b06cfb 100644 --- a/src/TheRestOfYourLife/rtw_stb_image.h +++ b/src/TheRestOfYourLife/rtw_stb_image.h @@ -53,7 +53,6 @@ class rtw_image { ~rtw_image() { delete[] bdata; - STBI_FREE(fdata); } bool load(const std::string& filename) { @@ -64,16 +63,18 @@ class rtw_image { // below, for the full height of the image. auto n = bytes_per_pixel; // Dummy out parameter: original components per pixel - fdata = stbi_loadf(filename.c_str(), &image_width, &image_height, &n, bytes_per_pixel); + float *fdata = stbi_loadf(filename.c_str(), &image_width, &image_height, &n, bytes_per_pixel); if (fdata == nullptr) return false; bytes_per_scanline = image_width * bytes_per_pixel; - convert_to_bytes(); + convert_to_bytes(fdata); + + STBI_FREE(fdata); return true; } - int width() const { return (fdata == nullptr) ? 0 : image_width; } - int height() const { return (fdata == nullptr) ? 0 : image_height; } + int width() const { return image_width; } + int height() const { return image_height; } const unsigned char* pixel_data(int x, int y) const { // Return the address of the three RGB bytes of the pixel at x,y. If there is no image @@ -89,7 +90,6 @@ class rtw_image { private: const int bytes_per_pixel = 3; - float *fdata = nullptr; // Linear floating point pixel data unsigned char *bdata = nullptr; // Linear 8-bit pixel data int image_width = 0; // Loaded image width int image_height = 0; // Loaded image height @@ -110,7 +110,7 @@ class rtw_image { return static_cast(256.0 * value); } - void convert_to_bytes() { + void convert_to_bytes(float *fdata) { // Convert the linear floating point pixel data to bytes, storing the resulting byte // data in the `bdata` member. From 90b0df835a1f3e4b0b23d9be340ded4f8f2044b6 Mon Sep 17 00:00:00 2001 From: Steve Hollasch Date: Wed, 31 Dec 2025 18:47:46 -0800 Subject: [PATCH 2/2] Incorporate copilot review changes --- CHANGELOG.md | 2 +- books/RayTracingTheNextWeek.html | 9 +++++++-- src/TheNextWeek/rtw_stb_image.h | 9 +++++++-- src/TheRestOfYourLife/rtw_stb_image.h | 9 +++++++-- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index caec66c5c..30ec8338e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,12 +4,12 @@ Change Log / Ray Tracing in One Weekend # v4.0.3 (in progress) ### Common + - Change -- Improve lifetime of float image data `fdata` in `rtw_image` class (#1723) ### In One Weekend ### The Next Week - Fix -- Remove premature source line for call to `get_sphere_uv` (#1701) - - Change -- Improve lifetime of float image data `fdata` in `rtw_image` class (#1723) ### The Rest of Your Life diff --git a/books/RayTracingTheNextWeek.html b/books/RayTracingTheNextWeek.html index 6a52fb5fc..47e493341 100644 --- a/books/RayTracingTheNextWeek.html +++ b/books/RayTracingTheNextWeek.html @@ -1725,8 +1725,13 @@ // below, for the full height of the image. auto n = bytes_per_pixel; // Dummy out parameter: original components per pixel - float *fdata = stbi_loadf(filename.c_str(), &image_width, &image_height, &n, bytes_per_pixel); - if (fdata == nullptr) return false; + float *fdata = + stbi_loadf(filename.c_str(), &image_width, &image_height, &n, bytes_per_pixel); + + if (fdata == nullptr) { + image_width = image_height = 0; + return false; + } bytes_per_scanline = image_width * bytes_per_pixel; convert_to_bytes(fdata); diff --git a/src/TheNextWeek/rtw_stb_image.h b/src/TheNextWeek/rtw_stb_image.h index d92b06cfb..f356d3190 100644 --- a/src/TheNextWeek/rtw_stb_image.h +++ b/src/TheNextWeek/rtw_stb_image.h @@ -63,8 +63,13 @@ class rtw_image { // below, for the full height of the image. auto n = bytes_per_pixel; // Dummy out parameter: original components per pixel - float *fdata = stbi_loadf(filename.c_str(), &image_width, &image_height, &n, bytes_per_pixel); - if (fdata == nullptr) return false; + float *fdata = + stbi_loadf(filename.c_str(), &image_width, &image_height, &n, bytes_per_pixel); + + if (fdata == nullptr) { + image_width = image_height = 0; + return false; + } bytes_per_scanline = image_width * bytes_per_pixel; convert_to_bytes(fdata); diff --git a/src/TheRestOfYourLife/rtw_stb_image.h b/src/TheRestOfYourLife/rtw_stb_image.h index d92b06cfb..f356d3190 100644 --- a/src/TheRestOfYourLife/rtw_stb_image.h +++ b/src/TheRestOfYourLife/rtw_stb_image.h @@ -63,8 +63,13 @@ class rtw_image { // below, for the full height of the image. auto n = bytes_per_pixel; // Dummy out parameter: original components per pixel - float *fdata = stbi_loadf(filename.c_str(), &image_width, &image_height, &n, bytes_per_pixel); - if (fdata == nullptr) return false; + float *fdata = + stbi_loadf(filename.c_str(), &image_width, &image_height, &n, bytes_per_pixel); + + if (fdata == nullptr) { + image_width = image_height = 0; + return false; + } bytes_per_scanline = image_width * bytes_per_pixel; convert_to_bytes(fdata);