Paint: saved meshes follow the paint, new values without the CSG (#65)

* Saved meshes are kept only while their paint is the same

The hash that decides whether a model keeps the meshes saved with its scene
(and so what a build takes) has each generator's paint and what the shader of
each of its materials takes of paint. Paint changed outside the editor, or a
shader that takes paint differently, builds the model when the scene opens
rather than keeping meshes with the old paint.

* Paint: new values are drawn without building a brush

When an entity's paint changes only in its values on the same grids, the
values are written in place, the hashes of the surfaces they are on are put
right in the brushes' cached buffers (the paint's part of a surface's hash
is xor-ed in last for that), and the next update copies the meshes again
without building a brush. New grids, a tree that kept its saved meshes, or
brushes not built yet still have the brushes built.

* The saved-mesh hash doesn't hold on to a model's paint

The lists the paint part of the hash is gathered in are cleared after it, so
a destroyed model's meshes aren't kept loaded through them.

* Paint: what a material takes is compared per material

Only a material resolved before whose paint attributes changed has every
brush built again; a material resolved for the first time (every one, when
the editor starts) is in the brushes being built with it, so models keep
their saved meshes when a scene opens. The paint manager's work set is a set,
emptied after each update so it holds no generator between updates.

Author: LogicalError

Components/Managers/ChiselModelManager.cs

@@ -828,6 +828,7 @@ public static Hash128 GetInputHash(ChiselModelComponent model)
 				return default;
 			AppendEnvironment(ref hash);
 			AppendHotspotInputs(ref hash, model);
+			AppendPaintInputs(ref hash, model);
 			return hash;
 		}
 
@@ -845,6 +846,7 @@ public static Hash128 GetComponentInputHash(ChiselModelComponent model)
 				return default;
 			AppendEnvironment(ref hash);
 			AppendHotspotInputs(ref hash, model);
+			AppendPaintInputs(ref hash, model);
 			return hash;
 		}
 
@@ -931,6 +933,66 @@ static void AppendHotspotInputs(ref Hash128 hash, ChiselModelComponent model)
 			s_HotspotMaterials.Clear();
 		}
 
+		static readonly List<ChiselGeneratorComponent>      s_PaintGenerators = new();
+		static readonly List<Material>                      s_PaintMaterials  = new();
+		static readonly List<ChiselPaintMaterialSlot>       s_PaintSlots      = new();
+
+		// What the meshes take of paint: each generator's paint, and what the shader of each material its surfaces are
+		// drawn with takes (the attributes a mesh has even where nothing is painted)
+		static void AppendPaintInputs(ref Hash128 hash, ChiselModelComponent model)
+		{
+			s_PaintGenerators.Clear();
+			s_PaintMaterials.Clear();
+			model.GetComponentsInChildren(true, s_PaintGenerators);
+			foreach (var generator in s_PaintGenerators)
+			{
+				var paints = generator.SurfacePaint;
+				var paintCount = paints.Count;
+				hash.Append(ref paintCount);
+				foreach (var paint in paints)
+				{
+					if (paint == null)
+						continue;
+					var header = new Unity.Mathematics.int4(paint.surfaceIndex, (int)paint.attribute, (int)paint.components, paint.values?.Length ?? 0);
+					hash.Append(ref header);
+					var defaultValue = paint.defaultValue; hash.Append(ref defaultValue);
+					var origin = paint.origin; hash.Append(ref origin);
+					var cellU  = paint.cellU;  hash.Append(ref cellU);
+					var cellV  = paint.cellV;  hash.Append(ref cellV);
+					var resolution = paint.resolution; hash.Append(ref resolution);
+					if (paint.values != null && paint.values.Length > 0)
+						hash.Append(paint.values);
+				}
+
+				var surfaces = generator.SurfaceDefinition?.surfaces;
+				if (surfaces == null)
+					continue;
+				foreach (var surface in surfaces)
+				{
+					var material = surface?.RenderMaterial;
+					if (!material || s_PaintMaterials.Contains(material))
+						continue;
+					s_PaintMaterials.Add(material);
+					s_PaintSlots.Clear();
+					ChiselPaintMaterials.Resolver?.Invoke(material, s_PaintSlots);
+					if (s_PaintSlots.Count == 0)
+						continue;
+					AppendMaterialIdentity(ref hash, material);
+					foreach (var slot in s_PaintSlots)
+					{
+						var key = new Unity.Mathematics.int2((int)slot.attribute, (int)slot.components);
+						hash.Append(ref key);
+						var defaultValue = slot.defaultValue;
+						hash.Append(ref defaultValue);
+					}
+				}
+			}
+			// nothing of the model is held on to
+			s_PaintGenerators.Clear();
+			s_PaintMaterials.Clear();
+			s_PaintSlots.Clear();
+		}
+
 		static void AppendMaterialIdentity(ref Hash128 hash, Material material)
 		{
 			var identity = material ? GetSurfaceParameterIdentity(EntityId.ToULong(material.GetEntityId())) : default;

Components/Managers/ChiselPaintManager.cs

@@ -24,7 +24,7 @@ struct Given
         static bool s_All = true;
         static readonly HashSet<ChiselGeneratorComponent>             s_Changed = new();
         static readonly Dictionary<ChiselGeneratorComponent, Given>   s_Given   = new();
-        static readonly List<ChiselGeneratorComponent>                s_Work    = new();
+        static readonly HashSet<ChiselGeneratorComponent>             s_Work    = new();
 
         /// <summary>Every generator's paint is looked at again (what shaders take may have changed)</summary>
         public static void SetDirty() => s_All = true;
@@ -64,15 +64,12 @@ public static void Update()
                 }
                 s_All = false;
             }
