I think something is going wrong here:

The unwrap operation may fail to solve any islands (for example, if not enough seams are present), which is what happened in the above scenario. The operator displays an error message when this occurs, which is something we ought to do in the node as well. Generally, nodes use GeoNodeExecParams::error_message_add to log errors, but it is not immediately apparent to me how to do this from a field input node.

The latest version of the patch looks quite good. I do think it's a bit late for 3.2, but I could see adding this to 3.3 early in the cycle, depending on user feedback.

@Aleksi Juvani (aleksijuvani), could you update this patch to latest master? It doesn't apply currently. Then I'll make a build of it so it can be tested.

Build is ongoing here: https://builder.blender.org/admin/#/builders/18/builds/392

Here is a test file I did with it, hope it's help.

A new test build of this patch is available here: https://builder.blender.org/download/patch/D14389/

The code basically looks good to me, I think we should try to commit this to 3.3. I'll hold off accepting it though, because it would probably be good to discuss it in a module meeting briefly, just to make sure there's a consensus on the new menu, etc.

A concern that I have is that this only outputs square UV maps. If the user wants a non-square UV map, I was thinking that they could multiply the input positions by the aspect ratio and divide the resulting coordinates by the aspect ratio, but that doesn't seem all that intuitive, and I'm not sure if that's even correct. A simple solution would be to expose the parametrizer "aspect ratio" parameter as a float, which would let you specify the shape of the UV map.

I'm also concerned that the Pack UV Islands node might not be useful enough justify its inclusion. It can be used to "append" existing UV maps without destroying existing information, which might be useful, but I don't imagine that to be a very common thing to do. On the other hand it's not all that much code, and there does exist an operator with the same function, so we would be translating that same functionality into a node.

In D14389#405003, @Aleksi Juvani (aleksijuvani) wrote:

I'm also concerned that the Pack UV Islands node might not be useful enough justify its inclusion. It can be used to "append" existing UV maps without destroying existing information, which might be useful, but I don't imagine that to be a very common thing to do. On the other hand it's not all that much code, and there does exist an operator with the same function, so we would be translating that same functionality into a node.

I think both nodes are super useful! I think packing becomes more and more valuable the larger you system becomes. Some parts might want unwrapping in their one specific node trees but when added together, will need packing. I think it's worth including both!

Because I like to bring up consistency with other nodes, are there any guidelines about the order of the inputs?

Existing nodes tend to place Selection inputs above Value inputs, and Fields above Constants of the same type (purple then gray, diamond then circle). But these new nodes are the complete reverse.

In the module meeting we discussed these nodes briefly.
There was consensus that they should have "UV" in the name to make that more clear (even though there is a category).
Besides that, people who tested it thought they worked well. Even then, we thought some more specific testing and code review should be done to clarify what happens in some edge cases.
An example that was brought up: what happens when only part of a UV island is selected. Maybe the behavior is obvious, but since we're now baking these decisions into files, we need to make sure nothing sneaks by.

Existing nodes tend to place Selection inputs above Value inputs, and Fields above Constants of the same type (purple then gray, diamond then circle). But these new nodes are the complete reverse.

The convention is to put selection sockets right below the corresponding geometry, since these nodes are field nodes they have no geometry input, so it's a bit different. Maybe it still makes sense to have the selection on the top though.

In D14389#406972, @Hans Goudey (HooglyBoogly) wrote:

An example that was brought up: what happens when only part of a UV island is selected. Maybe the behavior is obvious, but since we're now baking these decisions into files, we need to make sure nothing sneaks by.

The nodes have the same behavior as the operators. As far as the code is concerned, faces outside the selection do not exist. For instance, if the Pack Islands node is invoked with a part of an island selected, the connected faces in that selection become their own islands. Faces outside the selection are left untouched. This is probably reasonable.

Current Situation	Fields on Top

With regards to the ordering of the inputs, let me know if having the fields on the top is preferable, or if you had something else in mind.

What about this order for Pack UV Islands?

• Selection
• UV
• Margin
• Rotate

Having the selection on top consistently might be nice.
The patch could use a merge to master again.

how do we get the 'reset uv' behavior out of this?

In D14389#416741, @Jacob Merrill (blueprintrandom) wrote:

how do we get the 'reset uv' behavior out of this?

If you look at the implementation of this operator in the source code, it looks to me like the function ultimately responsible for that behaviour is mesh_uv_reset_array. For triangles and quads, vertices get mapped to the corners of the texture space. For faces with more than four vertices, they seem to be mapped on to a circle. You may be able to replicate this behaviour with geometry nodes, but it's not immediately clear to me how to do this. I think you would have to be able to figure out how a face corner index maps to the indices of a face (i.e. given a face corner index, is it the 1st vertex on the face, the 2nd vertex on the face, etc.), from where you could go about replicating what this function does.

Will 'Smart UV Unwrap' be integrated into the UV Unwrap node at some point? It's difficult to procedurally generate correct seams for procedural geometry; it would be great to have Smart UV Unwrap for trickier UV unwrapping situations.