Fix Array and CanvasMapper crashes

Plugins/nosCompositing/Source/CanvasMapper.cpp

@@ -8,54 +8,72 @@
 
 namespace nos::compositing
 {
+// The shader declares fixed size arrays of this length, see Shaders/CanvasMapper.frag.
+constexpr uint32_t MAX_CANVAS_LAYERS = 16;
+
 struct CanvasMapperContext : public NodeContext
 {
-
-	nosResult ExecuteNode(nos::NodeExecuteParams const& params)
+	nosResult ExecuteNode(nos::NodeExecuteParams const& params) override
 	{
-		auto arrayObj = nos::ArrayObjectRef(params.GetPinObject(NSN_Input));
-
-		auto inputs = (flatbuffers::Vector<flatbuffers::Offset<nos::compositing::CanvasLayer>>*)arrayObj.
-			GetObjectDataView().Ok()->Data;
+		auto arrayObj = params.GetPinObject<ArrayObjectRef>(NSN_Input);
+		if (!arrayObj.IsValid())
+			return NOS_RESULT_SUCCESS;
 
-		if (0 == inputs->size())
+		size_t layerCount = arrayObj.GetSize();
+		if (0 == layerCount)
 			return NOS_RESULT_SUCCESS;
 
-		int size = inputs->size();
+		auto outputInfo = sys::vulkan::GetResourceInfo(params.GetPinObject(NSN_Output));
+		if (!outputInfo || outputInfo->Type != NOS_RESOURCE_TYPE_TEXTURE)
+			return NOS_RESULT_FAILED;
+		auto outputSize = glm::vec2(outputInfo->Texture.Width, outputInfo->Texture.Height);
+		if (0 == outputSize.x || 0 == outputSize.y)
+			return NOS_RESULT_FAILED;
 
-		auto output = *nos::sys::vulkan::GetResourceInfo(params.GetPinObject(NSN_Output));
-		auto outputSize = glm::vec2(output.Texture.Width, output.Texture.Height);
 		auto rgss = *params.GetPinValue<bool>(NOS_NAME_STATIC("RGSS"));
 
-		std::array<nos::fb::vec2, 16> pos = {};
-		std::array<nos::fb::vec2, 16> sca = {};
-		std::array<float, 16> rot = {};
-		std::array<nos::fb::vec2, 16> ori = {};
+		std::array<nos::fb::vec2, MAX_CANVAS_LAYERS> pos = {};
+		std::array<nos::fb::vec2, MAX_CANVAS_LAYERS> sca = {};
+		std::array<float, MAX_CANVAS_LAYERS> rot = {};
+		std::array<nos::fb::vec2, MAX_CANVAS_LAYERS> ori = {};
 		u32 ble = 0;
-		std::array<float, 16> opa = {};
+		std::array<float, MAX_CANVAS_LAYERS> opa = {};
 		std::vector<nosTextureObject> textures;
 		std::vector<nosTextureFilter> filters;
+		// Keeps the layer textures alive until the pass is submitted.
+		std::vector<ObjectRef> textureRefs;
 
 		u32 last = 0;
-
-		for (u32 i = 0; i < inputs->size(); ++i)
+		for (size_t i = 0; i < layerCount && last < MAX_CANVAS_LAYERS; ++i)
 		{
-			auto layer = inputs->Get(i);
-			if (!layer->texture())
+			auto layer = arrayObj.GetElement<CompositeObjectRef>(i);
+			if (!layer || !layer->IsValid())
+				continue;
+			auto texture = layer->GetField(NOS_NAME_STATIC("texture"));
+			if (!texture || !texture->IsValid())
 				continue;
 
-			auto texture = *arrayObj.GetElement<nos::CompositeObjectRef>(i)->GetField(NOS_NAME("texture"));
-			textures.push_back(texture);
+			// Read fields through the object API rather than casting the array's data view to a flatbuffers vector:
+			// the view is absent when the pin holds no object, and it belongs to a temporary whose guard reference
+			// dies with the statement that produced it.
+			auto size = layer->GetFieldValue<nos::fb::vec2u>(NOS_NAME_STATIC("size"));
+			if (!size || 0 == size->x() || 0 == size->y())
+				continue;
+			sca[last] = nos::fb::vec2(float(size->x()) / outputSize.x, float(size->y()) / outputSize.y);
+
+			// A layer that leaves one of these unset is drawn with the zero the arrays were initialized with.
+			pos[last] = layer->GetFieldValue<nos::fb::vec2>(NOS_NAME_STATIC("position")).value_or(nos::fb::vec2());
+			ori[last] = layer->GetFieldValue<nos::fb::vec2>(NOS_NAME_STATIC("origin")).value_or(nos::fb::vec2());
+			rot[last] = layer->GetFieldValue<float>(NOS_NAME_STATIC("rotation")).value_or(0.f);
+			opa[last] = layer->GetFieldValue<float>(NOS_NAME_STATIC("opacity")).value_or(0.f);
+
+			u32 blendMode = layer->GetFieldValue<u32>(NOS_NAME_STATIC("blend_mode")).value_or(0);
+			// The shader tests one bit per drawn layer, so index by the packed position, not the source index.
+			ble |= (blendMode & 1u) << last;
+
+			textureRefs.push_back(std::move(*texture));
+			textures.push_back(textureRefs.back());
 			filters.push_back(NOS_TEXTURE_FILTER_LINEAR);
-			pos[last] = *layer->position();
-			rot[last] = layer->rotation();
-			ori[last] = *layer->origin();
-			sca[last] = nos::fb::vec2(
-				float(layer->size()->x()) / outputSize.x,
-				float(layer->size()->y()) / outputSize.y
-				);
-			ble |= u32(layer->blend_mode()) << i;
-			opa[last] = layer->opacity();
 			last++;
 		}
 
@@ -69,7 +87,7 @@ struct CanvasMapperContext : public NodeContext
 				NOS_NAME_STATIC("Textures"),
 				textures.data(),
 				filters.data(),
-				(u32)textures.size()),
+				count),
 			nos::sys::vulkan::ShaderDataBinding(NOS_NAME_STATIC("OutputSize"), outputSize),
 			nos::sys::vulkan::ShaderDataBinding(NOS_NAME_STATIC("BackgroundColor"), backgroundColor),
 			nos::sys::vulkan::ShaderDataBinding(NOS_NAME_STATIC("Positions"), pos),
@@ -94,10 +112,8 @@ struct CanvasMapperContext : public NodeContext
 		nosVulkan->End(cmd, 0);
 		return NOS_RESULT_SUCCESS;
 	}
