From 745bd60b2e9a006d7ea5f332f07671d81448f324 Mon Sep 17 00:00:00 2001 From: Stefan Werner Date: Wed, 19 Nov 2025 11:23:26 +0100 Subject: [PATCH 1/7] Reduced copies, using move semantics in more places --- tutorials/common/scenegraph/materials.h | 10 +++++----- tutorials/common/scenegraph/obj_loader.cpp | 4 ++-- tutorials/common/scenegraph/ply_loader.cpp | 6 +++--- tutorials/common/scenegraph/scenegraph.h | 2 +- tutorials/common/scenegraph/texture.cpp | 2 +- tutorials/common/scenegraph/texture.h | 2 +- tutorials/common/scenegraph/xml_loader.cpp | 12 ++++++------ tutorials/external/catch.hpp | 2 +- 8 files changed, 20 insertions(+), 20 deletions(-) diff --git a/tutorials/common/scenegraph/materials.h b/tutorials/common/scenegraph/materials.h index f966096dd2..335cba8aa8 100644 --- a/tutorials/common/scenegraph/materials.h +++ b/tutorials/common/scenegraph/materials.h @@ -109,11 +109,11 @@ namespace embree map_d(nullptr), map_Ka(nullptr), map_Kd(nullptr), map_Ks(nullptr), map_Kt(nullptr), map_Ns(nullptr), map_Displ(nullptr) {} OBJMaterial (float d, const std::shared_ptr map_d, - const Vec3fa& Ka, const std::shared_ptr map_Ka, - const Vec3fa& Kd, const std::shared_ptr map_Kd, - const Vec3fa& Ks, const std::shared_ptr map_Ks, - const Vec3fa& Kt, const std::shared_ptr map_Kt, - const float Ns, const std::shared_ptr map_Ns, + const Vec3fa& Ka, const std::shared_ptr &map_Ka, + const Vec3fa& Kd, const std::shared_ptr &map_Kd, + const Vec3fa& Ks, const std::shared_ptr &map_Ks, + const Vec3fa& Kt, const std::shared_ptr &map_Kt, + const float Ns, const std::shared_ptr &map_Ns, const std::shared_ptr map_Displ) : base(MATERIAL_OBJ), illum(0), d(d), Ns(Ns), Ni(1.f), Ka(Ka), Kd(Kd), Ks(Ks), Kt(Kt), map_d(nullptr), map_Ka(nullptr), map_Kd(nullptr), map_Ks(nullptr), map_Kt(nullptr), map_Ns(nullptr), map_Displ(nullptr), diff --git a/tutorials/common/scenegraph/obj_loader.cpp b/tutorials/common/scenegraph/obj_loader.cpp index f23f33f1b4..17deb2496a 100644 --- a/tutorials/common/scenegraph/obj_loader.cpp +++ b/tutorials/common/scenegraph/obj_loader.cpp @@ -199,7 +199,7 @@ namespace embree face.push_back(vtx); parseSepOpt(token); } - curGroup.push_back(face); + curGroup.emplace_back(face); continue; } @@ -230,7 +230,7 @@ namespace embree hair[3*i+0].w = r; if (i != N) hair[3*i+1].w = r; } - curGroupHair.push_back(hair); + curGroupHair.emplace_back(hair); } /*! parse edge crease */ diff --git a/tutorials/common/scenegraph/ply_loader.cpp b/tutorials/common/scenegraph/ply_loader.cpp index be31c81140..20d18c3430 100644 --- a/tutorials/common/scenegraph/ply_loader.cpp +++ b/tutorials/common/scenegraph/ply_loader.cpp @@ -118,7 +118,7 @@ namespace embree if (line == "end_header") break; if (line.find_first_of('#') == 0) continue; if (line == "") continue; - header.push_back(line); + header.emplace_back(line); } /* parse header */ @@ -191,7 +191,7 @@ namespace embree Type ty = parseType(line); std::string name; line >> name; elt.type[name] = ty; - elt.properties.push_back(name); + elt.properties.emplace_back(name); } mesh.elements[name] = elt; @@ -267,7 +267,7 @@ namespace embree std::vector lst; size_t num = loadInteger(index_ty); for (size_t i=0; i>&& lights, BBox1f time_range) - : lights(lights), time_range(time_range) {} + : lights(std::move(lights)), time_range(time_range) {} virtual LightType getType() const { return lights[0]->getType(); diff --git a/tutorials/common/scenegraph/texture.cpp b/tutorials/common/scenegraph/texture.cpp index 9c1302aa93..d7706ef069 100644 --- a/tutorials/common/scenegraph/texture.cpp +++ b/tutorials/common/scenegraph/texture.cpp @@ -21,7 +21,7 @@ namespace embree Texture::Texture () : width(-1), height(-1), format(INVALID), bytesPerTexel(0), width_mask(0), height_mask(0), data(nullptr) {} - Texture::Texture(Ref img, const std::string fileName) + Texture::Texture(Ref img, const std::string &&fileName) : width(unsigned(img->width)), height(unsigned(img->height)), format(RGBA8), bytesPerTexel(4), width_mask(0), height_mask(0), data(nullptr), fileName(fileName) { width_mask = isPowerOf2(width) ? width-1 : 0; diff --git a/tutorials/common/scenegraph/texture.h b/tutorials/common/scenegraph/texture.h index 4f3e71da97..092359fe54 100644 --- a/tutorials/common/scenegraph/texture.h +++ b/tutorials/common/scenegraph/texture.h @@ -41,7 +41,7 @@ namespace embree public: Texture (); - Texture (Ref image, const std::string fileName); + Texture (Ref image, const std::string &&fileName); Texture (unsigned width, unsigned height, const Format format, const char* in = nullptr); ~Texture (); diff --git a/tutorials/common/scenegraph/xml_loader.cpp b/tutorials/common/scenegraph/xml_loader.cpp index af0ac1cc2f..479844aee8 100644 --- a/tutorials/common/scenegraph/xml_loader.cpp +++ b/tutorials/common/scenegraph/xml_loader.cpp @@ -77,7 +77,7 @@ namespace embree Variant (const std::string& str) : type(STRING), str(str) {} /*! Constructs a variant object holding a texture value. */ - Variant (const std::shared_ptr tex) : type(TEXTURE), texture(tex) {} + Variant (const std::shared_ptr tex) : type(TEXTURE), texture(std::move(tex)) {} /*! Extracts a boolean from the variant type. */ bool getBool () const { return b[0]; } @@ -1371,7 +1371,7 @@ namespace embree data[i++] = Vec3fa(0, r, 0); data[i++] = Vec3fa(0, -r, 0); - mesh->positions.push_back(data); + mesh->positions.emplace_back(data); } { @@ -1500,10 +1500,10 @@ namespace embree } } - hairs->positions.push_back(pos); - if (is_normaloriented) hairs->normals.push_back(norm); - if (is_hermite) hairs->tangents.push_back(tans); - if (is_hermite && is_normaloriented) hairs->dnormals.push_back(dnorm); + hairs->positions.emplace_back(pos); + if (is_normaloriented) hairs->normals.emplace_back(norm); + if (is_hermite) hairs->tangents.emplace_back(tans); + if (is_hermite && is_normaloriented) hairs->dnormals.emplace_back(dnorm); // linear uses 3 segments per hair if (is_linear) { diff --git a/tutorials/external/catch.hpp b/tutorials/external/catch.hpp index 8c4cfe478c..05afd4535d 100644 --- a/tutorials/external/catch.hpp +++ b/tutorials/external/catch.hpp @@ -4635,7 +4635,7 @@ namespace Catch { TestSpec::PatternPtr pattern = std::make_shared( token ); if( m_exclusion ) pattern = std::make_shared( pattern ); - m_currentFilter.m_patterns.push_back( pattern ); + m_currentFilter.m_patterns.emplace_back( pattern ); } m_exclusion = false; m_mode = None; From 9d20216e259a2d3da36c139cd8a192995be91ef5 Mon Sep 17 00:00:00 2001 From: Stefan Werner Date: Thu, 20 Nov 2025 12:14:25 +0100 Subject: [PATCH 2/7] Fixed integer over/underflows --- kernels/builders/primrefgen_presplit.h | 2 +- kernels/subdiv/half_edge.h | 1 + tutorials/verify/verify.cpp | 15 ++++++++------- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/kernels/builders/primrefgen_presplit.h b/kernels/builders/primrefgen_presplit.h index db9010995d..72688f11aa 100644 --- a/kernels/builders/primrefgen_presplit.h +++ b/kernels/builders/primrefgen_presplit.h @@ -116,7 +116,7 @@ namespace embree const float area_aabb = area(ref.bounds()); const float area_prim = primitiveArea(ref); if (area_prim == 0.0f) return 0.0f; - const unsigned int diff = 31 - lzcnt(mc.x^mc.y); + const int diff = 31 - lzcnt(mc.x^mc.y); //assert(area_prim <= area_aabb); // may trigger due to numerical issues const float area_diff = max(0.0f, area_aabb - area_prim); //const float priority = powf(area_diff * powf(PRIORITY_SPLIT_POS_WEIGHT,(float)diff),1.0f/4.0f); diff --git a/kernels/subdiv/half_edge.h b/kernels/subdiv/half_edge.h index baf019cd79..8cb4f24845 100644 --- a/kernels/subdiv/half_edge.h +++ b/kernels/subdiv/half_edge.h @@ -316,6 +316,7 @@ namespace embree const Vec3fa v = vertices[p->getStartVertexIndex()]; if (!isvalid(v)) return false; } + if (n < 2) return false; N += n-2; return n >= 3 && n <= MAX_PATCH_VALENCE; } diff --git a/tutorials/verify/verify.cpp b/tutorials/verify/verify.cpp index 1839da439d..cb1edc00aa 100644 --- a/tutorials/verify/verify.cpp +++ b/tutorials/verify/verify.cpp @@ -7074,15 +7074,16 @@ namespace embree plot << "set title \"" << outFileName.name() << "\"" << std::endl; plot << "set xlabel \"" + xlabel + "\"" << std::endl; if (f != 1.0f) plot << "set logscale x" << std::endl; - if (benchmarks.size()) + if (benchmarks.size()) { plot << "set ylabel \"" << benchmarks[0]->unit << "\"" << std::endl; - plot << "set yrange [0:]" << std::endl; + plot << "set yrange [0:]" << std::endl; - plot << "plot \\" << std::endl; - for (size_t i=0; iname << ".txt\" using 1:2 title \"" << benchmarks[i]->name << "\" with lines"; - if (i != benchmarks.size()-1) plot << ",\\"; - plot << std::endl; + plot << "plot \\" << std::endl; + for (size_t i=0; iname << ".txt\" using 1:2 title \"" << benchmarks[i]->name << "\" with lines"; + if (i != benchmarks.size()-1) plot << ",\\"; + plot << std::endl; + } } plot << std::endl; plot.close(); From ceaa0ced02e2959977aad2a28b345d8907a68fd9 Mon Sep 17 00:00:00 2001 From: Stefan Werner Date: Thu, 11 Dec 2025 16:01:40 +0100 Subject: [PATCH 3/7] Checking for potential divide by zero --- kernels/builders/primrefgen_presplit.h | 5 ++++- tutorials/buildbench/buildbench_device.cpp | 15 ++++++++++----- tutorials/common/tutorial/benchmark_render.h | 2 +- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/kernels/builders/primrefgen_presplit.h b/kernels/builders/primrefgen_presplit.h index 72688f11aa..a63371235d 100644 --- a/kernels/builders/primrefgen_presplit.h +++ b/kernels/builders/primrefgen_presplit.h @@ -287,7 +287,10 @@ namespace embree },[](const float& a, const float& b) -> float { return a+b; }); /* compute number of splits per primitive */ - const float inv_psum = 1.0f / psum; + float inv_psum = 1.0f; + if (psum > 0.0) { + inv_psum = 1.0f / psum; + } parallel_for( size_t(0), numPrimitives, size_t(MIN_STEP_SIZE), [&](const range& r) -> void { for (size_t i=r.begin(); i 0.0) { + std::cout << primitives << " primitives, " << objects << " objects, " + << time/iterations << " s, " + << 1.0 / (time/iterations) * primitives / 1000000.0 << " Mprims/s" << std::endl; + } rtcReleaseScene(helper.scene); } diff --git a/tutorials/common/tutorial/benchmark_render.h b/tutorials/common/tutorial/benchmark_render.h index 54a2ce2457..4b56f98d68 100644 --- a/tutorials/common/tutorial/benchmark_render.h +++ b/tutorials/common/tutorial/benchmark_render.h @@ -109,7 +109,7 @@ static void renderBenchmarkLegacy(BenchState& state, BenchParams& params ,int ar if (numTotalFrames >= 1024 && (i % 64 == 0)) { double rate = 0; - if (fpsStat.getAvg()) rate = 100.0f*fpsStat.getSigma()/fpsStat.getAvg(); + if (fpsStat.getAvg() != 0.0) rate = 100.0f*fpsStat.getSigma()/fpsStat.getAvg(); std::cout << "frame [" << std::setw(3) << i << " / " << std::setw(3) << numTotalFrames << "]: " << std::setw(8) << fps << " fps, " From 1cd0c8671b3b669f7f111a547c5021780acfa626 Mon Sep 17 00:00:00 2001 From: Stefan Werner Date: Mon, 15 Dec 2025 12:55:52 +0100 Subject: [PATCH 4/7] Initializing HitList in next hit tutorial before use --- tutorials/next_hit/next_hit_device.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tutorials/next_hit/next_hit_device.cpp b/tutorials/next_hit/next_hit_device.cpp index b723525f2a..c45ff6f6e8 100644 --- a/tutorials/next_hit/next_hit_device.cpp +++ b/tutorials/next_hit/next_hit_device.cpp @@ -26,12 +26,13 @@ RTCFeatureFlags g_feature_mask; struct HitList { HitList (const TutorialData& data) - : data(data), begin(0), end(0) {} + : data(data), begin(0), end(0) { + } /* Hit structure that defines complete order over hits */ struct Hit { - Hit() {} + Hit() : opaque(false), t(0.0f), primID(RTC_INVALID_GEOMETRY_ID), geomID(RTC_INVALID_GEOMETRY_ID), instID(RTC_INVALID_GEOMETRY_ID) {} Hit (bool opaque, float t, unsigned int primID = 0xFFFFFFFF, unsigned int geomID = 0xFFFFFFFF, unsigned int instID = 0xFFFFFFFF) : opaque(opaque), t(t), primID(primID), geomID(geomID), instID(instID) {} From 1130b992e4d3a07623e663ff64d9e773ab1ac02e Mon Sep 17 00:00:00 2001 From: Stefan Werner Date: Thu, 20 Nov 2025 12:14:55 +0100 Subject: [PATCH 5/7] Fixed potentially uninitialized variables --- tutorials/next_hit/next_hit_device.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tutorials/next_hit/next_hit_device.cpp b/tutorials/next_hit/next_hit_device.cpp index c45ff6f6e8..0978c6bb6b 100644 --- a/tutorials/next_hit/next_hit_device.cpp +++ b/tutorials/next_hit/next_hit_device.cpp @@ -318,6 +318,10 @@ Vec3ff renderPixelStandard(const TutorialData& data, float x, float y, /* initialize ray */ Ray ray(Vec3fa(camera.xfm.p), Vec3fa(normalize(x*camera.xfm.l.vx + y*camera.xfm.l.vy + camera.xfm.l.vz)), 0.0f, inf, 0.0f); + ray.Ng = {0.0f, 1.0f, 0.0f}; + ray.flags = 0; + ray.id = 0; + ray.u = ray.v = 0.0f; /* either gather hits in single pass or using multiple passes */ HitList hits(data); From dc7fb23438d11921608b1cb207f851fb6a0fa3b5 Mon Sep 17 00:00:00 2001 From: Stefan Werner Date: Tue, 16 Dec 2025 12:21:46 +0100 Subject: [PATCH 6/7] Updated TinyEXR to fix resource leaks --- tutorials/common/image/tinyexr.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tutorials/common/image/tinyexr.h b/tutorials/common/image/tinyexr.h index a3aa50db54..54fa1b5644 100644 --- a/tutorials/common/image/tinyexr.h +++ b/tutorials/common/image/tinyexr.h @@ -6717,20 +6717,22 @@ int LoadEXRFromMemory(float **out_rgba, int *width, int *height, if (idxR == -1) { tinyexr::SetErrorMessage("R channel not found", err); - - // @todo { free exr_image } + FreeEXRHeader(&exr_header); + FreeEXRImage(&exr_image); return TINYEXR_ERROR_INVALID_DATA; } if (idxG == -1) { tinyexr::SetErrorMessage("G channel not found", err); - // @todo { free exr_image } + FreeEXRHeader(&exr_header); + FreeEXRImage(&exr_image); return TINYEXR_ERROR_INVALID_DATA; } if (idxB == -1) { tinyexr::SetErrorMessage("B channel not found", err); - // @todo { free exr_image } + FreeEXRHeader(&exr_header); + FreeEXRImage(&exr_image); return TINYEXR_ERROR_INVALID_DATA; } From d6d34de2792e089810390989ffa45e84fad16d13 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 19 Dec 2025 17:28:36 +0000 Subject: [PATCH 7/7] Fix move semantics for Texture constructor fileName parameter Co-authored-by: stefanatwork <93931354+stefanatwork@users.noreply.github.com> --- tutorials/common/scenegraph/texture.cpp | 4 ++-- tutorials/common/scenegraph/texture.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tutorials/common/scenegraph/texture.cpp b/tutorials/common/scenegraph/texture.cpp index d7706ef069..d2f25810ab 100644 --- a/tutorials/common/scenegraph/texture.cpp +++ b/tutorials/common/scenegraph/texture.cpp @@ -21,8 +21,8 @@ namespace embree Texture::Texture () : width(-1), height(-1), format(INVALID), bytesPerTexel(0), width_mask(0), height_mask(0), data(nullptr) {} - Texture::Texture(Ref img, const std::string &&fileName) - : width(unsigned(img->width)), height(unsigned(img->height)), format(RGBA8), bytesPerTexel(4), width_mask(0), height_mask(0), data(nullptr), fileName(fileName) + Texture::Texture(Ref img, std::string &&fileName) + : width(unsigned(img->width)), height(unsigned(img->height)), format(RGBA8), bytesPerTexel(4), width_mask(0), height_mask(0), data(nullptr), fileName(std::move(fileName)) { width_mask = isPowerOf2(width) ? width-1 : 0; height_mask = isPowerOf2(height) ? height-1 : 0; diff --git a/tutorials/common/scenegraph/texture.h b/tutorials/common/scenegraph/texture.h index 092359fe54..f53b97497a 100644 --- a/tutorials/common/scenegraph/texture.h +++ b/tutorials/common/scenegraph/texture.h @@ -41,7 +41,7 @@ namespace embree public: Texture (); - Texture (Ref image, const std::string &&fileName); + Texture (Ref image, std::string &&fileName); Texture (unsigned width, unsigned height, const Format format, const char* in = nullptr); ~Texture ();