From dc56d51f118e080eb35d966918795c3d9cddcc68 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 29 Apr 2023 13:19:02 -0400 Subject: [PATCH 1/2] Split registry rewrite into separate off-by-default option This needs more testing --- .../core/config/ModernFixEarlyConfig.java | 2 + .../ForgeRegistryMixin.java | 48 -------------- .../rewrite_registry/ForgeRegistryMixin.java | 63 +++++++++++++++++++ .../ForgeRegistrySnapshotMixin.java | 2 +- src/main/resources/modernfix.mixins.json | 3 +- 5 files changed, 68 insertions(+), 50 deletions(-) create mode 100644 src/main/java/org/embeddedt/modernfix/mixin/perf/rewrite_registry/ForgeRegistryMixin.java rename src/main/java/org/embeddedt/modernfix/mixin/perf/{fast_registry_validation => rewrite_registry}/ForgeRegistrySnapshotMixin.java (94%) diff --git a/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java b/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java index 96cc4656..a2770142 100644 --- a/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java +++ b/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java @@ -45,6 +45,8 @@ public class ModernFixEarlyConfig { this.addMixinRule("feature.reduce_loading_screen_freezes", false); this.addMixinRule("feature.direct_stack_trace", false); this.addMixinRule("perf.fast_registry_validation", true); + // not stable yet + this.addMixinRule("perf.rewrite_registry", false); this.addMixinRule("perf.use_integrated_resources.jepb", modPresent("jepb")); this.addMixinRule("perf.use_integrated_resources.jeresources", modPresent("jeresources")); this.addMixinRule("perf.jeresources_startup", modPresent("jeresources")); diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java index 09b7daf7..8aa3e2d8 100644 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java @@ -1,19 +1,11 @@ package org.embeddedt.modernfix.mixin.perf.fast_registry_validation; -import com.google.common.collect.BiMap; -import net.minecraft.core.Registry; -import net.minecraft.resources.ResourceKey; -import net.minecraft.resources.ResourceLocation; import net.minecraftforge.fml.common.ObfuscationReflectionHelper; import net.minecraftforge.registries.ForgeRegistry; import net.minecraftforge.registries.IForgeRegistryEntry; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.Marker; -import org.embeddedt.modernfix.registry.FastForgeRegistry; -import org.spongepowered.asm.mixin.Final; import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.Mutable; -import org.spongepowered.asm.mixin.Shadow; import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject; import org.spongepowered.asm.mixin.injection.Redirect; @@ -67,44 +59,4 @@ public class ForgeRegistryMixin> { private void skipTrace(Logger logger, Marker marker, String s, Object o, Object o1, Object o2, Object o3, Object o4) { } - - @Shadow @Final @Mutable private BiMap ids; - - @Shadow @Final @Mutable private BiMap, V> keys; - - @Shadow @Final private ResourceKey> key; - - @Shadow @Final @Mutable private BiMap names; - - @Shadow @Final @Mutable private BiMap owners; - - private FastForgeRegistry fastRegistry; - - /** - * The following code replaces the Forge HashBiMaps with a more efficient data structure based around - * an array list for IDs and one HashMap going from value -> information. - */ - @Inject(method = "", at = @At("RETURN")) - private void replaceBackingMaps(CallbackInfo ci) { - this.fastRegistry = new FastForgeRegistry<>(this.key); - this.ids = fastRegistry.getIds(); - this.keys = fastRegistry.getKeys(); - this.names = fastRegistry.getNames(); - this.owners = fastRegistry.getOwners(); - } - - @Inject(method = "freeze", at = @At("RETURN")) - private void optimizeRegistry(CallbackInfo ci) { - this.fastRegistry.optimize(); - } - - @Redirect(method = "sync", at = @At(value = "INVOKE", target = "Lcom/google/common/collect/BiMap;clear()V")) - private void clearBiMap(BiMap map) { - if(map == this.owners) { - this.fastRegistry.clear(); - } else if(map == this.keys || map == this.names || map == this.ids) { - // do nothing, the registry is faster at clearing everything at once - } else - map.clear(); - } } diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/rewrite_registry/ForgeRegistryMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/rewrite_registry/ForgeRegistryMixin.java new file mode 100644 index 00000000..558796d1 --- /dev/null +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/rewrite_registry/ForgeRegistryMixin.java @@ -0,0 +1,63 @@ +package org.embeddedt.modernfix.mixin.perf.rewrite_registry; + +import com.google.common.collect.BiMap; +import net.minecraft.core.Registry; +import net.minecraft.resources.ResourceKey; +import net.minecraft.resources.ResourceLocation; +import net.minecraftforge.registries.ForgeRegistry; +import net.minecraftforge.registries.IForgeRegistryEntry; +import org.embeddedt.modernfix.registry.FastForgeRegistry; +import org.spongepowered.asm.mixin.Final; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.Mutable; +import org.spongepowered.asm.mixin.Shadow; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Inject; +import org.spongepowered.asm.mixin.injection.Redirect; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; + +@Mixin(value = ForgeRegistry.class, remap = false) +public class ForgeRegistryMixin> { + @Shadow + @Final + @Mutable + private BiMap ids; + + @Shadow @Final @Mutable private BiMap, V> keys; + + @Shadow @Final private ResourceKey> key; + + @Shadow @Final @Mutable private BiMap names; + + @Shadow @Final @Mutable private BiMap owners; + + private FastForgeRegistry fastRegistry; + + /** + * The following code replaces the Forge HashBiMaps with a more efficient data structure based around + * an array list for IDs and one HashMap going from value -> information. + */ + @Inject(method = "", at = @At("RETURN")) + private void replaceBackingMaps(CallbackInfo ci) { + this.fastRegistry = new FastForgeRegistry<>(this.key); + this.ids = fastRegistry.getIds(); + this.keys = fastRegistry.getKeys(); + this.names = fastRegistry.getNames(); + this.owners = fastRegistry.getOwners(); + } + + @Inject(method = "freeze", at = @At("RETURN")) + private void optimizeRegistry(CallbackInfo ci) { + this.fastRegistry.optimize(); + } + + @Redirect(method = "sync", at = @At(value = "INVOKE", target = "Lcom/google/common/collect/BiMap;clear()V")) + private void clearBiMap(BiMap map) { + if(map == this.owners) { + this.fastRegistry.clear(); + } else if(map == this.keys || map == this.names || map == this.ids) { + // do nothing, the registry is faster at clearing everything at once + } else + map.clear(); + } +} diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistrySnapshotMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/rewrite_registry/ForgeRegistrySnapshotMixin.java similarity index 94% rename from src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistrySnapshotMixin.java rename to src/main/java/org/embeddedt/modernfix/mixin/perf/rewrite_registry/ForgeRegistrySnapshotMixin.java index ad941ab0..87fb4de7 100644 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistrySnapshotMixin.java +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/rewrite_registry/ForgeRegistrySnapshotMixin.java @@ -1,4 +1,4 @@ -package org.embeddedt.modernfix.mixin.perf.fast_registry_validation; +package org.embeddedt.modernfix.mixin.perf.rewrite_registry; import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; diff --git a/src/main/resources/modernfix.mixins.json b/src/main/resources/modernfix.mixins.json index 99ce333d..bc1b1668 100644 --- a/src/main/resources/modernfix.mixins.json +++ b/src/main/resources/modernfix.mixins.json @@ -64,7 +64,8 @@ "perf.kubejs.CustomIngredientMixin", "perf.nbt_memory_usage.CompoundTagMixin", "perf.fast_registry_validation.ForgeRegistryMixin", - "perf.fast_registry_validation.ForgeRegistrySnapshotMixin", + "perf.rewrite_registry.ForgeRegistryMixin", + "perf.rewrite_registry.ForgeRegistrySnapshotMixin", "perf.fast_registry_validation.ResourceKeyMixin", "perf.cache_strongholds.ChunkGeneratorMixin", "perf.cache_upgraded_structures.StructureManagerMixin", From 5ac369acb8254ef981a345990d05544e7d734a0a Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 29 Apr 2023 14:06:53 -0400 Subject: [PATCH 2/2] Fix some items not rendering in inventories --- .../ItemModelShaperMixin.java | 106 ++++++++++++++++-- .../dynamic_resources/ModelBakeryMixin.java | 6 +- 2 files changed, 100 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ItemModelShaperMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ItemModelShaperMixin.java index 86b708bd..018cdf66 100644 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ItemModelShaperMixin.java +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ItemModelShaperMixin.java @@ -6,18 +6,26 @@ import net.minecraft.client.resources.model.ModelManager; import net.minecraft.client.resources.model.ModelResourceLocation; import net.minecraft.world.item.Item; import net.minecraftforge.client.ItemModelMesherForge; +import net.minecraftforge.registries.ForgeRegistries; import net.minecraftforge.registries.IRegistryDelegate; import org.embeddedt.modernfix.dynamicresources.ModelLocationCache; -import org.spongepowered.asm.mixin.Final; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.Overwrite; -import org.spongepowered.asm.mixin.Shadow; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.spongepowered.asm.mixin.*; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Inject; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; +import java.util.Collection; +import java.util.HashMap; import java.util.Map; +import java.util.Set; @Mixin(ItemModelMesherForge.class) public abstract class ItemModelShaperMixin extends ItemModelShaper { - @Shadow @Final private Map, ModelResourceLocation> locations; + @Shadow @Final @Mutable private Map, ModelResourceLocation> locations; + + private Map, ModelResourceLocation> overrideLocations; public ItemModelShaperMixin(ModelManager arg) { super(arg); @@ -25,6 +33,86 @@ public abstract class ItemModelShaperMixin extends ItemModelShaper { private static final ModelResourceLocation SENTINEL = new ModelResourceLocation("modernfix", "sentinel"); + @Inject(method = "", at = @At("RETURN")) + private void replaceLocationMap(CallbackInfo ci) { + overrideLocations = new HashMap<>(); + // need to replace this map because mods query locations through it + locations = new Map, ModelResourceLocation>() { + @Override + public int size() { + return ForgeRegistries.ITEMS.getValues().size(); + } + + @Override + public boolean isEmpty() { + return false; + } + + @Override + public boolean containsKey(Object key) { + return true; + } + + @Override + public boolean containsValue(Object value) { + return false; + } + + @Override + public ModelResourceLocation get(Object key) { + return getLocation(((IRegistryDelegate)key).get()); + } + + @Nullable + @Override + public ModelResourceLocation put(IRegistryDelegate key, ModelResourceLocation value) { + throw new UnsupportedOperationException(); + } + + @Override + public ModelResourceLocation remove(Object key) { + throw new UnsupportedOperationException(); + } + + @Override + public void putAll(@NotNull Map, ? extends ModelResourceLocation> m) { + throw new UnsupportedOperationException(); + } + + @Override + public void clear() { + throw new UnsupportedOperationException(); + } + + @NotNull + @Override + public Set> keySet() { + throw new UnsupportedOperationException(); + } + + @NotNull + @Override + public Collection values() { + throw new UnsupportedOperationException(); + } + + @NotNull + @Override + public Set, ModelResourceLocation>> entrySet() { + throw new UnsupportedOperationException(); + } + }; + } + + private ModelResourceLocation getLocation(Item item) { + ModelResourceLocation map = overrideLocations.getOrDefault(item.delegate, SENTINEL); + if(map == SENTINEL) { + /* generate the appropriate location from our cache */ + map = ModelLocationCache.get(item); + } + return map; + } + /** * @reason Get the stored location for that item and meta, and get the model * from that location from the model manager. @@ -32,11 +120,7 @@ public abstract class ItemModelShaperMixin extends ItemModelShaper { @Overwrite @Override public BakedModel getItemModel(Item item) { - ModelResourceLocation map = locations.getOrDefault(item.delegate, SENTINEL); - if(map == SENTINEL) { - /* generate the appropriate location from our cache */ - map = ModelLocationCache.get(item); - } + ModelResourceLocation map = getLocation(item); return map == null ? null : getModelManager().getModel(map); } @@ -47,7 +131,7 @@ public abstract class ItemModelShaperMixin extends ItemModelShaper { @Overwrite @Override public void register(Item item, ModelResourceLocation location) { - locations.put(item.delegate, location); + overrideLocations.put(item.delegate, location); } /** diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java index a3c39583..e7e3e708 100644 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java @@ -515,8 +515,10 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { } @Redirect(method = "loadModel", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/level/block/state/StateDefinition;getPossibleStates()Lcom/google/common/collect/ImmutableList;")) private ImmutableList loadOnlyRelevantBlockState(StateDefinition stateDefinition, ResourceLocation location) { - Set> fixedProperties = new HashSet<>(); ModelResourceLocation mrl = (ModelResourceLocation)location; + if(Objects.equals(mrl.getVariant(), "inventory")) + return ImmutableList.of(); + Set> fixedProperties = new HashSet<>(); BlockState fixedState = stateDefinition.any(); for(String s : COMMA_SPLITTER.split(mrl.getVariant())) { Iterator iterator = EQUAL_SPLITTER.split(s).iterator(); @@ -574,6 +576,8 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { LOGGER.info("Baking {}", arg); UnbakedModel iunbakedmodel = this.getModel(arg); iunbakedmodel.getMaterials(this::getModel, new HashSet<>()); + if(iunbakedmodel == missingModel && debugDynamicModelLoading) + LOGGER.warn("Model {} not present", arg); BakedModel ibakedmodel = null; if (iunbakedmodel instanceof BlockModel) { BlockModel blockmodel = (BlockModel)iunbakedmodel;