-
 };
 
-
 void RegisterCanvasMapper(nosNodeFunctions* nodeFunctions)
 {
 	NOS_BIND_NODE_CLASS(NOS_NAME_STATIC("CanvasMapper"), CanvasMapperContext, nodeFunctions);

Plugins/nosReflect/Source/Array.cpp

@@ -156,8 +156,8 @@ struct ArrayNode : NodeContext
 			return NOS_RESULT_FAILED;
 
 		auto& rawParams = *params.RawParams;
-		size_t arrayIndex = 0;
 		std::vector<nosObjectId> inputObjects;
+		inputObjects.reserve(rawParams.PinCount);
 		nosName outputTypeName{};
 		for (size_t pinIndex = 0; pinIndex < rawParams.PinCount; pinIndex++)
 		{
@@ -167,13 +167,23 @@ struct ArrayNode : NodeContext
 				outputTypeName = pin->TypeName;
 				continue;
 			}
+			// Only the element pins contribute to the array. Match them by name: an element pin can be shown as a
+			// property rather than an input pin, and it is still executed, so ShowAs must not be used to identify one.
+			if (GetInputElementIndexFromName(pin->Name) == std::numeric_limits<size_t>::max())
+				continue;
+			if (!pin->Object)
+			{
+				// Skipping would shift every later element, and the remove requests index the array by pin name.
+				nosEngine.LogE("Array: Input pin %s has no object", nos::Name(pin->Name).AsCStr());
+				return NOS_RESULT_FAILED;
+			}
 			inputObjects.push_back(*pin->Object);
-			arrayIndex++;
 		}