-            foreach (var generator in s_Changed)
-            {
-                if (!s_Work.Contains(generator))
-                    s_Work.Add(generator);
-            }
+            s_Work.UnionWith(s_Changed);
             s_Changed.Clear();
 
             foreach (var generator in s_Work)
                 Give(generator);
+            s_Work.Clear();
         }
 
         // The generator's paint given to its model's tree, and taken from the tree it was in before
@@ -86,7 +83,7 @@ static void Give(ChiselGeneratorComponent generator)
                 if (given)
                 {
                     // built again without it, if it is still there
-                    if (before.tree.Valid && before.tree.SetPaint(before.entityID, default, default) && generator)
+                    if (before.tree.Valid && before.tree.SetPaint(before.entityID, default, default) != ChiselPaintChange.None && generator)
                         SetBrushesDirty(generator.TopTreeNode);
                     s_Given.Remove(generator);
                 }
@@ -136,8 +133,9 @@ static void Give(ChiselGeneratorComponent generator)
                 v += paint.values.Length;
             }
             s_Given[generator] = new Given { entityID = entityID, tree = tree, space = space };
-            // the brushes of a generator whose paint changed are built again, with it
-            if (tree.SetPaint(entityID, layers, values))
+            // the brushes of a generator whose grids changed are built again, with them; new values on the same grids only
+            // have the meshes copied again
+            if (tree.SetPaint(entityID, layers, values) == ChiselPaintChange.Grids)
                 SetBrushesDirty(generator.TopTreeNode);
         }
 

Components/Tests/ChiselPaintTests.cs

@@ -458,6 +458,67 @@ public void AGridWithoutItsValues_IsMadeAgainWhenPainted()
             Assert.That(box.GetSurfacePaint(top, ChiselPaintAttribute.Color), Is.SameAs(paint));
         }
 
