From 5dc96a0ed53addd755b88d74f931545212468f1d Mon Sep 17 00:00:00 2001 From: Marco Rebhan Date: Sat, 7 Aug 2021 13:34:39 +0200 Subject: [PATCH 1/4] Fix canvas rendering bugs --- .../block/entity/BakedBlockEntityRenderer.java | 17 +++++++++++++++++ .../entity/HyperlinkBlockEntityRenderer.java | 14 +++++++++++++- .../entity/ItemDisplayBlockEntityRenderer.java | 11 ++++++++++- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/BakedBlockEntityRenderer.java b/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/BakedBlockEntityRenderer.java index 4ed782a..e1d675a 100644 --- a/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/BakedBlockEntityRenderer.java +++ b/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/BakedBlockEntityRenderer.java @@ -5,6 +5,7 @@ import it.unimi.dsi.fastutil.objects.Object2ObjectArrayMap; import it.unimi.dsi.fastutil.objects.ObjectArraySet; import net.fabricmc.fabric.api.client.rendering.v1.InvalidateRenderStateCallback; +import net.fabricmc.loader.api.FabricLoader; import net.minecraft.block.entity.BlockEntity; import net.minecraft.client.MinecraftClient; import net.minecraft.client.gl.VertexBuffer; @@ -16,13 +17,17 @@ import net.minecraft.client.world.ClientWorld; import net.minecraft.util.math.BlockPos; import net.minecraft.util.math.Matrix4f; +import net.minecraft.util.math.Quaternion; import net.minecraft.util.math.Vec3d; +import net.minecraft.util.math.Vec3f; import net.minecraft.world.World; import org.jetbrains.annotations.NotNull; import java.util.*; public abstract class BakedBlockEntityRenderer implements BlockEntityRenderer { + private static final boolean USE_CANVAS_HACK = FabricLoader.getInstance().isModLoaded("canvas"); + protected final BlockEntityRendererFactory.Context context; protected BakedBlockEntityRenderer(BlockEntityRendererFactory.Context context) { @@ -229,6 +234,14 @@ public void render(MatrixStack matrices, Matrix4f projectionMatrix, Camera camer double camZ = vec3d.getZ(); RenderRegionPos centerRegion = new RenderRegionPos((int)camX >> REGION_SHIFT, (int)camZ >> REGION_SHIFT); + if (USE_CANVAS_HACK) { + float pitch = camera.getPitch(); + float yaw = camera.getYaw(); + matrices.push(); + matrices.multiply(new Quaternion(Vec3f.POSITIVE_X, pitch, true)); + matrices.multiply(new Quaternion(Vec3f.POSITIVE_Y, yaw + 180, true)); + } + // Iterate over all RegionBuilders, render and upload to RegionBuffers Set usedRenderLayers = new ObjectArraySet<>(); List blockEntities = new ArrayList<>(); @@ -293,6 +306,10 @@ public void render(MatrixStack matrices, Matrix4f projectionMatrix, Camera camer } layer.endDrawing(); } + + if (USE_CANVAS_HACK) { + matrices.pop(); + } } public void activateRegion(BlockPos pos) { diff --git a/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/HyperlinkBlockEntityRenderer.java b/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/HyperlinkBlockEntityRenderer.java index 8f8c1c5..bfce126 100644 --- a/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/HyperlinkBlockEntityRenderer.java +++ b/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/HyperlinkBlockEntityRenderer.java @@ -34,7 +34,19 @@ public void render(HyperlinkBlockEntity entity, float f, MatrixStack matrices, V float scale = 0.025F; matrices.scale(scale, scale, scale); matrices.translate(-MinecraftClient.getInstance().textRenderer.getWidth(entity.url) / 2F, -4, 0); - MinecraftClient.getInstance().textRenderer.drawWithShadow(matrices, entity.url, 0, 0, 0xFFFFFF); + + String name = entity.url; + int color = -1; + + MinecraftClient.getInstance().textRenderer.draw(name, 0, 0, color, false, matrices.peek().getModel(), vertexConsumers, false, 0, 0xF000F0); + + // doing shadow manually because the way text renderer adds the + // shadow offset is wrong + // this is going to slightly break with unicode fonts too since + // they use an offset of 0.5 instead of 1 but better than nothing + matrices.translate(1.0, 1.0, 0.03); + int shadowColor = (int) ((color & 0xFF) * 0.25) | (int) ((color >> 8 & 0xFF) * 0.25) << 8 | (int) ((color >> 16 & 0xFF) * 0.25) << 16 | color & 0xFF000000; + MinecraftClient.getInstance().textRenderer.draw(name, 0, 0, shadowColor, false, matrices.peek().getModel(), vertexConsumers, false, 0, 0xF000F0); } matrices.pop(); } diff --git a/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/ItemDisplayBlockEntityRenderer.java b/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/ItemDisplayBlockEntityRenderer.java index 4ff6c4f..4a62750 100644 --- a/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/ItemDisplayBlockEntityRenderer.java +++ b/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/ItemDisplayBlockEntityRenderer.java @@ -99,7 +99,16 @@ public void render(ItemDisplayBlockEntity entity, float tickDelta, MatrixStack m int color = name.getStyle().getColor() == null ? 0xFFFFFF : name.getStyle().getColor().getRgb(); matrices.translate(-MinecraftClient.getInstance().textRenderer.getWidth(name) / 2F, -4, 0); - MinecraftClient.getInstance().textRenderer.drawWithShadow(matrices, name, 0, 0, color); + + MinecraftClient.getInstance().textRenderer.draw(name, 0, 0, color, false, matrices.peek().getModel(), vertexConsumers, false, 0, 0xF000F0); + + // doing shadow manually because the way text renderer adds the + // shadow offset is wrong + // this is going to slightly break with unicode fonts too since + // they use an offset of 0.5 instead of 1 but better than nothing + matrices.translate(1.0, 1.0, 0.03); + int shadowColor = (int) ((color & 0xFF) * 0.25) | (int) ((color >> 8 & 0xFF) * 0.25) << 8 | (int) ((color >> 16 & 0xFF) * 0.25) << 16 | color & 0xFF000000; + MinecraftClient.getInstance().textRenderer.draw(name, 0, 0, shadowColor, false, matrices.peek().getModel(), vertexConsumers, false, 0, 0xF000F0); } } From 72ed37429779de374cd6e6bdbf77eaa1f72f123a Mon Sep 17 00:00:00 2001 From: Marco Rebhan Date: Sat, 7 Aug 2021 18:10:34 +0200 Subject: [PATCH 2/4] Improve shadow rendering --- .../entity/HyperlinkBlockEntityRenderer.java | 15 +------- .../ItemDisplayBlockEntityRenderer.java | 12 +----- .../mixin/client/TextRendererMixin.java | 38 +++++++++++++++++++ src/main/resources/glowcase.mixins.json | 5 ++- 4 files changed, 45 insertions(+), 25 deletions(-) create mode 100644 src/main/java/dev/hephaestus/glowcase/mixin/client/TextRendererMixin.java diff --git a/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/HyperlinkBlockEntityRenderer.java b/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/HyperlinkBlockEntityRenderer.java index bfce126..b73238a 100644 --- a/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/HyperlinkBlockEntityRenderer.java +++ b/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/HyperlinkBlockEntityRenderer.java @@ -32,21 +32,10 @@ public void render(HyperlinkBlockEntity entity, float f, MatrixStack matrices, V HitResult hitResult = MinecraftClient.getInstance().crosshairTarget; if (hitResult instanceof BlockHitResult && ((BlockHitResult) hitResult).getBlockPos().equals(entity.getPos())) { float scale = 0.025F; - matrices.scale(scale, scale, scale); + matrices.scale(scale, scale, -scale); matrices.translate(-MinecraftClient.getInstance().textRenderer.getWidth(entity.url) / 2F, -4, 0); - String name = entity.url; - int color = -1; - - MinecraftClient.getInstance().textRenderer.draw(name, 0, 0, color, false, matrices.peek().getModel(), vertexConsumers, false, 0, 0xF000F0); - - // doing shadow manually because the way text renderer adds the - // shadow offset is wrong - // this is going to slightly break with unicode fonts too since - // they use an offset of 0.5 instead of 1 but better than nothing - matrices.translate(1.0, 1.0, 0.03); - int shadowColor = (int) ((color & 0xFF) * 0.25) | (int) ((color >> 8 & 0xFF) * 0.25) << 8 | (int) ((color >> 16 & 0xFF) * 0.25) << 16 | color & 0xFF000000; - MinecraftClient.getInstance().textRenderer.draw(name, 0, 0, shadowColor, false, matrices.peek().getModel(), vertexConsumers, false, 0, 0xF000F0); + MinecraftClient.getInstance().textRenderer.draw(entity.url, 0, 0, -1, true, matrices.peek().getModel(), vertexConsumers, false, 0, 0xF000F0); } matrices.pop(); } diff --git a/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/ItemDisplayBlockEntityRenderer.java b/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/ItemDisplayBlockEntityRenderer.java index 4a62750..a1bb4a6 100644 --- a/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/ItemDisplayBlockEntityRenderer.java +++ b/src/main/java/dev/hephaestus/glowcase/client/render/block/entity/ItemDisplayBlockEntityRenderer.java @@ -95,20 +95,12 @@ public void render(ItemDisplayBlockEntity entity, float tickDelta, MatrixStack m matrices.translate(0, 0, -0.4); float scale = 0.025F; - matrices.scale(scale, scale, scale); + matrices.scale(scale, scale, -scale); int color = name.getStyle().getColor() == null ? 0xFFFFFF : name.getStyle().getColor().getRgb(); matrices.translate(-MinecraftClient.getInstance().textRenderer.getWidth(name) / 2F, -4, 0); - MinecraftClient.getInstance().textRenderer.draw(name, 0, 0, color, false, matrices.peek().getModel(), vertexConsumers, false, 0, 0xF000F0); - - // doing shadow manually because the way text renderer adds the - // shadow offset is wrong - // this is going to slightly break with unicode fonts too since - // they use an offset of 0.5 instead of 1 but better than nothing - matrices.translate(1.0, 1.0, 0.03); - int shadowColor = (int) ((color & 0xFF) * 0.25) | (int) ((color >> 8 & 0xFF) * 0.25) << 8 | (int) ((color >> 16 & 0xFF) * 0.25) << 16 | color & 0xFF000000; - MinecraftClient.getInstance().textRenderer.draw(name, 0, 0, shadowColor, false, matrices.peek().getModel(), vertexConsumers, false, 0, 0xF000F0); + MinecraftClient.getInstance().textRenderer.draw(name, 0, 0, color, true, matrices.peek().getModel(), vertexConsumers, false, 0, 0xF000F0); } } diff --git a/src/main/java/dev/hephaestus/glowcase/mixin/client/TextRendererMixin.java b/src/main/java/dev/hephaestus/glowcase/mixin/client/TextRendererMixin.java new file mode 100644 index 0000000..9f86d50 --- /dev/null +++ b/src/main/java/dev/hephaestus/glowcase/mixin/client/TextRendererMixin.java @@ -0,0 +1,38 @@ +package dev.hephaestus.glowcase.mixin.client; + +import net.minecraft.client.font.TextRenderer; +import net.minecraft.util.math.Matrix4f; +import net.minecraft.util.math.Vec3f; + +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Redirect; + +@Mixin(TextRenderer.class) +public class TextRendererMixin { + // These addToLastColumn calls are originally used for offsetting the text + // shadow, however simply adding to the last column of the matrix to produce + // an offset is wrong since it will ignore any previous transformations made + // and will only work correctly in special cases (the transformation matrix + // having no rotation). + // So since what's actually wanted here is a translation, that's what we do. + // Another option would be to modify addToLastColumn directly because these + // two lines of code are the only thing that ever calls that method, but + // then its name would be wrong. + + @Redirect( + method = "drawInternal(Ljava/lang/String;FFIZLnet/minecraft/util/math/Matrix4f;Lnet/minecraft/client/render/VertexConsumerProvider;ZIIZ)I", + at = @At(value = "INVOKE", target = "Lnet/minecraft/util/math/Matrix4f;addToLastColumn(Lnet/minecraft/util/math/Vec3f;)V") + ) + private void fixTranslation1(Matrix4f matrix4f, Vec3f vector) { + matrix4f.multiplyByTranslation(vector.getX(), vector.getY(), vector.getZ()); + } + + @Redirect( + method = "drawInternal(Lnet/minecraft/text/OrderedText;FFIZLnet/minecraft/util/math/Matrix4f;Lnet/minecraft/client/render/VertexConsumerProvider;ZII)I", + at = @At(value = "INVOKE", target = "Lnet/minecraft/util/math/Matrix4f;addToLastColumn(Lnet/minecraft/util/math/Vec3f;)V") + ) + private void fixTranslation2(Matrix4f matrix4f, Vec3f vector) { + matrix4f.multiplyByTranslation(vector.getX(), vector.getY(), vector.getZ()); + } +} diff --git a/src/main/resources/glowcase.mixins.json b/src/main/resources/glowcase.mixins.json index 7856cd1..a7b5169 100644 --- a/src/main/resources/glowcase.mixins.json +++ b/src/main/resources/glowcase.mixins.json @@ -3,13 +3,14 @@ "package": "dev.hephaestus.glowcase.mixin", "compatibilityLevel": "JAVA_8", "mixins": [ - "block.EntityShapeContextAccessor", + "block.EntityShapeContextAccessor" ], "client": [ "client.MinecraftClientAccessor", "client.render.ber.RenderPhaseAccessor", "client.render.ber.WorldRendererMixin", - "client.render.entity.EntityRenderDispatcherAccessor" + "client.render.entity.EntityRenderDispatcherAccessor", + "client.TextRendererMixin" ], "injectors": { "defaultRequire": 1 From e6fa72a7f55a7b0a3833330413d92d1e4e00e3e3 Mon Sep 17 00:00:00 2001 From: Marco Rebhan Date: Sat, 7 Aug 2021 18:14:52 +0200 Subject: [PATCH 3/4] Keep behavior similar to the original code --- .../mixin/client/TextRendererMixin.java | 37 +++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/src/main/java/dev/hephaestus/glowcase/mixin/client/TextRendererMixin.java b/src/main/java/dev/hephaestus/glowcase/mixin/client/TextRendererMixin.java index 9f86d50..3e6c05a 100644 --- a/src/main/java/dev/hephaestus/glowcase/mixin/client/TextRendererMixin.java +++ b/src/main/java/dev/hephaestus/glowcase/mixin/client/TextRendererMixin.java @@ -3,6 +3,7 @@ import net.minecraft.client.font.TextRenderer; import net.minecraft.util.math.Matrix4f; import net.minecraft.util.math.Vec3f; +import net.minecraft.util.math.Vector4f; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.injection.At; @@ -15,17 +16,47 @@ public class TextRendererMixin { // an offset is wrong since it will ignore any previous transformations made // and will only work correctly in special cases (the transformation matrix // having no rotation). - // So since what's actually wanted here is a translation, that's what we do. + // One option here would be to simply use normal translation, but I believe + // the reason for using addToLastColumn here is to keep the offset constant + // and not make it increase when the matrix scales up coordinates. + // So, what we do here is get the axis unit vectors stored in the first + // three columns of the matrix and use it to calculate a constant offset. + // This way, we keep as close to the original behavior as possible. + // Technically, only handling the Z axis vector should be required here + // since the original code always passes in (0, 0, 0.03) as the vector, but + // let's make it correct. // Another option would be to modify addToLastColumn directly because these // two lines of code are the only thing that ever calls that method, but // then its name would be wrong. + private void translateUnscaled(Matrix4f mat, Vec3f vec) { + // the matrix doesn't provide us with a way of accessing its data, so + // this will have to do + Vector4f x = new Vector4f(1, 0, 0, 0); + Vector4f y = new Vector4f(0, 1, 0, 0); + Vector4f z = new Vector4f(0, 0, 1, 0); + x.transform(mat); + y.transform(mat); + z.transform(mat); + Vec3f x1 = new Vec3f(x); + Vec3f y1 = new Vec3f(y); + Vec3f z1 = new Vec3f(z); + x1.normalize(); + y1.normalize(); + z1.normalize(); + mat.addToLastColumn(new Vec3f( + x1.getX() * vec.getX() + y1.getX() * vec.getY() + z1.getX() * vec.getZ(), + x1.getY() * vec.getX() + y1.getY() * vec.getY() + z1.getY() * vec.getZ(), + x1.getZ() * vec.getX() + y1.getZ() * vec.getY() + z1.getZ() * vec.getZ() + )); + } + @Redirect( method = "drawInternal(Ljava/lang/String;FFIZLnet/minecraft/util/math/Matrix4f;Lnet/minecraft/client/render/VertexConsumerProvider;ZIIZ)I", at = @At(value = "INVOKE", target = "Lnet/minecraft/util/math/Matrix4f;addToLastColumn(Lnet/minecraft/util/math/Vec3f;)V") ) private void fixTranslation1(Matrix4f matrix4f, Vec3f vector) { - matrix4f.multiplyByTranslation(vector.getX(), vector.getY(), vector.getZ()); + this.translateUnscaled(matrix4f, vector); } @Redirect( @@ -33,6 +64,6 @@ private void fixTranslation1(Matrix4f matrix4f, Vec3f vector) { at = @At(value = "INVOKE", target = "Lnet/minecraft/util/math/Matrix4f;addToLastColumn(Lnet/minecraft/util/math/Vec3f;)V") ) private void fixTranslation2(Matrix4f matrix4f, Vec3f vector) { - matrix4f.multiplyByTranslation(vector.getX(), vector.getY(), vector.getZ()); + this.translateUnscaled(matrix4f, vector); } } From 84e5563e34134514ac29ddeb52b1c2c0c32e0109 Mon Sep 17 00:00:00 2001 From: Marco Rebhan Date: Sat, 7 Aug 2021 18:44:09 +0200 Subject: [PATCH 4/4] Revert "Keep behavior similar to the original code" This actually looks worse. This reverts commit e6fa72a7f55a7b0a3833330413d92d1e4e00e3e3. --- .../mixin/client/TextRendererMixin.java | 37 ++----------------- 1 file changed, 3 insertions(+), 34 deletions(-) diff --git a/src/main/java/dev/hephaestus/glowcase/mixin/client/TextRendererMixin.java b/src/main/java/dev/hephaestus/glowcase/mixin/client/TextRendererMixin.java index 3e6c05a..9f86d50 100644 --- a/src/main/java/dev/hephaestus/glowcase/mixin/client/TextRendererMixin.java +++ b/src/main/java/dev/hephaestus/glowcase/mixin/client/TextRendererMixin.java @@ -3,7 +3,6 @@ import net.minecraft.client.font.TextRenderer; import net.minecraft.util.math.Matrix4f; import net.minecraft.util.math.Vec3f; -import net.minecraft.util.math.Vector4f; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.injection.At; @@ -16,47 +15,17 @@ public class TextRendererMixin { // an offset is wrong since it will ignore any previous transformations made // and will only work correctly in special cases (the transformation matrix // having no rotation). - // One option here would be to simply use normal translation, but I believe - // the reason for using addToLastColumn here is to keep the offset constant - // and not make it increase when the matrix scales up coordinates. - // So, what we do here is get the axis unit vectors stored in the first - // three columns of the matrix and use it to calculate a constant offset. - // This way, we keep as close to the original behavior as possible. - // Technically, only handling the Z axis vector should be required here - // since the original code always passes in (0, 0, 0.03) as the vector, but - // let's make it correct. + // So since what's actually wanted here is a translation, that's what we do. // Another option would be to modify addToLastColumn directly because these // two lines of code are the only thing that ever calls that method, but // then its name would be wrong. - private void translateUnscaled(Matrix4f mat, Vec3f vec) { - // the matrix doesn't provide us with a way of accessing its data, so - // this will have to do - Vector4f x = new Vector4f(1, 0, 0, 0); - Vector4f y = new Vector4f(0, 1, 0, 0); - Vector4f z = new Vector4f(0, 0, 1, 0); - x.transform(mat); - y.transform(mat); - z.transform(mat); - Vec3f x1 = new Vec3f(x); - Vec3f y1 = new Vec3f(y); - Vec3f z1 = new Vec3f(z); - x1.normalize(); - y1.normalize(); - z1.normalize(); - mat.addToLastColumn(new Vec3f( - x1.getX() * vec.getX() + y1.getX() * vec.getY() + z1.getX() * vec.getZ(), - x1.getY() * vec.getX() + y1.getY() * vec.getY() + z1.getY() * vec.getZ(), - x1.getZ() * vec.getX() + y1.getZ() * vec.getY() + z1.getZ() * vec.getZ() - )); - } - @Redirect( method = "drawInternal(Ljava/lang/String;FFIZLnet/minecraft/util/math/Matrix4f;Lnet/minecraft/client/render/VertexConsumerProvider;ZIIZ)I", at = @At(value = "INVOKE", target = "Lnet/minecraft/util/math/Matrix4f;addToLastColumn(Lnet/minecraft/util/math/Vec3f;)V") ) private void fixTranslation1(Matrix4f matrix4f, Vec3f vector) { - this.translateUnscaled(matrix4f, vector); + matrix4f.multiplyByTranslation(vector.getX(), vector.getY(), vector.getZ()); } @Redirect( @@ -64,6 +33,6 @@ private void fixTranslation1(Matrix4f matrix4f, Vec3f vector) { at = @At(value = "INVOKE", target = "Lnet/minecraft/util/math/Matrix4f;addToLastColumn(Lnet/minecraft/util/math/Vec3f;)V") ) private void fixTranslation2(Matrix4f matrix4f, Vec3f vector) { - this.translateUnscaled(matrix4f, vector); + matrix4f.multiplyByTranslation(vector.getX(), vector.getY(), vector.getZ()); } }