Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More fft utils #795

Merged
merged 45 commits into from
Jan 16, 2025
Merged

More fft utils #795

merged 45 commits into from
Jan 16, 2025

Conversation

Fletterio
Copy link
Contributor

Description

Still adding more changes on this side

Testing

All thesting done on the FFT_Bloom example on its respective branch

Comment on lines 16 to 28
static inline uint32_t3 padDimensions(uint32_t3 dimensions, std::span<uint16_t> axes, bool realFFT = false)
{
uint16_t axisCount = 0;
for (auto i : axes)
{
dimensions[i] = core::roundUpToPoT(dimensions[i]);
if (realFFT && !axisCount++)
dimensions[i] /= 2;
}
return dimensions;
}

static inline uint64_t getOutputBufferSize(const uint32_t3& inputDimensions, uint32_t numChannels, std::span<uint16_t> axes, bool realFFT = false, bool halfFloats = false)

Choose a reason for hiding this comment

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

you could have just replaced uint32_t3 with vector<uint32_t,M> and made it available both for C++ and HLSL

also why is span<uint16_t> used, you could just deduce the axis count from N ?

roundUpToPoT could move to hlsl namespace actually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

roundUpToPoT is in core/math/intutil.h. I can create a builtin/math/intutil.hlsl, copy most functions over and refactor every usage of the functions in that file

Choose a reason for hiding this comment

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

roundUpToPoT is in core/math/intutil.h. I can create a builtin/math/intutil.hlsl, copy most functions over and refactor every usage of the functions in that file

ask @Przemog1 about the preferred location so it fits in with the spirit of #801

You don't actually need to do big refactor if you provide a "legacy" alias (make the old nbl::core::roundUpToPoT call nbl::hlsl::roundUpToPoT) and add the [[deprecated]] attribute

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'm creating builtin/hlsl/math/intutil.hlsl, @Przemog1 lmk if you'd rather have it be named different or placed somewhere else

Copy link
Member

@devshgraphicsprogramming devshgraphicsprogramming left a comment

Choose a reason for hiding this comment

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

I hate GH reviews

Comment on lines 12 to 84
template<typename Integer NBL_FUNC_REQUIRES(is_integral_v<Integer>)
NBL_CONSTEXPR_FORCED_INLINE_FUNC bool isNPoT(Integer value)
{
return value & (value - Integer(1));
}

template<typename Integer NBL_FUNC_REQUIRES(is_integral_v<Integer>)
NBL_CONSTEXPR_FORCED_INLINE_FUNC bool isPoT(Integer value)
{
return !isNPoT<Integer>(value);
}


template<typename Integer NBL_FUNC_REQUIRES(is_integral_v<Integer>)
NBL_CONSTEXPR_FORCED_INLINE_FUNC Integer roundUpToPoT(Integer value)
{
return Integer(0x1u) << Integer(1 + hlsl::findMSB<Integer>(value - Integer(1))); // this wont result in constexpr because findMSB is not one
}

template<typename Integer NBL_FUNC_REQUIRES(is_integral_v<Integer>)
NBL_CONSTEXPR_FORCED_INLINE_FUNC Integer roundDownToPoT(Integer value)
{
return Integer(0x1u) << hlsl::findMSB<Integer>(value);
}

template<typename Integer NBL_FUNC_REQUIRES(is_integral_v<Integer>)
NBL_CONSTEXPR_FORCED_INLINE_FUNC Integer roundUp(Integer value, Integer multiple)
{
Integer tmp = (value + multiple - 1u) / multiple;
return tmp * multiple;
}

template<typename Integer NBL_FUNC_REQUIRES(is_integral_v<Integer>)
NBL_CONSTEXPR_FORCED_INLINE_FUNC Integer align(Integer alignment, Integer size, NBL_REF_ARG(Integer) address, NBL_REF_ARG(Integer) space)
{
Integer nextAlignedAddr = roundUp<Integer>(address, alignment);

Integer spaceDecrement = nextAlignedAddr - address;
if (spaceDecrement > space)
return 0u;

Integer newSpace = space - spaceDecrement;
if (size > newSpace)
return 0u;

space = newSpace;
return address = nextAlignedAddr;
}

#ifndef __HLSL_VERSION

// Have to wait for the HLSL patch for `is_enum`. Would also have to figure out how to do it without initializer lists for HLSL use.

//! Get bitmask from variadic arguments passed.
/*
For example if you were to create bitmask for vertex attributes
having positions inteeger set as 0, colors as 1 and normals
as 3, just pass them to it and use the value returned.
*/

template<typename BitmaskType NBL_FUNC_REQUIRES(is_integral_v<BitmaskType> || std::is_enum_v<BitmaskType>)
NBL_CONSTEXPR_FORCED_INLINE_FUNC uint64_t createBitmask(std::initializer_list<BitmaskType> initializer)
{
uint64_t retval{};
for (const auto& it : initializer)
retval |= (1ull << it);
return retval;
}

#endif

} // end namespace hlsl
} // end namespace nbl

Choose a reason for hiding this comment

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

very nice, I'll let @Przemog1 review, because he's also getting rid of the old round and isPoT in another PR #760 (or the one after)

@devshgraphicsprogramming devshgraphicsprogramming merged commit da80234 into master Jan 16, 2025
1 check failed
@devshgraphicsprogramming devshgraphicsprogramming deleted the more_fft_utils branch January 16, 2025 01:12
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.

2 participants