+        // A scene keeps its saved meshes only while their paint is the same: the paint and what the shaders take of it are
+        // in the hash that decides it
+        [Test]
+        public void TheSavedMeshesInputHash_HasThePaintAndWhatTheShaderTakes()
+        {
+            var model = Model();
+            var box   = Box(model, Vector3.zero);
+            Update();
+            var unpainted = ChiselModelManager.GetComponentInputHash(model);
+
+            var paint = PaintTop(box, ChiselPaintAttribute.Color, ChiselPaintComponents.XYZW);
+            Update();
+            var painted = ChiselModelManager.GetComponentInputHash(model);
+            Assert.That(painted, Is.Not.EqualTo(unpainted), "painted");
+
+            paint[0, 0] = new Vector4(0.5f, 0.5f, 0.5f, 1);
+            Assert.That(ChiselModelManager.GetComponentInputHash(model), Is.Not.EqualTo(painted), "a value changed");
+            var repainted = ChiselModelManager.GetComponentInputHash(model);
+
+            ChiselPaintProfiles.Unregister(profile);
+            Assert.That(ChiselModelManager.GetComponentInputHash(model), Is.Not.EqualTo(repainted), "the shader takes no paint");
+            ChiselPaintProfiles.Register(profile);
+            Assert.That(ChiselModelManager.GetComponentInputHash(model), Is.EqualTo(repainted), "and takes it again");
+        }
+
+        // The buffer the box's brush was last built into
+        static Unity.Entities.BlobAssetReference<ChiselBrushRenderBuffer> BuiltBuffer(ChiselModelComponent model, ChiselBoxComponent box)
+        {
+            var entityID = EntityId.ToULong(box.GetEntityId());
+            var cache = ChiselTreeLookup.Value[model.Node].brushRenderBufferCache;
+            for (int b = 0; b < cache.Length; b++)
+            {
+                if (cache[b].IsCreated && cache[b].Value.entityID == entityID)
+                    return cache[b];
+            }
+            Assert.Fail("the box's brush has no buffer");
+            return default;
+        }
+
+        // New values on the same grid are copied into the mesh without building the brush again; a new grid builds it
+        [Test]
+        public void NewValues_AreDrawnWithoutBuildingTheBrush()
+        {
+            var model = Model();
+            var box   = Box(model, Vector3.zero);
+            var paint = PaintTop(box, ChiselPaintAttribute.Color, ChiselPaintComponents.XYZW);
+            Update();
+            var built = BuiltBuffer(model, box);
+
+            paint[0, 0] = new Vector4(0, 1, 1, 1);
+            box.PaintChanged();
+            Update();
+            AssertValuesAt(RenderMesh(model), VertexAttribute.Color, new Vector3(0, 1, 0), new Vector4(0, 1, 1, 1), "the new value");
+            Assert.That(BuiltBuffer(model, box), Is.EqualTo(built), "the brush was built again for new values");
+
+            box.SetSurfacePaint(ChiselSurfacePaint.Create(paint.surfaceIndex, paint.attribute, paint.components, Vector4.one,
+                                                          paint.origin, paint.cellU * 0.5f, paint.cellV * 0.5f, new Vector2Int(3, 3)));
+            Update();
+            Assert.That(BuiltBuffer(model, box), Is.Not.EqualTo(built), "a new grid didn't build the brush");
+        }
+
         // The grid moves with its generator
         [Test]
         public void Paint_MovesWithItsGenerator()

Components/Tests/ChiselSavedOutputTests.cs

@@ -223,6 +223,84 @@ public void AModelThatChangedWhileClosed_IsBuiltWhenItOpens()
             Assert.That(IsMarked(models[1]), Is.True);
         }
 
