Address path tracer merge leftover comments by keptsecret · Pull Request #991 · Devsh-Graphics-Programming/Nabla

keptsecret · 2026-01-22T08:05:01Z

Makes changes left from #966

…mek's remove_core_matrix branch, added create from mat3x3

…c cast to/from truncated quat

…ical triangle

…nstead

…tion

keptsecret · 2026-02-02T03:22:59Z

hlsl/matrix_utils/transformation_matrix_utils.hlsl removed because of https://github.com/Devsh-Graphics-Programming/Nabla/pull/966/files#r2618467323

include/nbl/builtin/hlsl/sampling/projected_spherical_triangle.hlsl

include/nbl/builtin/hlsl/bxdf/fresnel.hlsl

include/nbl/builtin/hlsl/sampling/projected_spherical_triangle.hlsl

devshgraphicsprogramming · 2026-02-12T16:50:16Z

include/nbl/builtin/hlsl/sampling/projected_spherical_triangle.hlsl

most parameters should be members #966 (comment)

Also #966 (comment)

for next sampling PR, to rework in line with concepts of #1001

devshgraphicsprogramming · 2026-02-12T16:51:19Z

include/nbl/builtin/hlsl/sampling/projected_spherical_triangle.hlsl

        return retval;
    }

    vector4_type computeBilinearPatch(const vector3_type receiverNormal, bool isBSDF)


why is this returning vector4_type instead of sampling::bilinear

ok sampling PR

devshgraphicsprogramming · 2026-02-12T16:52:18Z

include/nbl/builtin/hlsl/sampling/projected_spherical_triangle.hlsl

    static ProjectedSphericalTriangle<T> create(NBL_CONST_REF_ARG(shapes::SphericalTriangle<T>) tri)
    {
        ProjectedSphericalTriangle<T> retval;
        retval.tri = tri;
        return retval;
    }


remove, see #966 (comment)

ok sampling PR

devshgraphicsprogramming · 2026-02-12T16:53:31Z

include/nbl/builtin/hlsl/sampling/projected_spherical_triangle.hlsl

        vector3_type cos_vertices, sin_vertices;
-        const scalar_type solidAngle = tri.solidAngleOfTriangle(cos_vertices, sin_vertices, cos_a, cos_c, csc_b, csc_c);
+        const scalar_type solidAngle = tri.solidAngle(cos_vertices, sin_vertices);


should be precomputed and stored as members #966 (comment)

ok sampling PR

devshgraphicsprogramming · 2026-02-12T16:54:22Z

include/nbl/builtin/hlsl/sampling/projected_spherical_triangle.hlsl

again function with bazillion parameters that are really local members

ok sampling PR

devshgraphicsprogramming · 2026-02-24T14:57:26Z

include/nbl/builtin/hlsl/path_tracing/unidirectional.hlsl

+                // example only uses isotropic bxdfs
+                quotient_pdf_type bsdf_quotient_pdf = materialSystem.quotient_and_pdf(matID, nee_sample, interaction.isotropic, _cache.iso_cache);
+                neeContrib.quotient *= materialSystem.eval(matID, nee_sample, interaction.isotropic, _cache.iso_cache) * rcpChoiceProb;
+                const scalar_type otherGenOverLightAndChoice = bsdf_quotient_pdf.pdf * rcpChoiceProb / neeContrib.pdf;


pass full aniso interaction, let the material system deal with only using the isotropic part

Also your PDF is mismatched.

@keptsecret still using the wrong PDF to pair with eval

include/nbl/builtin/hlsl/path_tracing/unidirectional.hlsl

…n interaction instead of just isBSDF

devshgraphicsprogramming · 2026-02-25T15:17:35Z

include/nbl/builtin/hlsl/path_tracing/concepts.hlsl