-		
+
 		ObjectRef newArrayObject{};
-		nosEngine.ObjectAPI->CreateArrayObject(outputTypeName, inputObjects.data(), inputObjects.size(), &newArrayObject.GetStorage());
-		if (!newArrayObject.IsValid())
+		auto res = nosEngine.ObjectAPI->CreateArrayObject(
+			outputTypeName, inputObjects.data(), inputObjects.size(), &newArrayObject.GetStorage());
+		if (res != NOS_RESULT_SUCCESS || !newArrayObject.IsValid())
 			return NOS_RESULT_FAILED;
 		ArrayObject = std::move(newArrayObject);
 		SetPinObject(NSN_Output, ArrayObject);
@@ -246,8 +256,7 @@ struct ArrayNode : NodeContext
 				.Element = newElement
 			}
 		};
-		nosEngine.ObjectAPI->CopyArrayObjectWithEdits(ArrayObject, &delta, 1, &ArrayObject.GetStorage());
-		SetPinObject(NSN_Output, ArrayObject);
+		ApplyArrayDelta(delta);
 	}
 
 	void SendRemoveElementRequest(std::optional<size_t> elementIndex = std::nullopt) {
@@ -261,6 +270,12 @@ struct ArrayNode : NodeContext
 		if (!elementIndex)
 			elementIndex = inputs.size() - 1;
 
+		if (*elementIndex >= inputs.size())
+		{
+			nosEngine.LogE("Array: No element at index %zu to remove", *elementIndex);
+			return;
+		}
+
 		std::vector<fb::UUID> id = { inputs[*elementIndex]->Id};
 		HandleEvent(
 			CreateAppEvent(fbb, CreatePartialNodeUpdateDirect(fbb, &NodeId, ClearFlags::NONE, &id)));
@@ -271,7 +286,23 @@ struct ArrayNode : NodeContext
 				.Index = *elementIndex
 			}
 		};
-		nosEngine.ObjectAPI->CopyArrayObjectWithEdits(ArrayObject, &delta, 1, &ArrayObject.GetStorage());
+		ApplyArrayDelta(delta);
+	}
+
+	// Publishes an edited copy of the output array. Writing into ArrayObject's own storage would leak the reference it
+	// already holds, so the copy goes to a fresh ref and is moved in.
+	void ApplyArrayDelta(nosArrayObjectDelta const& delta)
+	{
+		if (!ArrayObject.IsValid())
+			return; // Nothing published yet, the next execution rebuilds the array from the pins.
+		ObjectRef edited{};
+		auto res = nosEngine.ObjectAPI->CopyArrayObjectWithEdits(ArrayObject, &delta, 1, &edited.GetStorage());
+		if (res != NOS_RESULT_SUCCESS || !edited.IsValid())
+		{
+			nosEngine.LogE("Array: Failed to apply edit to the output array");
+			return;
+		}
+		ArrayObject = std::move(edited);
 		SetPinObject(NSN_Output, ArrayObject);
 	}
 
@@ -288,7 +319,10 @@ struct ArrayNode : NodeContext
 		{
 			std::optional<size_t> elementIndx = std::nullopt;
 			if (itemId != NodeId) {
-				if (auto pinName = GetPin(itemId)->DisplayName; pinName != NSN_Output)
+				auto* pin = GetPin(itemId);
+				if (!pin)
+					return;
+				if (auto pinName = pin->DisplayName; pinName != NSN_Output)
 					elementIndx = GetInputElementIndexFromName(pinName);
 			}
 			SendRemoveElementRequest(elementIndx);