There is no good reason imo to have object parenting work differently for curves when there are such things as constraints to achieve the same thing. There is a series of other hidden functionalities baked into the curve object.

Some feedback on the current state:

• For me personally, the way that the brush direction is considered and emphasized doesn't feel right for hair sculpting. For meshes it makes sense, as this is useful to create edges in the surface. For the clumping of hair it makes more sense to be omni-directional imo
• I think the we need a way to scale the brush strength from root to tip along the curves. I'm not sure how that fits into the current design, whether that is a selection thing or a setting of the brush
• I'm not sure about the Clump Radius. I feel like the problems it is trying to resolve should be possible to be resolved better with a different approach when we iterate over the brush
• In my opinion the pinching creates more natural results as points that are further away regularly travel a longer distance per brush sample. Right now they all move at the same speed (assuming a constant falloff). When I adjust the influence to be approximately multiplied by the distance from the cursor the result is more natural imo

Thanks for the updates, looks good now from my end!

That was quick, thanks for the patch!

Ah, sorry, I didn't see

Ah, sorry. I searched if there was already a report but didn't find this one.

Yea, that seems to work, thanks! I always found it very unpredictable what an operator actually needs to have overridden, but I guess I should just override everything that could be potentially needed.
But yea, it probably still shouldn't crash. The issue seems to also just be with the lattice modifier from what I have seen.

I'm talking about the default of inputs of the node group for when it's used in another node tree.
I phrased it badly, what I meant was that there the default value should be used regardless. So the default attribute behavior would only affect the modifier version of that node group.
maybe there has to be a bit of discussion on this topic, but I think this makes the most sense.

Small addition: In the case of modifier inputs that would also require to set the default input to be an attribute in the first place.
For outputs that is given, for nodes the input should be passed via node link.

I'm able to reproduce on Linux, this looks like the same issue that I was having.

Beautiful, this is how it should be!

Thanks for looking at this functionality Jacques!

@Henrik Dick (weasel) I tested the patch. It makes an improvement, but it does not seem to fully fix the issue. In the initial example I shared there is still a (smaller)artefact visible that wasn't there before.
I also tested another mesh where it appears to be a regression.
I will update the example file in the task description to include both meshes.

Oh sorry, I missed making the mesh local. I replaced the file

@Hans Goudey (HooglyBoogly) I'm getting a bit lost in what you are proposing. It seems to me that while we are disagreeing on the reasoning an a fundamental level, we are combing to the same conclusions on what this node should look like. That makes it a bit confusing to me.

Hm. This might be personal bias due to my physics background, but to me combining a vector socket from 3 components is fundamentally the same operation regardless of the used coordinate system.
Sure, in the code the cartesian system is used for everything, but I personally would prefer if a more agnostic approach.

Thanks!

I'm now having the strange issue with the provided example video file that when I add it to the sequencer, now the preview without proxy is not matching until I made a first render or changed the timecode.

@Ludvik Koutny (rawalanche) How is the solution you are mentioning different from the 4th one I added in the description? In that you hold the modifier key before you confirm the search choice? That seems to make it a bit opaque to me what the modifier key is actually doing.

Some of my initial thoughts to keep track in this task:

Thanks for the clarification. Then I guess I misunderstood that initially. For me the whole problem are the proxies not matching the original.

Not sure if I'm misunderstanding something. The proxies are indeed causing the issue for me. Without them everything is fine.
Without proxies I do get the warning (not reliably though). Selecting a timecode index method does help indeed.

I don't seem to be getting any warning with the example file that I posted. Maybe I'm expecting something else than is happening? Could you post a screenshot of what it should look like?

I'm not sure I'm 100% up to date with where to cause and fix the issue after the fix that you're proposing.

Having this work by default for new strips sounds acceptable to me. Still, there should be a warning displayed, in my opinion, in cases that we know cause issues.

Okay, that sounds fine to me, thanks!