+namespace impl
+{
+struct DummyInteraction {};
+}
+
 #define NBL_CONCEPT_NAME Ray
 #define NBL_CONCEPT_TPLT_PRM_KINDS (typename)
 #define NBL_CONCEPT_TPLT_PRM_NAMES (T)
 #define NBL_CONCEPT_PARAM_0 (ray, T)
 #define NBL_CONCEPT_PARAM_1 (v, typename T::vector3_type)
-#define NBL_CONCEPT_PARAM_2 (b, bool)
+#define NBL_CONCEPT_PARAM_2 (interaction, impl::DummyInteraction)


not sure how an empty struct tests any better than a real default aniso interaction

I didn't want to include the bxdf/common.hlsl in here.
But could change to surface_interaction::SIsotropic if that's better?

devshgraphicsprogramming · 2026-02-25T15:26:33Z

include/nbl/builtin/hlsl/path_tracing/concepts.hlsl

-    ((NBL_CONCEPT_REQ_EXPR_RET_TYPE)((scene.getNormal(id, intersectP)), ::nbl::hlsl::is_same_v, typename T::vector3_type))
+    ((NBL_CONCEPT_REQ_EXPR_RET_TYPE)((scene.template getInteraction<impl::DummyRay>(id, intersectP, ray)), ::nbl::hlsl::is_same_v, typename T::interaction_type))
 );
+#undef ray


why are you still adamant on the scene giving you an interaction and not the closestHit trace ray of the intersector !?

You do realize that in general thats not possible with a ray, intersection position and an object ID ?

