#5584: Simplify things a bit, get rid of the bit-shifting

codereader · Nov 7, 2021 · 888359b · 888359b
1 parent db2f11e
commit 888359b
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 50 deletions.
diff --git a/include/iwindingrenderer.h b/include/iwindingrenderer.h
@@ -25,7 +25,7 @@ class IWindingRenderer
     virtual ~IWindingRenderer() {}
 
     using Slot = std::uint64_t;
-    static constexpr Slot InvalidSlot = std::numeric_limits<std::size_t>::max();
+    static constexpr Slot InvalidSlot = std::numeric_limits<Slot>::max();
 
     // Allocate a slot to hold the vertex data of a winding of the given size
     // Returns the handle which can be used to update or deallocate the data later

diff --git a/libs/render/WindingRenderer.h b/libs/render/WindingRenderer.h
@@ -30,8 +30,8 @@ class WindingRenderer :
     };
 
     std::vector<SlotMapping> _slots;
-    static constexpr std::uint32_t InvalidSlotMapping = std::numeric_limits<std::uint32_t>::max();
-    std::uint32_t _freeSlotMappingHint;
+    static constexpr std::size_t InvalidSlotMapping = std::numeric_limits<std::size_t>::max();
+    std::size_t _freeSlotMappingHint;
 
     std::size_t _windings;
 
@@ -62,58 +62,63 @@ class WindingRenderer :
         auto slotMappingIndex = getNextFreeSlotMapping();
 
         auto& slotMapping = _slots[slotMappingIndex];
-        slotMapping.occupied = true;
+        slotMapping.bucketIndex = bucketIndex;
         slotMapping.slotNumber = bufferSlot;
 
         ++_windings;
 
-        return getSlot(bucketIndex, slotMappingIndex);
+        return slotMappingIndex;
     }
 
     void removeWinding(Slot slot) override
     {
-        // Get the bucket and mapping indices from the encoded Slot
-        auto bucket = getBucketIndex(slot);
-        auto slotMappingIndex = getSlotMappingIndex(slot);
+        assert(slot < _slots.size());
+        auto& slotMapping = _slots[slot];
 
-        auto& slotMapping = _slots[slotMappingIndex];
-
-        assert(slotMapping.occupied);
+        auto bucketIndex = slotMapping.bucketIndex;
+        assert(bucketIndex != InvalidBucketIndex);
 
         // Remove the winding from the bucket
-        _buckets[bucket].removeWinding(slotMapping.slotNumber);
-
-        // Invalidate the slot mapping
-        slotMapping.occupied = false;
-        slotMapping.slotNumber = InvalidSlotMapping;
+        _buckets[bucketIndex].removeWinding(slotMapping.slotNumber);
 
         // Update the value in other slot mappings, now that the bucket shrunk
+        for (auto& mapping : _slots)
+        {
+            // Every index in the same bucket beyond the removed winding needs to be shifted to left
+            if (mapping.bucketIndex == bucketIndex && mapping.slotNumber > slotMapping.slotNumber)
+            {
+                --mapping.slotNumber;
+            }
+        }
 
+        // Invalidate the slot mapping
+        slotMapping.bucketIndex = InvalidBucketIndex;
+        slotMapping.slotNumber = InvalidVertexBufferSlot;
 
-        // Update the free slot hint
-        if (slotMappingIndex < _freeSlotMappingHint)
+        // Update the free slot hint, for the next round we allocate one
+        if (slot < _freeSlotMappingHint)
         {
-            _freeSlotMappingHint = slotMappingIndex;
+            _freeSlotMappingHint = slot;
         }
 
         --_windings;
     }
 
     void updateWinding(Slot slot, const std::vector<ArbitraryMeshVertex>& vertices) override
     {
-        auto bucketIndex = getBucketIndex(slot);
-        auto slotMappingIndex = getSlotMappingIndex(slot);
+        assert(slot < _slots.size());
+        auto& slotMapping = _slots[slot];
+
+        assert(slotMapping.bucketIndex != InvalidBucketIndex);
 
-        auto& bucket = _buckets[bucketIndex];
+        auto& bucket = _buckets[slotMapping.bucketIndex];
 
         if (bucket.getWindingSize() != vertices.size())
         {
             throw std::logic_error("Winding size changes are not supported through updateWinding.");
         }
 
-        assert(_slots[slotMappingIndex].occupied);
-        auto bufferSlot = _slots[slotMappingIndex].slotNumber;
-        bucket.replaceWinding(bufferSlot, vertices);
+        bucket.replaceWinding(slotMapping.slotNumber, vertices);
     }
 
     void render()
@@ -139,13 +144,13 @@ class WindingRenderer :
     }
 
 private:
-    std::uint32_t getNextFreeSlotMapping()
+    IWindingRenderer::Slot getNextFreeSlotMapping()
     {
-        auto numSlots = static_cast<std::uint32_t>(_slots.size());
+        auto numSlots = static_cast<IWindingRenderer::Slot>(_slots.size());
 
         for (auto i = _freeSlotMappingHint; i < numSlots; ++i)
         {
-            if (!_slots[i].occupied)
+            if (_slots[i].bucketIndex != InvalidBucketIndex)
             {
                 _freeSlotMappingHint = i + 1; // start searching here next time
                 return i;
@@ -156,28 +161,6 @@ class WindingRenderer :
         return numSlots; // == the size before we emplaced the new slot
     }
 
-    inline static BucketIndex getBucketIndex(IWindingRenderer::Slot slot)
-    {
-        // Highest part of the slot stores the bucket index
-        // The lower bytes are the index within the bucket
-        constexpr auto BitsToShiftRight = (sizeof(IWindingRenderer::Slot) - sizeof(BucketIndex)) << 3;
-
-        return slot >> BitsToShiftRight;
-    }
-
-    inline static std::uint32_t getSlotMappingIndex(IWindingRenderer::Slot slot)
-    {
-        constexpr auto BitsToShiftLeft = (sizeof(IWindingRenderer::Slot) - sizeof(BucketIndex)) << 3;
-        constexpr auto SlotMappingMask = ~(static_cast<IWindingRenderer::Slot>(std::numeric_limits<BucketIndex>::max()) << BitsToShiftLeft);
-        return static_cast<std::uint32_t>(slot & SlotMappingMask);
-    }
-
-    inline static IWindingRenderer::Slot getSlot(BucketIndex bucketIndex, std::uint32_t slotMappingIndex)
-    {
-        constexpr auto BitsToShiftLeft = (sizeof(IWindingRenderer::Slot) - sizeof(BucketIndex)) << 3;
-        return (static_cast<IWindingRenderer::Slot>(bucketIndex) << BitsToShiftLeft) + slotMappingIndex;
-    }
-
     inline static BucketIndex getBucketIndexForWindingSize(std::size_t windingSize)
     {
         // Since there are no windings with sizes 0, 1, 2, we can deduct 3 to get the bucket index