From 1866bc85ff717a5f234b766b232377880e79c185 Mon Sep 17 00:00:00 2001 From: atkurtul Date: Wed, 2 Sep 2026 18:36:35 +0300 Subject: [PATCH 1/4] Read canvas layers through the object API CanvasMapper took the Input array's data view, cast it to a flatbuffers vector and called size() on it. Ok() returns null when the pin holds no object, and the view it would have returned belongs to a temporary whose guard reference dies with the statement, so feeding the node from an Array node crashed it. Read every layer field through the object API instead. The loop also stops at the 16 layers the shader declares, indexes the blend mode bits by the packed draw position rather than the source position so a skipped layer no longer shifts them, and keeps each layer texture referenced until the pass is submitted. --- .../nosCompositing/Source/CanvasMapper.cpp | 98 ++++++++++++------- 1 file changed, 65 insertions(+), 33 deletions(-) diff --git a/Plugins/nosCompositing/Source/CanvasMapper.cpp b/Plugins/nosCompositing/Source/CanvasMapper.cpp index 7b5bf02d..90e86896 100644 --- a/Plugins/nosCompositing/Source/CanvasMapper.cpp +++ b/Plugins/nosCompositing/Source/CanvasMapper.cpp @@ -3,59 +3,93 @@ #include #include +#include + #include "nosCompositing/CanvasMapper_generated.h" #include "Names.h" namespace nos::compositing { -struct CanvasMapperContext : public NodeContext +// The shader declares fixed size arrays of this length, see Shaders/CanvasMapper.frag. +constexpr uint32_t MAX_CANVAS_LAYERS = 16; + +// Reads a POD field out of a layer object. 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. +template +static bool ReadLayerField(CompositeObjectRef& layer, nos::Name fieldName, T& out) { + auto field = layer.GetField(fieldName); + if (!field || !field->IsValid()) + return false; + auto view = field->GetObjectDataView(); + auto* buf = view.Ok(); + if (!buf || !buf->Data || buf->Size < sizeof(T)) + return false; + std::memcpy(&out, buf->Data, sizeof(T)); + return true; +} - nosResult ExecuteNode(nos::NodeExecuteParams const& params) +struct CanvasMapperContext : public NodeContext +{ + nosResult ExecuteNode(nos::NodeExecuteParams const& params) override { - auto arrayObj = nos::ArrayObjectRef(params.GetPinObject(NSN_Input)); - - auto inputs = (flatbuffers::Vector>*)arrayObj. - GetObjectDataView().Ok()->Data; + auto arrayObj = params.GetPinObject(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(NOS_NAME_STATIC("RGSS")); - std::array pos = {}; - std::array sca = {}; - std::array rot = {}; - std::array ori = {}; + std::array pos = {}; + std::array sca = {}; + std::array rot = {}; + std::array ori = {}; u32 ble = 0; - std::array opa = {}; + std::array opa = {}; std::vector textures; std::vector filters; + // Keeps the layer textures alive until the pass is submitted. + std::vector 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(i); + if (!layer || !layer->IsValid()) continue; + auto texture = layer->GetField(NOS_NAME_STATIC("texture")); + if (!texture || !texture->IsValid()) + continue; + + nos::fb::vec2u size{}; + if (!ReadLayerField(*layer, NOS_NAME_STATIC("size"), size) || 0 == size.x() || 0 == size.y()) + continue; + sca[last] = nos::fb::vec2(float(size.x()) / outputSize.x, float(size.y()) / outputSize.y); + + ReadLayerField(*layer, NOS_NAME_STATIC("position"), pos[last]); + ReadLayerField(*layer, NOS_NAME_STATIC("origin"), ori[last]); + ReadLayerField(*layer, NOS_NAME_STATIC("rotation"), rot[last]); + ReadLayerField(*layer, NOS_NAME_STATIC("opacity"), opa[last]); - auto texture = *arrayObj.GetElement(i)->GetField(NOS_NAME("texture")); - textures.push_back(texture); + u32 blendMode = 0; + ReadLayerField(*layer, NOS_NAME_STATIC("blend_mode"), blendMode); + // 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 +103,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 +128,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); From 7a10046722565e6c1100e3905db2df88d173ab50 Mon Sep 17 00:00:00 2001 From: atkurtul Date: Wed, 2 Sep 2026 18:36:46 +0300 Subject: [PATCH 2/4] Harden the Array node against missing pins and elements Four faults, all reachable from the editor. Removing an element whose name did not parse indexed the input vector with SIZE_MAX. The remove menu command dereferenced GetPin without checking it found anything. ExecuteNode dereferenced a pin's object pointer unconditionally. And both edit paths copied the array into the storage of the ObjectRef that already held the old array, overwriting the reference without releasing it, so every add and remove leaked one. Bounds check the index, check the pin, fail the execution rather than silently emitting a shorter array, and route both edits through ApplyArrayDelta, which writes to a fresh reference and moves it in so the previous one is released. Element pins are matched by name because they can be shown as properties and are still executed. --- Plugins/nosReflect/Source/Array.cpp | 52 ++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/Plugins/nosReflect/Source/Array.cpp b/Plugins/nosReflect/Source/Array.cpp index d615932c..0e4c2656 100644 --- a/Plugins/nosReflect/Source/Array.cpp +++ b/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 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::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 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 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 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); From 15910eb12e4358e92ea57bc4077671a584ab5733 Mon Sep 17 00:00:00 2001 From: atkurtul Date: Thu, 3 Sep 2026 13:54:49 +0300 Subject: [PATCH 3/4] Read layer fields through ObjectRef::GetValue The SDK already interprets an object's data view for a trivially copyable type, so the manual buffer handling was a reimplementation. --- Plugins/nosCompositing/Source/CanvasMapper.cpp | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/Plugins/nosCompositing/Source/CanvasMapper.cpp b/Plugins/nosCompositing/Source/CanvasMapper.cpp index 90e86896..757c2e64 100644 --- a/Plugins/nosCompositing/Source/CanvasMapper.cpp +++ b/Plugins/nosCompositing/Source/CanvasMapper.cpp @@ -3,8 +3,6 @@ #include #include -#include - #include "nosCompositing/CanvasMapper_generated.h" #include "Names.h" @@ -13,20 +11,19 @@ namespace nos::compositing // The shader declares fixed size arrays of this length, see Shaders/CanvasMapper.frag. constexpr uint32_t MAX_CANVAS_LAYERS = 16; -// Reads a POD field out of a layer object. 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. +// Reads a trivially copyable field out of a layer object. 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. template static bool ReadLayerField(CompositeObjectRef& layer, nos::Name fieldName, T& out) { auto field = layer.GetField(fieldName); - if (!field || !field->IsValid()) + if (!field) return false; - auto view = field->GetObjectDataView(); - auto* buf = view.Ok(); - if (!buf || !buf->Data || buf->Size < sizeof(T)) + auto* val = field->GetValue(); + if (!val) return false; - std::memcpy(&out, buf->Data, sizeof(T)); + out = *val; return true; } From 11aa7280be6cc256d42379262f4393a64cae3c06 Mon Sep 17 00:00:00 2001 From: atkurtul Date: Thu, 3 Sep 2026 14:18:43 +0300 Subject: [PATCH 4/4] Read canvas layer fields with GetFieldValue Drops the local helper now that the SDK reads a trivially copyable field in one call. --- .../nosCompositing/Source/CanvasMapper.cpp | 37 ++++++------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/Plugins/nosCompositing/Source/CanvasMapper.cpp b/Plugins/nosCompositing/Source/CanvasMapper.cpp index 757c2e64..c29b9142 100644 --- a/Plugins/nosCompositing/Source/CanvasMapper.cpp +++ b/Plugins/nosCompositing/Source/CanvasMapper.cpp @@ -11,22 +11,6 @@ namespace nos::compositing // The shader declares fixed size arrays of this length, see Shaders/CanvasMapper.frag. constexpr uint32_t MAX_CANVAS_LAYERS = 16; -// Reads a trivially copyable field out of a layer object. 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. -template -static bool ReadLayerField(CompositeObjectRef& layer, nos::Name fieldName, T& out) -{ - auto field = layer.GetField(fieldName); - if (!field) - return false; - auto* val = field->GetValue(); - if (!val) - return false; - out = *val; - return true; -} - struct CanvasMapperContext : public NodeContext { nosResult ExecuteNode(nos::NodeExecuteParams const& params) override @@ -69,18 +53,21 @@ struct CanvasMapperContext : public NodeContext if (!texture || !texture->IsValid()) continue; - nos::fb::vec2u size{}; - if (!ReadLayerField(*layer, NOS_NAME_STATIC("size"), size) || 0 == size.x() || 0 == size.y()) + // 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_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); + sca[last] = nos::fb::vec2(float(size->x()) / outputSize.x, float(size->y()) / outputSize.y); - ReadLayerField(*layer, NOS_NAME_STATIC("position"), pos[last]); - ReadLayerField(*layer, NOS_NAME_STATIC("origin"), ori[last]); - ReadLayerField(*layer, NOS_NAME_STATIC("rotation"), rot[last]); - ReadLayerField(*layer, NOS_NAME_STATIC("opacity"), opa[last]); + // A layer that leaves one of these unset is drawn with the zero the arrays were initialized with. + pos[last] = layer->GetFieldValue(NOS_NAME_STATIC("position")).value_or(nos::fb::vec2()); + ori[last] = layer->GetFieldValue(NOS_NAME_STATIC("origin")).value_or(nos::fb::vec2()); + rot[last] = layer->GetFieldValue(NOS_NAME_STATIC("rotation")).value_or(0.f); + opa[last] = layer->GetFieldValue(NOS_NAME_STATIC("opacity")).value_or(0.f); - u32 blendMode = 0; - ReadLayerField(*layer, NOS_NAME_STATIC("blend_mode"), blendMode); + u32 blendMode = layer->GetFieldValue(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;