clean up partial occlusion test caching

TheCBProject · Mar 22, 2024 · 3d78536 · 3d78536
1 parent f831fc1
commit 3d78536
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 24 deletions.
diff --git a/src/main/java/codechicken/multipart/api/part/MultiPart.java b/src/main/java/codechicken/multipart/api/part/MultiPart.java
@@ -20,9 +20,12 @@
 import net.minecraft.world.entity.player.Player;
 import net.minecraft.world.item.ItemStack;
 import net.minecraft.world.item.context.UseOnContext;
+import net.minecraft.world.level.BlockGetter;
 import net.minecraft.world.level.Explosion;
 import net.minecraft.world.level.Level;
+import net.minecraft.world.level.block.Block;
 import net.minecraft.world.level.block.SoundType;
+import net.minecraft.world.level.block.state.BlockState;
 import net.minecraft.world.level.chunk.LevelChunk;
 import net.minecraft.world.phys.shapes.CollisionContext;
 import net.minecraft.world.phys.shapes.Shapes;
@@ -174,26 +177,56 @@ default boolean occlusionTest(MultiPart npart) {
         return true;
     }
 
+    /**
+     * Maps to {@link Block#getShape(BlockState, BlockGetter, BlockPos, CollisionContext)}.
+     * <p>
+     * If this shape changes after initial placement, call {@link TileMultipart#markShapeChange()}.
+     */
     default VoxelShape getShape(CollisionContext context) {
         return Shapes.empty();
     }
 
+    /**
+     * Maps to {@link Block#getCollisionShape(BlockState, BlockGetter, BlockPos, CollisionContext)}.
+     * <p>
+     * If this shape changes after initial placement, call {@link TileMultipart#markShapeChange()}.
+     */
     default VoxelShape getCollisionShape(CollisionContext context) {
         return getShape(context);
     }
 
+    /**
+     * Maps to {@link Block#getOcclusionShape(BlockState, BlockGetter, BlockPos)}.
+     * <p>
+     * If this shape changes after initial placement, call {@link TileMultipart#markShapeChange()}.
+     */
     default VoxelShape getRenderOcclusionShape() {
         return getShape(CollisionContext.empty());
     }
 
+    /**
+     * Maps to {@link Block#getInteractionShape(BlockState, BlockGetter, BlockPos)}.
+     * <p>
+     * If this shape changes after initial placement, call {@link TileMultipart#markShapeChange()}.
+     */
     default VoxelShape getInteractionShape() {
         return getShape(CollisionContext.empty());
     }
 
+    /**
+     * Maps to {@link Block#getBlockSupportShape(BlockState, BlockGetter, BlockPos)}.
+     * <p>
+     * If this shape changes after initial placement, call {@link TileMultipart#markShapeChange()}.
+     */
     default VoxelShape getBlockSupportShape() {
         return getShape(CollisionContext.empty());
     }
 
+    /**
+     * Maps to {@link Block#getVisualShape(BlockState, BlockGetter, BlockPos, CollisionContext)}.
+     * <p>
+     * If this shape changes after initial placement, call {@link TileMultipart#markShapeChange()}.
+     */
     default VoxelShape getVisualShape(CollisionContext context) {
         return getShape(context);
     }

diff --git a/src/main/java/codechicken/multipart/api/part/PartialOcclusionPart.java b/src/main/java/codechicken/multipart/api/part/PartialOcclusionPart.java
@@ -1,5 +1,6 @@
 package codechicken.multipart.api.part;
 
+import codechicken.multipart.block.TileMultipart;
 import net.minecraft.world.phys.shapes.VoxelShape;
 
 /**
@@ -18,6 +19,8 @@ public interface PartialOcclusionPart extends MultiPart {
      * <p>
      * It is expected that this method return some form of cached instance that does NOT change
      * each call, unless some internal state has changed.
+     * <p>
+     * If this shape changes after initial placement, call {@link TileMultipart#markShapeChange()}.
      *
      * @return the VoxelShape for partial occlusion tests.
      */

diff --git a/src/main/java/codechicken/multipart/trait/TPartialOcclusionTile.java b/src/main/java/codechicken/multipart/trait/TPartialOcclusionTile.java
@@ -20,45 +20,38 @@
  */
 public class TPartialOcclusionTile extends TileMultipart {
 
-    //Static cache.
+    // Cached partial occlusion test results for performance
+    // This cache exists almost entirely for the microblock highlight renderer, due to how expensive combining VoxelShapes is.
+    // Normal occlusion operations happen infrequently enough that this is not a performance concern during normal gameplay.
+    // TODO, Figure out how to cleanly nuke this and do caching inside MicroblockRender.
     @Nullable
-    private static TPartialOcclusionTile lastOcclusionTestedTile;
+    private Iterable<MultiPart> lastTestParts = null;
     @Nullable
-    private static VoxelShape lastOcclusionTestedShape;
-    private static boolean lastOcclusionTestedResult;
+    private VoxelShape lastTestShape = null;
+    private boolean lastTestResult = false;
 
     public TPartialOcclusionTile(BlockPos pos, BlockState state) {
         super(pos, state);
     }
 
     @Override
-    public void bindPart(MultiPart newPart) {
-        super.bindPart(newPart);
-
-        if (newPart instanceof PartialOcclusionPart && lastOcclusionTestedTile == this) {
-            lastOcclusionTestedTile = null;
-        }
-    }
-
-    @Override
-    public void partRemoved(MultiPart remPart, int p) {
-        super.partRemoved(remPart, p);
-
-        if (remPart instanceof PartialOcclusionPart && lastOcclusionTestedTile == this) {
-            lastOcclusionTestedTile = null;
-        }
+    public void markShapeChange() {
+        super.markShapeChange();
+        // Invalidate cached results on part add, remove, or shape change
+        lastTestParts = null;
+        lastTestShape = null;
     }
 
     @Override
     public boolean occlusionTest(Iterable<MultiPart> parts, MultiPart npart) {
         if (npart instanceof PartialOcclusionPart newPart) {
             VoxelShape newShape = newPart.getPartialOcclusionShape();
-            if (lastOcclusionTestedTile != this || lastOcclusionTestedShape != newShape) {
-                lastOcclusionTestedTile = this;
-                lastOcclusionTestedShape = newShape;
-                lastOcclusionTestedResult = partialOcclusionTest(parts, newPart);
+            if (lastTestParts != parts || lastTestShape != newShape) {
+                lastTestParts = parts;
+                lastTestShape = newShape;
+                lastTestResult = partialOcclusionTest(parts, newPart);
             }
-            if (!lastOcclusionTestedResult) {
+            if (!lastTestResult) {
                 return false;
             }
         }