Hi! Relatively new Blender user but long time 3D graphics programmer.
I was experiencing some odd behaviour exploring how mesh face-based instancing works and in trying to figure out what I was seeing I found someone else had reported it (https://developer.blender.org/T77575).
Since I’d previously downloaded the Blender source I was curious to see if I could find the code responsibile for this and, lo and behold, the mirror modifier does work exactly as I suspected, as per this comment in BKE_mesh_mirror_apply_mirror_on_axis():
/* reverse the loop, but we keep the first vertex in the face the same,
* to ensure that quads are split the same way as on the other side */
Since modifying how instancing works seems way too hairy, I was thinking of adding some extra options to the Mirror operator to control 1. how it treats normals and 2. its output vertex order.
This seems like a pretty self-contained first contribution but I thought I’d ask here before I start: am I going down a dangerous path? Should I reconsider? And am I asking this question in the right place?
Edit to add: a small rewrite later and I have more predictable behaviour under mirroring operations. I slightly misunderstood how the orientation is calculated: the Y axis appears to be always perpendicular to the normal and the first edge in the edge loop.
Before (note that X and Y axis point in seemingly random directions)
I think the only risks for you are a) you might find out that the way it’s being done is best for one reason or another and that changing the behavior causes more problems than it solves, or b) that you are able to fix it but maybe it isn’t accepted in master.
From the looks of it, a) isn’t a problem and I don’t think b) is much to worry about because it seems like a simple and benign modification.
Need help testing? I have a couple Linux computers I can test this on, when you publish the patch.
Changing the vertex order does affect how quads are split so I can only assume it will affect n-gons in a similar way. I’ve implemented it as an additional flag on the mirror modifier so you can keep the current behaviour.
Thanks! Once I’ve made a patch I’ll share it here & on the bug ticket.
Ah, thanks for the tip. I just manually created a patch through the “create a patch” webpage. Seems like I should familiarize myself with arc, unless there’s a way to fix it manually in the meantime?
Edit: I think I used arc right after a couple of flubs. Lmk if it looks good now. I see it made a build and appears to have run some tests?
Following up on this: this is really only necessary because the way face instancing works is inscrutable as an end user. Even the documentation says:
If the geometry of the children is not symmetrical then the orientation of the face (as determined by the order of its vertices) could matter. As of 2.70 Blender does not have tools which allow you to adjust the ordering of the vertices on a face.
However Blender does have a consistent concept of face orientation: the UV channels used for texturing are literally a parameterization of space.
Controlling the orientation of instances based on the UV channel data on polygons would give much more predictable–and customizable–behaviour under mirroring or any other operations. This would require adding to the parameters for face instancing and I’m not sure if that’s a more or less desirable change.
(As a sidenote: it’s not obvious as a new contributor where the best place is to ask questions like this! This forum, chat, on the bug tickets…?)
Hi Josephburg, I have an objection.
Even if the normal direction of the plane overturned by the Mirror Modifier is changed, there is certainly no apparent problem outwardly.
But when the plane plays the role of the Face Instancer, the problem is revealed.
I was modeling the audio mixing console when I found this problem. Look at the slider knob that has returned to 90 degrees on the other side.
Wow, I never saw the follow-up responses. Thanks for the ping, looking forward to checking out that patch.
Anyhow, I was agreeing with OP that it seems necessary and a benign change. I agree with you, too. It’s something that we should address, for sure.
Hey! I’m just catching up on this after getting incredibly busy the last few months.
As I commented on the bug report (and above), I’ve got mixed feelings about the patch I submitted. I like that it’s self-contained but it definitely feels like a fudge on top of something that already works in a weird way. The fact the documentation has to basically make excuses for the feature doesn’t really sit nicely with me.
I feel like I could make a similarly self-contained patch that implements a UV-based approach:
Use the tangent space of the UV map to define the local x/y space
z is calculated to be perpendicular (same as current behaviour)
Mirroring geometry will produce predicable results
