Faces/States/SubModels node numbers don't match the Custom grid / Wiring View when a node number is skipped

[heffneil]

Summary

On a Custom model that skips a node number (the usual way to leave a null/unaddressed pixel mid-string), the Faces, States and SubModels editors interpret a typed node number differently from the Custom grid and Wiring View.

Wiring View and the grid treat the number as a channel slot (gap-aware). Faces / States / SubModels treat it as an ordinal position in the list of defined nodes (gap-blind). They agree on a gapless model and drift apart by one for every skipped number.

Net effect: on a model numbered 1…9, 11, 12, typing 11 into a Face, State or SubModel node range selects grid node 12.

Steps to reproduce

1. Create a Custom model and number the cells 1,2,3,4,5,6,7,8,9,11,12 — deliberately skipping 10 so pixel 10 is a null pixel.
2. Open Wiring View. The eleventh physical pixel is labelled 11, matching what you typed.
3. Open Model Definitions → Faces, choose a Node Ranges face, and enter 11 for a face part.
4. The highlighted/selected node is the one the grid calls 12, not 11.

Same behaviour in States, and in SubModels node ranges.

Why (with references, all against 2026.15.1 / current master)

The grid number is the channel slot — a skipped number keeps its channels.
src-core/models/CustomModel.cpp:718

Nodes.back()->ActChan = firstStartChan + idx * cpn;   // idx = (typed number - 1)

So 11 gets the 11th slot and the channels for the skipped 10 stay reserved. That is the desired null-pixel behaviour — the physical pixel still consumes its bytes.

But the node vector is compacted — it holds only the cells you filled:
src-core/models/CustomModel.cpp:712

nodemap[idx] = Nodes.size();

Wiring View deliberately works back from the channel, and the existing comment states exactly why:
src-ui-wx/model/WiringDialog.cpp:205-206

// because a custom mdoel can skip nodes we need to reverse engineer the node number
stringnode = 1 + (nodeList[i]->ActChan - nodeList[0]->ActChan) / nodeList[0]->GetChanCount();

Faces / States / SubModels index the compacted vector instead:

• Model::ParseFaceNodes() — src-core/models/Model.cpp:971 — decrements to a 0-based value, which is then used directly as a node index (ModelFacesPanel.cpp:1148, :1224 → Model::SetNodeColor()).
• States — ModelStatesPanel.cpp:710, :834 → same SetNodeColor() path.
• SubModels — SubModel::initRangeXY() — src-core/models/SubModel.cpp:385, resolving via parent->Nodes[nn] at :436.

So with grid 1…9, 11, 12:

Nodes[] index	0	…	8	9	10
grid number	1	…	9	11	12

11 → Nodes[10] → grid node 12.

Which surfaces agree

Surface	Number derived from	Skipped numbers
Custom model grid	the number is the channel slot	reserves channels
Wiring View	1 + (ActChan − first) / chanCount	aware
Node Layout, tooltips	Model::GetNodeNumber() (Model.cpp:2429)	aware
Faces (node ranges)	Nodes[n − 1]	blind
States	Nodes[n − 1]	blind
SubModels	parent->Nodes[n − 1]	blind

The three gap-blind surfaces are precisely the ones where a user types numbers read off the model, which is why this reads as a bug rather than an intentional convention.

The part that needs a decision before any fix

Making Faces / States / SubModels resolve by node number would be the intuitive behaviour, but it would silently relocate every existing definition on a gapped model — someone who worked around this by entering ordinals would have their faces move.

So it probably needs a migration rather than a straight change, e.g. version-stamp existing face/state/submodel definitions and convert old ordinal ranges to node numbers on load, leaving gapless models (the overwhelming majority) untouched either way.

Happy to put up a PR, but the migration strategy seems worth agreeing on first — and it's also possible the current behaviour is intentional for reasons I haven't found, in which case labelling the fields as positions would be the cheaper fix.

Also noticed nearby (not the main report)

Model::GetNodeNumber() at src-core/models/Model.cpp:2429 divides the channel offset by a hardcoded 3:

return (Nodes[nodenum]->ActChan - stringStartChan[sn]) / 3 + sn * NodesPerString() + 1;