@Richard Antalik (ISS) I wasn't able to reproduce the issue that you are mentioning with the exact steps you provided. For me the non-proxy preview seems to be consistent with the render here.
But, indeed, the fundamental issue here is not necessarily that the proxies are not matching, but in turn that because of this the render doesn't match the preview. So if there is another source for this issue then that is also part of the problem.
To clarify, though. If your example is a point about frame-perfect matching, then that's also valid, but not nearly as drastic as the issue that I'm describing with the proxies, as here differences can stack up throughout the video for an arbitrary time offset, much greater than a single frame. And fixing this after the fact is nearly impossible, so an edit has to be manually redone in most cases.

This looks good to me! Glad we could come to a conclusion that makes everyone happy :)

The node looks generally good to me! I have a couple of remarks and questions:

Just adding myself as a reviewer to track this. This doesn't regard the code review.

Thanks, I could build it again.

I couldn't apply the patch on yesterday's master for some reason. But here some smaller notes judging from screenshots and code:

In D13739#366648, @Ludvik Koutny (rawalanche) wrote:

Or am I wrong, and will this change also reorder the items when there is something typed in the search field?

In D12035#366474, @Charlie Jolly (charlie) wrote:

• Remove Range and End... this node then operates solely in the [0-1] range with just a Value input. This reduces the ambiguity and simplifies the node.

This is a big node, so I have a couple of remarks:

I generally agree on the point about predictability, but to me this doesn't apply here.

For me this patch is good to go now :)

I noted my thoughts on this originally in the design task:

Thank you, I think it would be nice to have mote boolean logic in the node!

Thank you for the patch Charlie!

Thanks for the changes!

For usability I would suggest the following definitions:

• Node layout:
  • Like Separate/Combine XYZ
• Naming and Scope:
  • Components: Radius/Zenith/Azimuth -> R/Theta/Phi
  • Maybe this could also be solved with more generic Combine/Separate Vector nodes that can have different 3D coordinate modes. E.g. Cartesian (X/Y/Z) (->default), Spherical (R/Phi/Theta), Cylindrical (Rho,Phi,Z)
• Units: Radians, shown as deg for the UI (same as e.g. Vector Rotate)
• Value Ranges: Phi [-pi, pi], Theta [-pi/2, pi/2]

Cool, thank you!

We chatted about this patch in today's meeting and agreed that the rotation input can be removed.

I just noticed that the Rotation input was a request by @Hans Goudey (HooglyBoogly) :D
I personally don't think that this node needs this functionality itself, as I think usually you would want to use this node for relative rather than absolute transformations and this can be done with a single node afterwards.
But this is not a very crucial point for me.

Sorry, missed this patch for a while!

In D13709#364267, @Hans Goudey (HooglyBoogly) wrote:

Okay, here is my current hypothesis: Duplicate the extruded faces if all of the extruded edges making up the boundary region do not connect to any other faces.

I just noticed the inconsistency with the mesh editing behaviour when the selection of the extrusion is equal to a full mesh island (?). I'm actually not quite sure what the rules for this are, it does seem a bit arbitrary.
I do think though, that ideally the node behaviour would be consistent with what the operator does.

I do personally like this behaviour with implicit normal offset a lot better!

In T92588#1283773, @Ludvik Koutny (rawalanche) wrote:

Otherwise, if terminology consistency is a priority here, it would mean all these useful nodes would have to work in the same wonky way that the old MixRGB node does.

In T94598#1283699, @Ludvik Koutny (rawalanche) wrote:

I am afraid this would mean significant overcomplication of the node editor keymap and possible keyboard shortcut conflicts. When you are in a link drag, you are in sort of a mode, so it's fine that the shortcuts work as the awareness of being in that mode is explicit, but when you are just in a regular node editor state, then suddenly you would have to be constantly aware whether you currently have a node socket selected or not, and the keymap would behave differently based on that.

I might be misunderstanding something here.

I honestly don't think that clamping should be the default for a generic mix node.
I'm actually not even sure it should be an option in the node at all.
For MixRGB specifically it makes sense for the workflow, to avoid colors outside of 0/1. For generic mix operations of value/vector maps though I don't really see that much inherent value in this input.