Repository: google/filament
File: filament/backend/src/vulkan/VulkanStagePool.h (C/C++)

Code before:
          mMapping(mapping) {}

    class Block {
    public:
        Block(VulkanStage* parentStage, uint32_t capacity, uint32_t offset)
            : mParentStage(parentStage),
              mCapacity(capacity),
              mOffset(offset) {
            mParentStage->mStageBlocks.insert({ mOffset, this });
        }

        ~Block() { mParentStage->mStageBlocks.erase(offset()); }

        // Should not be copying this around.
        Block(const Block& other) = delete;
        Block(Block&& other) = delete;
        Block& operator=(const Block& other) = delete;
        Block& operator=(Block&& other) = delete;

        inline VulkanStage* parentStage() const { return mParentStage; }
        inline VkBuffer buffer() const { return parentStage()->buffer(); }
        inline VmaAllocation memory() const { return parentStage()->memory(); }
        inline uint32_t capacity() const { return mCapacity; }
        inline uint32_t offset() const { return mOffset; }

        inline void* mapping() {
            return reinterpret_cast<void*>(
                    reinterpret_cast<char*>(mParentStage->mapping()) + offset());
        }

    private:
        VulkanStage* const mParentStage;
        const uint32_t mCapacity;
        const uint32_t mOffset;
    };

    inline VmaAllocation memory() const { return mMemory; }
    inline VkBuffer buffer() const { return mBuffer; }
    inline uint32_t capacity() const { return mCapacity; }
    inline void* mapping() const { return mMapping; }

    inline uint32_t& currentOffset() { return mCurrentOffset; }

    inline bool isSafeToReset() const { return mStageBlocks.empty(); }

    inline void reset() { mCurrentOffset = 0; }

    inline std::unique_ptr<Block> acquireBlock(uint32_t numBytes) {
        auto block = std::make_unique<Block>(this, numBytes, currentOffset());
        currentOffset() += numBytes;
        return block;

Review comment (bug):
This is a neat pick but mapping to a uint8_t* ptr might be better (we don't need to cast the stage mapping to char*) and since your use mapping to std::memcpy you do not need to cast down to void* (I could be wrong on this?) but it makes the overall close clearner to my eye.

Code after:
        : mMemory(memory),
          mBuffer(buffer),
          mCapacity(capacity),
          mMapping(mapping) {}

    ~VulkanStage() = default;
    VulkanStage(const VulkanStage& other) = delete;
    VulkanStage(VulkanStage&& other) = delete;
    VulkanStage& operator=(const VulkanStage& other) = delete;
    VulkanStage& operator=(VulkanStage&& other) = delete;

    class Segment : public fvkmemory::Resource {
    public:
        using OnRecycle = std::function<void(uint32_t offset)>;

        Segment(VulkanStage* parentStage, uint32_t capacity, uint32_t offset,
                OnRecycle&& onRecycleFn)
            : mParentStage(parentStage),
              mCapacity(capacity),
              mOffset(offset),
              mOnRecycleFn(onRecycleFn) {}

        ~Segment() {
            if (mOnRecycleFn) {
                mOnRecycleFn(offset());
            }
        }

        // Should not be copying this around.
        Segment(const Segment& other) = delete;
        Segment(Segment&& other) = delete;
        Segment& operator=(const Segment& other) = delete;
        Segment& operator=(Segment&& other) = delete;

        inline VulkanStage* parentStage() const { return mParentStage; }
        inline VkBuffer buffer() const { return parentStage()->buffer(); }
        inline VmaAllocation memory() const { return parentStage()->memory(); }
        inline uint32_t capacity() const { return mCapacity; }
        inline uint32_t offset() const { return mOffset; }

        inline void* mapping() const {
            return reinterpret_cast<void*>(
                    reinterpret_cast<char*>(mParentStage->mapping()) + offset());
        }

    private:
        // Ensure parent class can access the terminate method.
        friend class VulkanStage;

        VulkanStage* const mParentStage;
        const uint32_t mCapacity;