(well or possible, but you'd need to re-intersect the BLAS just for that)

The scene already does give a closest hit ray. This is for filling out the interaction member of the closest hit return struct.
Regardless, I've now made the scene handle all of creating the closest hit return struct.

devshgraphicsprogramming · 2026-02-25T15:30:38Z

include/nbl/builtin/hlsl/path_tracing/unidirectional.hlsl

+            vector3_type direction = bxdfSample;
+            ray.init(origin, direction);
+            ray.template initInteraction<anisotropic_interaction_type>(interaction);
+            ray.setT(1.0/*kSceneSize*/);


huh? why 1.0 ?

t is used in the Tolerance::adjust function to control ray origin. It was also 1.0 in the glsl version but with the comment kSceneSize, I guess it should be that but not sure how to get it

devshgraphicsprogramming · 2026-02-25T15:32:25Z

include/nbl/builtin/hlsl/path_tracing/unidirectional.hlsl

+                quotient_pdf_type bsdf_quotient_pdf = materialSystem.quotient_and_pdf(matID, nee_sample, iso_interaction, _cache.iso_cache);
+                neeContrib.quotient *= materialSystem.eval(matID, nee_sample, iso_interaction, _cache.iso_cache) * rcpChoiceProb;


material system shall always take aniso interaction

devshgraphicsprogramming · 2026-02-25T15:32:33Z

include/nbl/builtin/hlsl/path_tracing/unidirectional.hlsl

-            quotient_pdf_type bsdf_quotient_pdf = materialSystem.quotient_and_pdf(matID, bsdf_sample, interaction.isotropic, _cache.iso_cache);
+            quotient_pdf_type bsdf_quotient_pdf = materialSystem.quotient_and_pdf(matID, bsdf_sample, iso_interaction, _cache.iso_cache);


material system shall always take aniso interaction

devshgraphicsprogramming · 2026-02-25T15:32:58Z

include/nbl/builtin/hlsl/path_tracing/unidirectional.hlsl

    {
        anisotropic_interaction_type interaction = intersectData.getInteraction();
-        isotropic_interaction_type iso_interaction = interaction.isotropic;
+        isotropic_interaction_type iso_interaction = interaction.getIsotropic();


in essence this should be an unused variable and removed

devshgraphicsprogramming · 2026-02-25T15:47:52Z

include/nbl/builtin/hlsl/path_tracing/unidirectional.hlsl

-        const vector3_type throughputCIE_Y = hlsl::normalize(spectralTypeToLumaCoeffs * throughput);
+        measure_type throughputCIE_Y = spectralTypeToLumaCoeffs * throughput;
+        {
+            scalar_type sum_throughput = throughputCIE_Y[0];
+            NBL_UNROLL for (uint16_t i = 1; i < vector_traits<measure_type>::Dimension; i++)
+                sum_throughput += throughputCIE_Y[i];
+            throughputCIE_Y /= sum_throughput;
+        }


cool, now this needs to in the interaction

devshgraphicsprogramming · 2026-02-25T15:53:17Z

include/nbl/builtin/hlsl/path_tracing/unidirectional.hlsl

does this update your thoroughput in the payload as well?

Then rename the function to updateThroughputAndMISWeights

devshgraphicsprogramming · 2026-02-25T15:55:02Z

include/nbl/builtin/hlsl/path_tracing/unidirectional.hlsl

            vector3_type direction = bxdfSample;
-            ray.initData(origin, direction, interaction.getN(), isBSDF);
+            ray.init(origin, direction);
+            ray.template initInteraction<anisotropic_interaction_type>(interaction);


setInteraction maybe would be a better name ?

devshgraphicsprogramming · 2026-02-25T15:56:01Z

include/nbl/builtin/hlsl/rwmc/SplattingParameters.hlsl

+    int32_t PackedRcpLog2BaseAndLog2BaseRoot;
+    float32_t BrightSampleLumaBias;


why do ou have two members now !?

Huh? It was always 2 members. Both combined unpack to 4 vars for SSplattingParameters, but since we removed one then it's 1 packed and 1 as is.

devshgraphicsprogramming · 2026-02-25T21:43:14Z

include/nbl/builtin/hlsl/path_tracing/unidirectional.hlsl

+                nee_ray.init(origin, direction);
+                nee_ray.template initInteraction<anisotropic_interaction_type>(interaction);
                nee_ray.setT(t);
                tolerance_method_type::template adjust<ray_type>(nee_ray, direction, depth);


isn't the direction already in the ray ?

you need to give it the geometric normal (not the interaction which has the shading normal) if you want to know why read the PBRT book, but offsetting the origin by geo normal keeps your t modification small, while offsetting along the ray can be potentially unbounded with a surface-grazing ray if you want to not self-intersect

devshgraphicsprogramming · 2026-02-25T21:44:41Z

include/nbl/builtin/hlsl/path_tracing/unidirectional.hlsl

-                rayAlive = closestHitProgram(d, sampleIndex, ray, intersection);
+            continuePath = intersection.foundHit();
+            if (continuePath)
+                continuePath &= closestHitProgram(d, sampleIndex, ray, intersection);


you don't even need a &= just a = will do, you're in the if statement, so its already true

devshgraphicsprogramming · 2026-02-25T22:07:36Z

include/nbl/builtin/hlsl/path_tracing/unidirectional.hlsl

-                materialSystem, scene, intersectP, interaction,
-                isBSDF, eps0, depth
+                scene, materialSystem, intersectP, interaction,
+                eps0, depth


you don't need depth

depth is used to adjust newRayMaxT inside the nee though?

…e internally to use iso

…rsected ray and gives intersection

…r sampleMeasure

keptsecret and others added 19 commits December 1, 2025 16:15

changed quaternion struct name, brought in quaternion stuff from prze…

9be65a0

…mek's remove_core_matrix branch, added create from mat3x3

_static_create quaternion to matrix, create quaternion from axis-angle

3e03467

added more quaternion funcs: transformVec, inverse, normalize + stati…

34d1385

…c cast to/from truncated quat

Modified LoadableImage and ResolveAccessorBase concepts

61540d6

Small RWMC fixes

6953879

changes to linear, bilinear, box muller for pdf and backward pdf

3d0c454

get polarized fresnel values by component

bea01f9

precompute some phase shift constants

5a36766

removed morton struct superseded by new one

d079f5b

changes to solid angle method name, simplified a lot of code in spher…

3f8fd48

…ical triangle

simplify call to iridescent helper through base struct

c06f199

removed redundant/unused variables from spherical triangle sample

407d0fd

spherical rectangle stores origin, extent, basis and takes observer i…

795b9fa

…nstead

added compressed spherical rectangle, comments for info of implementa…

f9b949e

…tion

removed unused code

140c5c6

path tracer (unidirectional) and concepts moved from example

2ce08d9

use correct quantized sequence decode

1a7b185

moved raygen out as basic ray gen with gaussian filter

3350351

merge master, fix conflicts

29910cf

keptsecret added 2 commits February 6, 2026 15:14

merge master, fix conflicts

a4a5a12

Merge branch 'master' into path_tracer_leftover_cleanup

ea38df1

devshgraphicsprogramming reviewed Feb 12, 2026

View reviewed changes

include/nbl/builtin/hlsl/sampling/projected_spherical_triangle.hlsl Outdated Show resolved Hide resolved

devshgraphicsprogramming reviewed Feb 12, 2026

View reviewed changes

include/nbl/builtin/hlsl/bxdf/fresnel.hlsl Show resolved Hide resolved

devshgraphicsprogramming reviewed Feb 12, 2026

View reviewed changes

include/nbl/builtin/hlsl/sampling/projected_spherical_triangle.hlsl Outdated Show resolved Hide resolved

devshgraphicsprogramming reviewed Feb 12, 2026

View reviewed changes

devshgraphicsprogramming reviewed Feb 24, 2026

View reviewed changes

include/nbl/builtin/hlsl/path_tracing/unidirectional.hlsl Outdated Show resolved Hide resolved

keptsecret added 6 commits February 25, 2026 11:05

scene returns an interaction, instead of just normal

4ceb3f2

minor fixes to spherical rectangle stuff

917acc5

remove unused variable in packed splatting params

4ea58c8

minor changes to concept for intersector, nee; document concept symbols

885e58f

changes to ray methods to separate interaction from init, nee takes a…

9aeb80c

…n interaction instead of just isBSDF

fix normalize throughputCIE, minor changes to method names

575f15b

devshgraphicsprogramming reviewed Feb 25, 2026

View reviewed changes

keptsecret added 9 commits February 26, 2026 12:31

use nbl this macro for fresnel iridescent that uses this

b5a4feb

added some asserts, material system should take aniso types and decid…

120e179

…e internally to use iso

correct pdf method for mat system, added shouldDoMIS for ray payload

5e243d9

added some comments regarding future use of MIS, scene takes and inte…

40d585c

…rsected ray and gives intersection

minor name changes of methods

8d8b4af

offset ray origin by geometric normal, not ray dir

82ee23c

path tracer doesn't store a ray gen, instead takes a generated ray fo…

f6987fb

…r sampleMeasure

env light id always 0, handle miss program like sampling env light

5922b38

Merge branch 'master' into path_tracer_leftover_cleanup

4319536

		quotient_pdf_type bsdf_quotient_pdf = materialSystem.quotient_and_pdf(matID, nee_sample, iso_interaction, _cache.iso_cache);
		neeContrib.quotient = materialSystem.eval(matID, nee_sample, iso_interaction, _cache.iso_cache) rcpChoiceProb;

		int32_t PackedRcpLog2BaseAndLog2BaseRoot;
		float32_t BrightSampleLumaBias;

Conversation

keptsecret commented Jan 22, 2026

Uh oh!

keptsecret commented Feb 2, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devshgraphicsprogramming Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devshgraphicsprogramming Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keptsecret Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devshgraphicsprogramming Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devshgraphicsprogramming Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

devshgraphicsprogramming Feb 24, 2026 •

edited

Loading

devshgraphicsprogramming Feb 24, 2026 •

edited

Loading

keptsecret Feb 27, 2026 •

edited

Loading

devshgraphicsprogramming Feb 25, 2026 •

edited

Loading

devshgraphicsprogramming Feb 25, 2026 •

edited

Loading