+        // Box 0's surfaces drawn with a material whose shader takes color paint, and its top painted red
+        ChiselSurfacePaint PaintBox0(out Material material, out ChiselPaintProfile profile)
+        {
+            material = new Material(Shader.Find("Universal Render Pipeline/Lit"));
+            profile  = new ChiselPaintProfile { shader = material.shader, modes = { ChiselPaintMode.Color("Color") } };
+            ChiselPaintProfiles.Register(profile);
+            foreach (var surface in boxes[0].SurfaceDefinition.surfaces)
+                surface.SetMaterial(material);
+            boxes[0].OnValidate();
+            Update();
+            var top = 0;
+            var brush = (CSGTreeBrush)boxes[0].TopTreeNode;
+            ref var mesh = ref BrushMeshManager.GetBrushMeshBlob(brush.BrushMesh).Value;
+            for (int s = 0; s < mesh.polygons.Length; s++)
+            {
+                if (mesh.localPlanes[s].y > 0.999f)
+                    top = mesh.polygons[s].descriptionIndex;
+            }
+            var paint = ChiselPaintBrush.PaintFor(boxes[0], top, profile.modes[0]);
+            ChiselPaintBrush.Fill(paint, profile.modes[0], new Vector4(1, 0, 0, 1));
+            boxes[0].PaintChanged();
+            Update();
+            return paint;
+        }
+
+        static bool HasRed(ChiselModelComponent model)
+        {
+            foreach (var color in RenderableWithGeometry(model).sharedMesh.colors)
+            {
+                if (color.r > 0.99f && color.g < 0.01f)
+                    return true;
+            }
+            return false;
+        }
+
+        // A painted model keeps its saved meshes, and with them its paint
+        [Test]
+        public void APaintedModel_KeepsItsSavedMeshes_WithTheirPaint()
+        {
+            PaintBox0(out var material, out var profile);
+            try
+            {
+                Assert.That(HasRed(models[0]), Is.True, "painted");
+                Save();
+                MarkSavedMeshes();
+                Reopen();
+                Assert.That(CompactHierarchyManager.IsTreeUpdateSkipped(models[0].Node), Is.True);
+                Assert.That(IsMarked(models[0]), Is.True, "the painted model was built again");
+                Assert.That(HasRed(models[0]), Is.True, "the saved mesh has the paint");
+            }
+            finally
+            {
+                ChiselPaintProfiles.Unregister(profile);
+                UnityEngine.Object.DestroyImmediate(material);
+            }
+        }
+
+        // Paint changed while the scene was closed is built when it opens
+        [Test]
+        public void PaintThatChangedWhileClosed_IsBuiltWhenItOpens()
+        {
+            var paint = PaintBox0(out var material, out var profile);
+            try
+            {
+                Save();
+                MarkSavedMeshes();
+                Reopen(whileClosed: () => ChiselPaintBrush.Fill(paint, profile.modes[0], new Vector4(0, 0, 1, 1)));
+                Assert.That(IsMarked(models[0]), Is.False, "the repainted model kept its old meshes");
+                Assert.That(IsMarked(models[1]), Is.True);
+                Assert.That(HasRed(models[0]), Is.False, "the old paint");
+            }
+            finally
+            {
+                ChiselPaintProfiles.Unregister(profile);
+                UnityEngine.Object.DestroyImmediate(material);
+            }
+        }
+
         [Test]
         public void AModelSavedWithoutAHash_IsBuiltWhenItOpens()
         {

Core/1.Input/Paint/ChiselPaint.cs

@@ -148,12 +148,13 @@ public static bool SetPaint(this CSGTree tree, NativeArray<ChiselPaintLayer> lay
         }
 
         /// <summary>
-        /// Replaces the paint of one entity: its layers, and the values their grids take from. True when it changed; the
-        /// entity's brushes then have to be rebuilt to show it (<see cref="CSGTreeNode.SetSurfacesDirty"/>).
+        /// Replaces the paint of one entity: its layers, and the values their grids take from. When only values changed on
+        /// the same grids the meshes are copied again at the next update without building a brush; when the grids changed
+        /// the entity's brushes have to be built again to show it (<see cref="CSGTreeNode.SetSurfacesDirty"/>).
         /// </summary>
-        public static bool SetPaint(this CSGTree tree, ulong entityID, NativeArray<ChiselPaintLayer> layers, NativeArray<float> values)
+        public static ChiselPaintChange SetPaint(this CSGTree tree, ulong entityID, NativeArray<ChiselPaintLayer> layers, NativeArray<float> values)
         {
-            return ChiselPaintStore.SetPaint(tree, entityID, layers, values);
+            return ChiselPaintStore.SetPaintOrValues(tree, entityID, layers, values);
         }
     }
 }

Core/1.Input/Paint/ChiselPaintMaterials.cs

@@ -28,7 +28,8 @@ public static class ChiselPaintMaterials
         static readonly Dictionary<ulong, Material>       s_Materials = new();
         static readonly List<ChiselPaintMaterialSlot>     s_Slots     = new();
         static readonly List<ChiselPaintMaterialSlot>     s_Resolved  = new();
-        static readonly List<ChiselPaintMaterialSlot>     s_Scratch   = new();
+        static readonly Dictionary<ulong, uint>           s_Before    = new();
+        static readonly Dictionary<ulong, uint>           s_After     = new();
         static readonly List<ulong>                       s_Gone      = new();
         static bool s_Dirty;
         static int  s_Version;
@@ -45,22 +46,28 @@ public static void Register(Material material)
         /// <summary>What shaders take may have changed; looked into at the next <see cref="Update"/></summary>
         public static void Changed() => s_Changed = true;
 