That looks wrong for single-colour (1 channel/node) and RGBW (4) models. It also indexes stringStartChan with Nodes[nodenum]->StringNum, while CustomModel::InitCustomMatrix() sets StringNum = idx (the node index, CustomModel.cpp:715-ish) rather than a string number — which looks like it could read out of bounds on a single-string custom model. I have not confirmed either of these, so treat it as a lead rather than a claim; happy to split it into its own issue if it holds up.

Environment

• xLights 2026.15.1 (current master)
• macOS, but the code paths involved are platform-independent

[computergeek1507]

I am pretty sure you have asked for this before, skipping nodes is not recommended and is undefined behavior.

[computergeek1507]

I personally would not allow skipping nodes, but for backwards compatibility it probably can't be removed

[heffneil]

I have and we briefly discussed it in the zoom room and would love to discuss it more. I understand it should be discouraged but I think a lot of us agree that as seeds become more popular people are using closer spaced seeds and those are difficult to splice or are unwilling to splice premade lines. The alternative is that you can skip which is "cleaner" on a physcial level but a problem in xlights.

I know there is the alternative submodel idea which isn't a bad one the problem is year to year and sequence by sequence you have to remember to map the effects ONLY to the submodels and not the main prop. I guess a renaming of the main prop to like DO NOT USE is a solution but seems like it isn't elegant.

Happy to continue the conversation and I appreciate the response.

[deplanche]

I think a better solution would be to determine a way to have Null pixels be able to be defined in the middle of a string, rather than just at the start or end. This would allow a native matrix to be use for a matrix or mega tree model, where you can't run from one strand to the next without a splice. In a custom model, you wouldn't leave out numbers for pixels not being used, you would only include the ones you are using. So if you needed to null pixels to make the jump from hole 10 to hole 11, you would say you have 2 null pixels after node 10. Then xlights would push to a controller that you have a string with 10 nodes and 2 null pixels at the end, then another string with however many pixels. Think of it like a model that spans multiple ports, where you put it on the first port and it goes vertically down to the rest. In this case it would go horizontally across a single port, just adding in the nulls where needed.

Of course, that would only work for controller that support null pixels, so can't get rid of skipping nodes in a custom model, but would be easier for those with more modern controllers.

[derwin12]

thought the same thing Jon, but what do you do for folks with seed trees and want to add a bunch along the bottom to skip around to the next strand?

[cybercop23]

I think this is a 2 part problem, creation and maintenance. Sure I can create and manage (somehow) the extra nodes and say I manage Faces/States and SubModels one time. But when the vendor updates any of them, how can one simply import them... they can't, the model is not the same.
If we add Skipped pixels, to the model properties, and use that as a way to adjust the "stream", will that make it no model data is changed or any faces/states or submodels?

[heffneil]

I think its fair to say if you modified the original model to work for your wiring outside of the suggestion, you're not going to import updated models from the vendor. You are now on your own? I don't expect someones update model to apply to my cockamamy custom model I modified?

I WOULD say that Brian's tool, which allows migrate faces, states and submodels from one model to another, is 100% dependant on the nodes being in the same grid pattern, and if you add nodes, then it breaks. The fix is to remove them and then re-add them back in after performing the migration, but if we had designated null nodes, we could then ignore them as if they aren't there, and the tool would now work in this rewired scenario!

[derwin12]

Dare I say .. too risky at this time..

[deplanche]

thought the same thing Jon, but what do you do for folks with seed trees and want to add a bunch along the bottom to skip around to the next strand?

What I am thinking is that for each string (port) you would be able to indicate where to place the null pixels and the number of null pixels to add. So if they had a tree with 4 strands, and wanted to put 5 nulls between pixels 100 and 101 they could. Or even add 1 null between 50 and 51 AND 1 more between 150 and 151. It's much more input, but doesn't break the existing model.

Of course, I have no idea how to code something like that in, but I think it could be done and work. Maybe.

[heffneil]

I think for the short term I was suggesting that instead of supporting designated null pixels, we continue to play with the missing ones (jumping in number from 9 to 11 - skipping 10) but the other tools (faces, submodels, and states) would adhere to the same pixel numbering as the model data. The channels seems to properly count by the rules if you skip a number in model data, and in a future iteration, we would figure out a way to designate null pixels in the middle or toggle a node as null or not in the model data node editor?