Fix for #987 (#1046)

* Fix #987, simplify geometry tracking, clean up scoping

* Bump indigo version

* Expose improved fromVanilla method

* Strip deprecations in non-API components

* bump renderer api version

* Clear tag for converted quads

Otherwise possible for tags added by transforms to pollute subsequent quads

* Borrow from Canvas - don't set nominal face twice, prevent header state leakage

fabric-renderer-api-v1/build.gradle

@@ -1,5 +1,5 @@
 archivesBaseName = "fabric-renderer-api-v1"
-version = getSubprojectVersion(project, "0.2.13")
+version = getSubprojectVersion(project, "0.3.0")
 
 dependencies {
 	compile project(path: ':fabric-api-base', configuration: 'dev')

fabric-renderer-api-v1/src/main/java/net/fabricmc/fabric/api/renderer/v1/mesh/MutableQuadView.java

@@ -143,9 +143,21 @@ public interface MutableQuadView extends QuadView {
 	 * This method should be performant whenever caller's vertex representation makes it feasible.
 	 *
 	 * <p>Calling this method does not emit the quad.
+	 *
+	 * @deprecated Use {@link #fromVanilla(BakedQuad, RenderMaterial, Direction, int[], int)}
+	 * which has better encapsulation and removed outdated item flag
 	 */
+	@Deprecated
 	MutableQuadView fromVanilla(int[] quadData, int startIndex, boolean isItem);
 
+	/**
+	 * Enables bulk vertex data transfer using the standard Minecraft vertex formats.
+	 * This method should be performant whenever caller's vertex representation makes it feasible.
+	 *
+	 * <p>Calling this method does not emit the quad.
+	 */
+	MutableQuadView fromVanilla(BakedQuad quad, RenderMaterial material, Direction cullFace);
+
 	/**
 	 * Encodes an integer tag with this quad that can later be retrieved via
 	 * {@link QuadView#tag()}.  Useful for models that want to perform conditional

fabric-renderer-indigo/build.gradle

@@ -1,5 +1,5 @@
 archivesBaseName = "fabric-renderer-indigo"
-version = getSubprojectVersion(project, "0.3.4")
+version = getSubprojectVersion(project, "0.4.0")
 
 dependencies {
 	compile project(path: ':fabric-api-base', configuration: 'dev')

fabric-renderer-indigo/src/main/java/net/fabricmc/fabric/impl/client/indigo/renderer/mesh/MeshBuilderImpl.java

@@ -19,7 +19,6 @@ package net.fabricmc.fabric.impl.client.indigo.renderer.mesh;
 import net.fabricmc.fabric.api.renderer.v1.mesh.Mesh;
 import net.fabricmc.fabric.api.renderer.v1.mesh.MeshBuilder;
 import net.fabricmc.fabric.api.renderer.v1.mesh.QuadEmitter;
-import net.fabricmc.fabric.impl.client.indigo.renderer.helper.GeometryHelper;
 
 /**
  * Our implementation of {@link MeshBuilder}, used for static mesh creation and baking.
@@ -69,12 +68,7 @@ public class MeshBuilderImpl implements MeshBuilder {
 	private class Maker extends MutableQuadViewImpl implements QuadEmitter {
 		@Override
 		public Maker emit() {
-			lightFace(GeometryHelper.lightFace(this));
-
-			if (isGeometryInvalid) {
-				geometryFlags(GeometryHelper.computeShapeFlags(this));
-			}
-
+			computeGeometry();
 			index += EncodingFormat.TOTAL_STRIDE;
 			ensureCapacity(EncodingFormat.TOTAL_STRIDE);
 			baseIndex = index;

fabric-renderer-indigo/src/main/java/net/fabricmc/fabric/impl/client/indigo/renderer/mesh/MutableQuadViewImpl.java

@@ -31,6 +31,7 @@ import static net.fabricmc.fabric.impl.client.indigo.renderer.mesh.EncodingForma
 
 import com.google.common.base.Preconditions;
 
+import net.minecraft.client.render.model.BakedQuad;
 import net.minecraft.client.texture.Sprite;
 import net.minecraft.util.math.Direction;
 
@@ -54,7 +55,6 @@ public abstract class MutableQuadViewImpl extends QuadViewImpl implements QuadEm
 
 	public void clear() {
 		System.arraycopy(EMPTY, 0, data, baseIndex, EncodingFormat.TOTAL_STRIDE);
-		isFaceNormalInvalid = true;
 		isGeometryInvalid = true;
 		nominalFace = null;
 		normalFlags(0);
@@ -81,13 +81,6 @@ public abstract class MutableQuadViewImpl extends QuadViewImpl implements QuadEm
 		return this;
 	}
 
-	public final MutableQuadViewImpl lightFace(Direction face) {
-		Preconditions.checkNotNull(face);
-
-		data[baseIndex + HEADER_BITS] = EncodingFormat.lightFace(data[baseIndex + HEADER_BITS], face);
-		return this;
-	}
-
 	@Override
 	public final MutableQuadViewImpl nominalFace(Direction face) {
 		nominalFace = face;
@@ -106,10 +99,27 @@ public abstract class MutableQuadViewImpl extends QuadViewImpl implements QuadEm
 		return this;
 	}
 
+	/**
+	 * @deprecated will be removed in 1.17 cycle - see docs in interface
+	 */
+	@Deprecated
 	@Override
 	public final MutableQuadViewImpl fromVanilla(int[] quadData, int startIndex, boolean isItem) {
 		System.arraycopy(quadData, startIndex, data, baseIndex + HEADER_STRIDE, QUAD_STRIDE);
-		this.invalidateShape();
+		isGeometryInvalid = true;
+		return this;
+	}
+
+	@Override
+	public final MutableQuadViewImpl fromVanilla(BakedQuad quad, RenderMaterial material, Direction cullFace) {
+		System.arraycopy(quad.getVertexData(), 0, data, baseIndex + HEADER_STRIDE, QUAD_STRIDE);
+		data[baseIndex + HEADER_BITS] = EncodingFormat.cullFace(0, cullFace);
+		nominalFace(quad.getFace());
+		colorIndex(quad.getColorIndex());
+		material(material);
+		tag(0);
+		shade(quad.hasShade());
+		isGeometryInvalid = true;
 		return this;
 	}
 
@@ -119,11 +129,11 @@ public abstract class MutableQuadViewImpl extends QuadViewImpl implements QuadEm
 		data[index] = Float.floatToRawIntBits(x);
 		data[index + 1] = Float.floatToRawIntBits(y);
 		data[index + 2] = Float.floatToRawIntBits(z);
-		isFaceNormalInvalid = true;
+		isGeometryInvalid = true;
 		return this;
 	}
 
-	public void normalFlags(int flags) {
+	protected void normalFlags(int flags) {
 		data[baseIndex + HEADER_BITS] = EncodingFormat.normalFlags(data[baseIndex + HEADER_BITS], flags);
 	}
 

fabric-renderer-indigo/src/main/java/net/fabricmc/fabric/impl/client/indigo/renderer/mesh/QuadViewImpl.java

@@ -48,9 +48,9 @@ import net.fabricmc.fabric.impl.client.indigo.renderer.helper.NormalHelper;
  */
 public class QuadViewImpl implements QuadView {
 	protected Direction nominalFace;
+	/** True when geometry flags or light face may not match geometry. */
 	protected boolean isGeometryInvalid = true;
 	protected final Vector3f faceNormal = new Vector3f();
-	protected boolean isFaceNormalInvalid = true;
 	private boolean shade = true;
 
 	/** Size and where it comes from will vary in subtypes. But in all cases quad is fully encoded to array. */
@@ -69,24 +69,16 @@ public class QuadViewImpl implements QuadView {
 		load();
 	}
 
-	/**
-	 * Used on vanilla quads or other quads that don't have encoded shape info
-	 * to signal that such should be computed when requested.
-	 */
-	public final void invalidateShape() {
-		isFaceNormalInvalid = true;
-		isGeometryInvalid = true;
-	}
-
 	/**
 	 * Like {@link #load(int[], int)} but assumes array and index already set.
 	 * Only does the decoding part.
 	 */
 	public final void load() {
-		// face normal isn't encoded but geometry flags are
-		isFaceNormalInvalid = true;
 		isGeometryInvalid = false;
 		nominalFace = lightFace();
+
+		// face normal isn't encoded
+		NormalHelper.computeFaceNormal(faceNormal, this);
 	}
 
 	/** Reference to underlying array. Use with caution. Meant for fast renderer access */
@@ -105,22 +97,22 @@ public class QuadViewImpl implements QuadView {
 
 	/** gets flags used for lighting - lazily computed via {@link GeometryHelper#computeShapeFlags(QuadView)}. */
 	public int geometryFlags() {
-		if (isGeometryInvalid) {
-			isGeometryInvalid = false;
-			final int result = GeometryHelper.computeShapeFlags(this);
-			data[baseIndex + HEADER_BITS] = EncodingFormat.geometryFlags(data[baseIndex + HEADER_BITS], result);
-			return result;
-		} else {
-			return EncodingFormat.geometryFlags(data[baseIndex + HEADER_BITS]);
-		}
+		computeGeometry();
+		return EncodingFormat.geometryFlags(data[baseIndex + HEADER_BITS]);
 	}
 
-	/**
-	 * Used to override geometric analysis for compatibility edge case.
-	 */
-	public void geometryFlags(int flags) {
-		isGeometryInvalid = false;
-		data[baseIndex + HEADER_BITS] = EncodingFormat.geometryFlags(data[baseIndex + HEADER_BITS], flags);
+	protected void computeGeometry() {
+		if (isGeometryInvalid) {
+			isGeometryInvalid = false;
+
+			NormalHelper.computeFaceNormal(faceNormal, this);
+
+			// depends on face normal
+			data[baseIndex + HEADER_BITS] = EncodingFormat.lightFace(data[baseIndex + HEADER_BITS], GeometryHelper.lightFace(this));
+
+			// depends on light face
+			data[baseIndex + HEADER_BITS] = EncodingFormat.geometryFlags(data[baseIndex + HEADER_BITS], GeometryHelper.computeShapeFlags(this));
+		}
 	}
 
 	@Override
@@ -145,6 +137,7 @@ public class QuadViewImpl implements QuadView {
 
 	@Override
 	public final Direction lightFace() {
+		computeGeometry();
 		return EncodingFormat.lightFace(data[baseIndex + HEADER_BITS]);
 	}
 
@@ -160,31 +153,20 @@ public class QuadViewImpl implements QuadView {
 
 	@Override
 	public final Vector3f faceNormal() {
-		if (isFaceNormalInvalid) {
-			NormalHelper.computeFaceNormal(faceNormal, this);
-			isFaceNormalInvalid = false;
-		}
-
+		computeGeometry();
 		return faceNormal;
 	}
 
 	@Override
 	public void copyTo(MutableQuadView target) {
+		computeGeometry();
+
 		final MutableQuadViewImpl quad = (MutableQuadViewImpl) target;
-		// copy everything except the header/material
+		// copy everything except the material
 		System.arraycopy(data, baseIndex + 1, quad.data, quad.baseIndex + 1, EncodingFormat.TOTAL_STRIDE - 1);
-		quad.isFaceNormalInvalid = this.isFaceNormalInvalid;
-
-		if (!this.isFaceNormalInvalid) {
-			quad.faceNormal.set(faceNormal.getX(), faceNormal.getY(), faceNormal.getZ());
-		}
-
-		quad.lightFace(lightFace());
-		quad.colorIndex(colorIndex());
-		quad.tag(tag());
-		quad.cullFace(cullFace());
+		quad.faceNormal.set(faceNormal.getX(), faceNormal.getY(), faceNormal.getZ());
 		quad.nominalFace = this.nominalFace;
-		quad.normalFlags(normalFlags());
+		quad.isGeometryInvalid = false;
 	}
 
 	@Override

fabric-renderer-indigo/src/main/java/net/fabricmc/fabric/impl/client/indigo/renderer/render/AbstractMeshConsumer.java

@@ -29,7 +29,6 @@ import net.fabricmc.fabric.api.renderer.v1.render.RenderContext.QuadTransform;
 import net.fabricmc.fabric.impl.client.indigo.renderer.IndigoRenderer;
 import net.fabricmc.fabric.impl.client.indigo.renderer.RenderMaterialImpl;
 import net.fabricmc.fabric.impl.client.indigo.renderer.aocalc.AoCalculator;
-import net.fabricmc.fabric.impl.client.indigo.renderer.helper.GeometryHelper;
 import net.fabricmc.fabric.impl.client.indigo.renderer.mesh.EncodingFormat;
 import net.fabricmc.fabric.impl.client.indigo.renderer.mesh.MeshImpl;
 import net.fabricmc.fabric.impl.client.indigo.renderer.mesh.MutableQuadViewImpl;
@@ -56,7 +55,7 @@ public abstract class AbstractMeshConsumer extends AbstractQuadRenderer implemen
 		// only used via RenderContext.getEmitter()
 		@Override
 		public Maker emit() {
-			lightFace(GeometryHelper.lightFace(this));
+			computeGeometry();
 			renderQuad(this);
 			clear();
 			return this;

fabric-renderer-indigo/src/main/java/net/fabricmc/fabric/impl/client/indigo/renderer/render/ItemRenderContext.java

@@ -45,9 +45,9 @@ import net.fabricmc.fabric.api.renderer.v1.mesh.QuadEmitter;
 import net.fabricmc.fabric.api.renderer.v1.model.FabricBakedModel;
 import net.fabricmc.fabric.api.renderer.v1.model.ModelHelper;
 import net.fabricmc.fabric.api.renderer.v1.render.RenderContext;
+import net.fabricmc.fabric.impl.client.indigo.renderer.IndigoRenderer;
 import net.fabricmc.fabric.impl.client.indigo.renderer.RenderMaterialImpl;
 import net.fabricmc.fabric.impl.client.indigo.renderer.helper.ColorHelper;
-import net.fabricmc.fabric.impl.client.indigo.renderer.helper.GeometryHelper;
 import net.fabricmc.fabric.impl.client.indigo.renderer.mesh.EncodingFormat;
 import net.fabricmc.fabric.impl.client.indigo.renderer.mesh.MeshImpl;
 import net.fabricmc.fabric.impl.client.indigo.renderer.mesh.MutableQuadViewImpl;
@@ -143,7 +143,7 @@ public class ItemRenderContext extends AbstractRenderContext implements RenderCo
 
 		@Override
 		public Maker emit() {
-			lightFace(GeometryHelper.lightFace(this));
+			computeGeometry();
 			renderQuad();
 			clear();
 			return this;
@@ -247,14 +247,7 @@ public class ItemRenderContext extends AbstractRenderContext implements RenderCo
 		final Maker editorQuad = this.editorQuad;
 
 		for (final BakedQuad q : quads) {
-			editorQuad.clear();
-			editorQuad.fromVanilla(q.getVertexData(), 0, false);
-			editorQuad.cullFace(cullFace);
-			final Direction lightFace = q.getFace();
-			editorQuad.lightFace(lightFace);
-			editorQuad.nominalFace(lightFace);
-			editorQuad.colorIndex(q.getColorIndex());
-			editorQuad.shade(q.hasShade());
+			editorQuad.fromVanilla(q, IndigoRenderer.MATERIAL_STANDARD, cullFace);
 			renderQuad();
 		}
 	}

fabric-renderer-indigo/src/main/java/net/fabricmc/fabric/impl/client/indigo/renderer/render/TerrainFallbackConsumer.java

@@ -87,45 +87,37 @@ public abstract class TerrainFallbackConsumer extends AbstractQuadRenderer imple
 		final BlockState blockState = blockInfo.blockState;
 
 		for (int i = 0; i < 6; i++) {
-			Direction face = ModelHelper.faceFromIndex(i);
-			List<BakedQuad> quads = model.getQuads(blockState, face, random.get());
+			final Direction face = ModelHelper.faceFromIndex(i);
+			final List<BakedQuad> quads = model.getQuads(blockState, face, random.get());
 			final int count = quads.size();
 
 			if (count != 0) {
 				for (int j = 0; j < count; j++) {
-					BakedQuad q = quads.get(j);
+					final BakedQuad q = quads.get(j);
 					renderQuad(q, face, defaultMaterial);
 				}
 			}
 		}
 
-		List<BakedQuad> quads = model.getQuads(blockState, null, random.get());
+		final List<BakedQuad> quads = model.getQuads(blockState, null, random.get());
 		final int count = quads.size();
 
 		if (count != 0) {
 			for (int j = 0; j < count; j++) {
-				BakedQuad q = quads.get(j);
+				final BakedQuad q = quads.get(j);
 				renderQuad(q, null, defaultMaterial);
 			}
 		}
 	}
 
 	private void renderQuad(BakedQuad quad, Direction cullFace, Value defaultMaterial) {
-		final int[] vertexData = quad.getVertexData();
-
-		if (!CompatibilityHelper.canRender(vertexData)) {
+		// TODO: should remove in 1.17 cycle, was for OF compat only
+		if (!CompatibilityHelper.canRender(quad.getVertexData())) {
 			return;
 		}
 
 		final MutableQuadViewImpl editorQuad = this.editorQuad;
-		System.arraycopy(vertexData, 0, editorBuffer, EncodingFormat.HEADER_STRIDE, EncodingFormat.QUAD_STRIDE);
-		editorQuad.cullFace(cullFace);
-		final Direction lightFace = quad.getFace();
-		editorQuad.lightFace(lightFace);
-		editorQuad.nominalFace(lightFace);
-		editorQuad.colorIndex(quad.getColorIndex());
-		editorQuad.material(defaultMaterial);
-		editorQuad.shade(quad.hasShade());
+		editorQuad.fromVanilla(quad, defaultMaterial, cullFace);
 
 		if (!transform.transform(editorQuad)) {
 			return;
@@ -139,14 +131,12 @@ public abstract class TerrainFallbackConsumer extends AbstractQuadRenderer imple
 
 		if (!editorQuad.material().disableAo(0)) {
 			// needs to happen before offsets are applied
-			editorQuad.invalidateShape();
 			aoCalc.compute(editorQuad, true);
 			tesselateSmooth(editorQuad, blockInfo.defaultLayer, editorQuad.colorIndex());
 		} else {
 			// Recomputing whether the quad has a light face is only needed if it doesn't also have a cull face,
 			// as in those cases, the cull face will always be used to offset the light sampling position
 			if (cullFace == null) {
-				editorQuad.invalidateShape();
 				// Can't rely on lazy computation in tesselateFlat() because needs to happen before offsets are applied
 				editorQuad.geometryFlags();
 			}