Skip to content

Conversation

@keptsecret
Copy link
Contributor

No description provided.

Comment on lines 65 to 66
template<class F, typename float_t, uint32_t Depth=6> // F has function __call(x)
struct AdaptiveSimpson
Copy link
Member

@devshgraphicsprogramming devshgraphicsprogramming Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. F should be required to have operator()(x) not __call (this is one is meant for statics
  2. make a concept for the above
  3. float_t you can default to decltype(declval<F>()(0))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also f should be NBL_ConST_REF_ARG and not NBL_REF_ARG (it doesn't make sense otherwise, you want there to be no inner mutable state because it can't be sensitive to order its getting called in)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree but the function we pass into this calls bxdf.pdf() which is not const (and bxdf is a member we store instead of referencing in a lambda)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the methods of a BxDF should most definitely be const

template<class F, typename float_t, uint32_t Depth=6> // F has function __call(x)
struct AdaptiveSimpson
{
static float_t __call(NBL_REF_ARG(F) f, float_t x0, float_t x1, float_t eps = 1e-6)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

documentation, what's eps used for ?

Comment on lines 19 to 31
static const float32_t3 RGB_LUT[256];

static float32_t3 map(float x)
{
x = hlsl::clamp(x, 0.f, 1.f);
int i = int(x * 255.f);
int j = hlsl::min(255, i+1);
float s = x * 255.f - float(i);
return RGB_LUT[i] + (RGB_LUT[j] - RGB_LUT[i]) * s;
}
};

inline const float32_t3 Turbo::RGB_LUT[256] = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the correct NBL_ macros for out of line constexpr array declarations and definitions

{
static const float32_t3 RGB_LUT[256];

static float32_t3 map(float x)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__call

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants