couldn't you make them into

namespace morton
{
namespace impl
{
template<uint16_t Dims, typename T>
struct mask;

// now the partial specializations
}
}

with the masks as NBL_CONSTEXPR member variables

This way you can have

template<uint16_t Dims, typename T, uint16_t BitDepth>
enable_if_t<is_scalar_v<T>&&is_integral_v<T>,T> decode(T x)
{
   static_assert(BitDepth <= sizeof(T)*8);

   x = x & mask<Dims,T>::value[0];
   
   .. rest of if-else statements
}

@Fletterio will take over

imho these should be template<uint16_t Dims, typename T, uint16_t BitDepth=sizeof(T)*8>

because that way you only need to write the encode and decode once

template<uint16_t Dims, typename T, uint16_t BitDepth=sizeof(T)*8>
vector<T,Dims> decode(const T _morton)
{
    vector<T,Dims> retval;
    for (uint16_t i=0; i<Dims; i++)
       retval[i] = impl::decode<Dims,T,BitDepth>(_morton>>i);
    return retval;
}

template<uint16_t Dims, typename T, uint16_t BitDepth=sizeof(T)*8>
T encode(const vector<T,Dims> coord)
{
    T retval = impl::separate_bits<Dims,T,BitDepth>(coord[0]);
    for (uint16_t i=1; i<Dims; i++)
       retval |= impl::separate_bits<Dims,T,BitDepth>(coord[1])<<i;
    return retval;
}

@Fletterio will take over

typo, you've set the min and max equal to each other

P.S. its also more useful to take a precomputed minLumaLog2 and lumaLog2Range (diff between log of max and log of min)

still outstanding for the geom meter

why max you already clamped!

btw if you have the log2 already precomputed then you can do

return min(spirv::nMax(log2(luma),lumaMinLog2),lumaMaxLog2);

nMin is a special SPIR-V version of min that will return the other operand if another is NaN (which happens on log of negative value or 0)

Fixed

luma should be called lumaLog2

Fixed

its somewhat semantically cleaner to pick the first, instead of last, esp since its a reduction you performed before

Fixed

don't give bad names to variables, this needs to be called workGroupIndex

this variable shouldn't even exist, because the MEAN meter didn't output to different address per X Y workgroup coordinate

Fixed

you're computing the wrong thing, every workgroup gets the same index 🤦

Also the original code was touching the same address with every for the MEAN meter mode

take the workgroup count and workgroup XYZ coordinate (or workgroup index) from outside (as function arguments) otherwise in the presence of solutions such as virtual workgroups or persistent threads, this whole thing will fall apart

take the workgroup count and workgroup XYZ coordinate (or workgroup index) from outside (as function arguments) otherwise in the presence of solutions such as virtual workgroups or persistent threads, this whole thing will fall apart

should already be precomputed as members

lets write some docs for this....

The val was produced by a workgroup reduction is performed of values in the [MinLog2,MaxLog2] range

Which makes the scaledLogLuma (the variable that should hold (val-minLog2)*rangeLog2) is between 0 and WorkGroupSize

This value is atomic added by N workgroups

You now want to represent it in Fixed Point during the atomic add, but not be vulnerable to overflow, this means the worst case is adding N times WorkGroupSize.

This means that we need to multiply the by (2^32-1)/N precomputed as a float or if you must round up N to PoT and see how many bits are left (512 workgroups, means 9 bits, so 23 are left). To avoid rounding precision errors, the PoT method is chosen.

I have no clue where you're getting +SubgroupSizeLog2 from.

Also the value of (1<<fixedPointBitsLeft)-1 must be precomputed in create and stored as a member

IT should be as easy as

const uint32_t scaledLumaLog2BitPattern = uint32_t((val-lumaMinLog2)*maxIncrement_over_lumaRangeLog2+float_t(0.5));

where maxIncrement = (0x1u<<(32u-uint32_t(ceil(log2(WorkGroupCount*WorkGroupSize)))))-1;

no, always the same address should be added to, if you wanted to stagger, then you should stagger based on modulo of the workgroup index

again, you're getting random floats based on workgroup index which thankfully was always the same (rare case of two wrongs making a right)

Again if you wanted to stagger, you should use entire subgroup to load the values, then subgroup reduce them

just converting to float_t is not the correct way to decode, you should divide by the maxIncrement

use totalSamples * percentile and totalSamples can be histo[BinCount-1]

lowerPercentile is in bin units while histo prefix sums are in samples, you compare different units hence the threshold is tiny and the median collapses to the first bins, your EV moves toward log2(lumaMin) and tonemapping is biased dark

counterexample https://godbolt.org/z/49WjcazMr