-        /// <summary>Before the trees are built: every brush is built again when a registered material's attributes changed</summary>
+        /// <summary>Before the trees are built: every brush is built again when what a material already resolved takes
+        /// changed (a material resolved for the first time is in the brushes being built with it)</summary>
         public static void Update()
         {
             if (!s_Changed)
                 return;
             s_Changed = false;
             s_Dirty = true;
-            var before = s_Slots.Count;
-            s_Scratch.Clear();
-            s_Scratch.AddRange(s_Slots);
+            s_Before.Clear();
+            HashByMaterial(s_Slots, s_Before);
             Resolve();
-            var same = before == s_Slots.Count;
-            for (int i = 0; same && i < before; i++)
+            s_After.Clear();
+            HashByMaterial(s_Slots, s_After);
+            var same = true;
+            foreach (var entry in s_Before)
             {
-                same = s_Scratch[i].material == s_Slots[i].material && s_Scratch[i].attribute == s_Slots[i].attribute &&
-                       s_Scratch[i].components == s_Slots[i].components && s_Scratch[i].defaultValue.Equals(s_Slots[i].defaultValue);
+                s_After.TryGetValue(entry.Key, out var after);
+                if (after != entry.Value)
+                {
+                    same = false;
+                    break;
+                }
             }
             if (same)
                 return;
@@ -73,6 +80,15 @@ public static void Update()
             }
         }
 
+        static void HashByMaterial(List<ChiselPaintMaterialSlot> slots, Dictionary<ulong, uint> hashes)
+        {
+            foreach (var slot in slots)
+            {
+                hashes.TryGetValue(slot.material, out var hash);
+                hashes[slot.material] = math.hash(new uint4(hash, (uint)slot.attribute, (uint)slot.components, math.hash(math.asuint(slot.defaultValue))));
+            }
+        }
+
         static void SetBrushesDirty(CSGTreeNode node)
         {
             if (!node.Valid)

Core/2.Processing/Cache/ChiselLookup.cs

@@ -49,6 +49,8 @@ public class Data
             public NativeList<ChiselPaintMaterialSlot>                          paintMaterialSlots;
             public NativeParallelHashMap<ulong, ChiselPaintRange>               paintMaterials;
             public int                                                          paintMaterialsVersion = -1;
+            // paint values changed in place: the meshes are copied again without building a brush
+            public bool                                                         paintValuesChanged;
 
             public NativeParallelHashMap<CompactNodeID, BlobAssetReference<ChiselBrushRenderBuffer>> brushRenderBufferLookup;
 

Core/2.Processing/Jobs/GenerateSurfaceTrianglesJob.cs

@@ -312,16 +312,17 @@ unsafe void FinishSurface(int surf, int brushNodeOrder, ulong entityID, ref Blob
 			buf.hotspot       = hotspotInfo;
 			buf.hotspotSource = hotspotSource;
 
-			// paint is found by the brush's entity and the surface's index, when the surface's mesh is copied
+			// the attributes its material's shader takes paint in are in its mesh, painted or not
+			if (paint.TryGetMaterial(parms.parameter1, out var materialRange))
+				buf.surfaceHashValue = math.hash(new uint2(buf.surfaceHashValue, materialRange.hash));
+			// paint is found by the brush's entity and the surface's index, when the surface's mesh is copied; its hash
+			// goes in last, by xor, so new values can replace it in place (ChiselPaintStore.SetValues)
 			var paintSurface = baseSurfaces[surf].descriptionIndex;
 			if (paint.TryGetRange(entityID, paintSurface, out var paintRange))
 			{
 				buf.paintSurface     = paintSurface;
-				buf.surfaceHashValue = math.hash(new uint2(buf.surfaceHashValue, paintRange.hash));
+				buf.surfaceHashValue ^= paintRange.hash;
 			}
-			// the attributes its material's shader takes paint in are in its mesh, painted or not
-			if (paint.TryGetMaterial(parms.parameter1, out var materialRange))
-				buf.surfaceHashValue = math.hash(new uint2(buf.surfaceHashValue, materialRange.hash));
 
 			if (HasBackFace(flags, outputFlags))
 				FinishBackFace(surf, entityID, flags, parms, outputFlags, surfaceIndexList, ref uniqueVertexMapper, builder, surfaceBuffers, slots);