Page MenuHome

Applying Subdivision/Multires Modifier De-merges UV's
Confirmed, LowPublicKNOWN ISSUE

Description

Update

The issue of custom-data (UV's specifically) becoming detached remains, this problem existed before rBb88dd3b8e7b9, it was just less obvious since selection connected near-by UV's. Now UV's are merged by default at a very low threshold when applying modifiers to mitigate the problem. Keeping this report open as it would be good if subdivision-surface didn't separate UV's, replies to this report contain some notes that could lead to a solution. In practice users should not experience problems caused by this behavior, so marking low priority. ~ @Campbell Barton (campbellbarton).


System Information
Operating system: Windows-10-10.0.18362-SP0 64 Bits
Graphics card: Radeon (TM) RX 480 Graphics ATI Technologies Inc. 4.5.14736 Core Profile Context 20.8.3 27.20.12027.1001

Blender Version
Broken: version: 2.91.0 Alpha, branch: master, commit date: 2020-09-22 12:02, hash: rB8d1123ba220b
Worked: working in 2.90

Caused by rBb88dd3b8e7b9: UV: remove selection threshold for nearby coordinates

Short description of error
When applying a subdivision modifier, certain UV's points disconnect from their neighbors. This does not seem to occur with the F3 menu subdivide operation.

Exact steps for others to reproduce the error

  • Open default blender scene.
  • Subdivide cube - 1 level and apply it. Do this step 4x
  • In the UV editor use "Alt select" to select face loops at random.
  • Alt click around until a face loop is not fully completed, this will reveal unmerged vertices.
  • Check area that did not complete by moving faces, vertices, edges etc to confirm unmerged UV's.

This does not seem to be an issue with the newly added rB9a52b30cc0de in my eyes, rather an issue with subdividing.

Event Timeline

One way to find out if a face is disconnected is to move it:

But I'm not sure if this is really a bug.
I'm not sure what the UV editor considers "connected".
But we cannot guarantee 100% accuracy in all operations.

(CC @Campbell Barton (campbellbarton))

Perhaps it's not a bug, I'm not sure, however I do know that it is certainly undesirable behavior. I think that if UV's were merged before any operation, then they should stay that way, unless specifically told to do otherwise, like in a cut operation etc. Perhaps it's not just related to this issue, but it's an underlying quirk with the UV workflow currently? Thanks for your time.

Richard Antalik (ISS) changed the task status from Needs Triage to Confirmed.Sep 29 2020, 8:56 PM

I can confirm this working in 2.90, broken somewhere between rB396d39c6b904 and rB0721fbb6e1e2

This is caused by rBb88dd3b8e7b. @Campbell Barton (campbellbarton) so I would say there is no bug here? subsurf probably generates small differences in UV coordinates due to numerical instability, this used to be ignored by UV selection code, now user should ensure those are perfectly matching using the merge tool first?

Campbell Barton (campbellbarton) changed the subtype of this task from "Report" to "Known Issue".

poke. Bug still here and it is important!

if bug is «unsolvable», why don`t we have just workaround (merge by distance)? at least it will eliminate issue on the user`s side.

I think the whole behavior of breaking uvs by moving polys is just not right and this is creating huge issues when exporting to meshes to other software. Also causing render engine errors. And there is no way to actually visualize if you accidentally have "De-merged UVs".
Breaking of UVs should only be possible on user's explicit demand. Right now, if you just start moving verts or poligons in the UV editor with the wrong "Sticky selection mode" you can make a huge mess without noticing. I am not sure what the intention of these "modes" is, but it all looks totally broken and buggy to be honest.

Philipp Oeser (lichtwerk) added a project: Restricted Project.Jul 20 2021, 9:59 AM

This is caused by rBb88dd3b8e7b. @Campbell Barton (campbellbarton) so I would say there is no bug here?

rBb88dd3b8e7b doesn't mention any benefit to users, nor does it reference any review / design document / official decision to change the behaviour. More importantly, this change was also not mentioned in the 2.91 release notes, so I would say it's fair to consider it a bug.

Philipp Oeser (lichtwerk) renamed this task from Applying Subdivision Modifier De-merges UV's to Applying Subdivision/Multires Modifier De-merges UV's.Mar 30 2022, 12:39 PM

This is caused by rBb88dd3b8e7b. @Campbell Barton (campbellbarton) so I would say there is no bug here?

rBb88dd3b8e7b doesn't mention any benefit to users, nor does it reference any review / design document / official decision to change the behavior. More importantly, this change was also not mentioned in the 2.91 release notes, so I would say it's fair to consider it a bug.

Agree this is a bug. At the time of committing this I wasn't aware of any user-visible changes or that the subdivision surface modifier was splitting up loop custom-data (including UV's).

Using an epsilon when comparing UV's for selection only hid the bug that was already there, this should be resolved in the subdivision surface modifier since UV selection isn't the only area that checks if UV's are in the same location (calculating tangents for example will be impacted by this).

Reverting rBb88dd3b8e7b would only be a partial fix, even for selection as there are multiple selection operations that walk around co-located UV's (UV rip, loop select, path selection - for e.g.).
Worse, the behavior of these operations wouldn't be deterministic - as matching all other UV locations could succeed or fail based on the UV of the initial loop which the others are compared too.

A stop-gap solution to this could be to merge UV's after applying the subdivision surface modifier (when the apply modifier operator is used to avoid adding overhead to updates for the viewport), although it would be better to investigate the problem in the subdivision-surface logic as this impacts all custom-data, not just UV's.

It's possible the subdiv surface modifier is deterministic for XYZ, but not UV. If this is the case, making the UV deterministic also should fix the problem.

I've managed to repro locally and will try investigate further...

As it's tedious to reproduce this manually each time, here is a script to redo the steps:

1# Run from the command line:
2#
3# blender -b --factory-startup --python reproduce_T81065.py
4
5import bpy
6from bpy import context
7bpy.ops.mesh.primitive_cube_add(enter_editmode=False)
8for _ in range(4):
9 bpy.ops.object.modifier_add(type='SUBSURF')
10 context.object.modifiers[-1].uv_smooth = 'SMOOTH_ALL'
11 bpy.ops.object.convert(target='MESH')
12
13set_of_uvs = set(tuple(item.uv) for item in context.object.data.uv_layers.active.data)
14target = 1649
15if len(set_of_uvs) != target:
16 print("Found", len(set_of_uvs), "UV's, expected, failure!", target)
17else:
18 print("Found exactly", target, "UV's, success!")

@Campbell Barton (campbellbarton), it is still unclear to me what the user benefit in rBb88dd3b8e7b9 is.

I am not sure making bit-perfect match of UV coordinates calculated from 2 different ptex faces is something doable in practice. It is hard to guarantee order of math operations along ptex face is the same for every side of the a seam, especially taking an external nature of the library.

Surely, having that figured out would be nice, but I do not think it is cool at all to dump extra work on others without any communication upfront, without any agreed-on design. The denial to clearly user benefits of the change which would overweight measurable regression does not help motivation either. While I can see that previous situation was not ideal, reverting the commit will bring Blender to a state which was known to work (with its limitations) for a long time, and will give time to re-asses the situation by all involved modules without stretching tight schedule. So to me a revert would still be better than dragging a real regression for such a long time.

I am not sure making bit-perfect match of UV coordinates calculated from 2 different ptex faces is something doable in practice.

The XYZ coordinates are being calculated bit-perfect, so we know it is possible. The question is, why are the UVs not *also* bit-perfect?

Sure, we know how to merge-nearby and sweep the problem under the rug, but I think it's worth identifying the root-cause while we have a live repro before coming up with solutions.

The XYZ coordinates are being calculated bit-perfect, so we know it is possible. The question is, why are the UVs not *also* bit-perfect?

I am not really sure how we know this.

For the Mesh codepath subdivided vertex positions along the edge are calculated from a single ptex face. Now when I think about it, it is not even strictly determined which ptex will be used for this due to a threaded evaluation nature. The UVs are evaluated for every point (including the ptex face boundary), since there night be discontinuation involved due to seams.

For the CCG codepath the coordinates are stitched together along the ptex face boundaries.

A possible solution would be to implement similar to coordinates heuristic and handle UV data in a special way: which is evaluate it only using single ptex when the edge between faces is not marked as seam. The downsides of this approach:

  • While it will work in simple cases, it is not guaranteed to work reliably in all cases as the seam information might get lost throughout modifier (or geometry nodes) evaluation
  • Is limited to a single modifier only, and all other modifiers will need to implement similar mechanism. The fact that the current report is only about subsurf modifier doesn't mean that, say, boolean or geometry nodes modifier are free from the same regression in behavior.

@Sergey Sharybin (sergey) there is no real user benefit to rBb88dd3b8e7b9 on it's own, however - making all selection tools properly respect this is bigger project (loop select, path select, UV rip, edge selection - which checks connected UV's), which would likely incur the overhead of a merge-by-distance for each user action.

At the time it didn't seem like users were depending on this functionality & even now, it's quite a spesific situation that shows up as a problem.

@Campbell Barton (campbellbarton), it is still unclear to me what the user benefit in rBb88dd3b8e7b9 is.

I am not sure making bit-perfect match of UV coordinates calculated from 2 different ptex faces is something doable in practice. It is hard to guarantee order of math operations along ptex face is the same for every side of the a seam, especially taking an external nature of the library.

Even if the math isn't accurate between adjacent uv-loops these calculations could be shared as the same loop-interpolation could be used for adjacent loops.

Surely, having that figured out would be nice, but I do not think it is cool at all to dump extra work on others without any communication upfront, without any agreed-on design.

This wasn't my intention as this could be solved at different levels (some not involving much in the way of opensubdiv development).

The denial to clearly user benefits of the change which would overweight measurable regression does not help motivation either.
While I can see that previous situation was not ideal, reverting the commit will bring Blender to a state which was known to work (with its limitations) for a long time, and will give time to re-asses the situation by all involved modules without stretching tight schedule. So to me a revert would still be better than dragging a real regression for such a long time.

My resistance to revert rBb88dd3b8e7b9 is that someone can immediately report the same issue with other selection tools, which don't use BM_uv_vert_map_create.

Basically to properly support this kind of selection, it means selection methods that use connectivity checks need to do a "merge by distance" (or behave as if that's been applied).
At that point I think we could consider doing that as part of the "Apply Modifier" action as it's not efficient to do this on every user action and as mentioned already, disconnected UV's can cause issues elsewhere.


Possible Solutions

  • Accept a partially working solution by reverting rBb88dd3b8e7b9, create a new report with remaining issues where disconnected UV's don't work properly.
  • Perform an additional merge-by-distance when applying modifiers (this could be done as a post-process). It's not ideal but it at least avoids disconnected UV's everywhere.
  • Investigate solving this for the subdivision-surface calculations (either using a method that doesn't diverge or re-use existing calculated UV's).

At the risk of missing the point, is it easy for someone to compile opensubdiv with -fno-fast-math -fno-fma -debug etc and report back if the bugged script^^ still repros?

I still feel like we need to identify the root-cause.

Just a quick update with my latest findings, the rounding error appears to be introduced somewhere inside this Blender function call:

BKE_subdiv_eval_face_varying(subdiv, layer_index, ptex_face_index, u, v, subdiv_loopuv->uv);

That code eventually resolves out to an opensubdiv evaluator.

On the opensubdiv side, there appears to be a whole family of different evaluation engines. FWIW, the CPU version of the code appears something like this :

void AddWithWeight(T const *src, float w) {
    if (_p) {
        for (int i = 0; i < _length; ++i) {
            _p[i] += src[i] * w;
        }
    }
}

These kinds of loops are sensitive to round-off error.

My next step is to get a copy of the actual contents of the buffers at the time we get different results. From there we can start looking at possible fixes and their performance trade-offs.

After examining the contents of the buffers, I've managed to extract a tighter repro:


Left is original mesh, right is after one subdiv and a little bit of adjusting UVs to make the demerge more obvious.

The bug doesn't seem to be particularly sensitive to initial UVs. However the output from opensubdiv appears to be rounding the last binary digit in a way I don't yet understand.

My current working theory is that difference seems to be related to the numbering of the vertices in the face. e.g. If four vertices are numbered 12, 24, 36, 48, then the same quad can be represented as [12, 24, 36, 48] or [24, 36, 48, 12], etc.

...Investigation continues...

OK, here's two simple .OBJ files. The vertices and UVs are in identical locations, the quads are geometrically the same, but have different representations:

# This object subdivides correctly
o SubDivRoundingSuccess
v -1 -1 0
v 0 -1 0
v 0 0 0
v -1 0 0
v 1 0 0
v 1 1 0
v 0 1 0
v 1 -1 0
v -1 1 0
vt 0.148790 0.764651
vt 0.283966 0.773115
vt 0.278536 0.826571
vt 0.145159 0.814600
vt 0.417592 0.843259
vt 0.414872 0.903889
vt 0.276509 0.881888
vt 0.424313 0.781021
vt 0.142887 0.867894
s 1
f 1/1 2/2 3/3 4/4
f 3/3 5/5 6/6 7/7
f 2/2 8/8 5/5 3/3
f 4/4 3/3 7/7 9/9
# This object demerges UVs when a subdiv modifier is applied
o SubDivRoundingProblem
v -1 -1 0
v 0 -1 0
v 0 0 0
v -1 0 0
v 1 0 0
v 1 1 0
v 0 1 0
v 1 -1 0
v -1 1 0
vt 0.148790 0.764651
vt 0.283966 0.773115
vt 0.278536 0.826571
vt 0.145159 0.814600
vt 0.417592 0.843259
vt 0.414872 0.903889
vt 0.276509 0.881888
vt 0.424313 0.781021
vt 0.142887 0.867894
s 1
f 1/1 2/2 3/3 4/4
f 5/5 6/6 7/7 3/3
f 2/2 8/8 5/5 3/3
f 3/3 7/7 9/9 4/4

To repro the bug, import the second .OBJ, add a subdiv modifier (default settings), apply the modifier, then edit the UVs on the new mesh.

Note that by the "Hairy Ball Theorem", in the general case, it's not possible to find a rotation of the vertex indices inside the quads which will fix this problem.

At this point, I'm also not confident we can fix the problem upstream in OpenSubDiv without a substantial performance hit for large meshes.

I feel like the best solution is to merge UVs that are within one ULP (or 2 ULPs?) when we copy back the output from OpenSubDiv, and just eat the threading complexity and performance hit at that stage.

I've also considered just truncating the output (or input) from OpenSubDiv. While that's certainly fast, I think it only decreases the probability of a demerge, rather than eliminating it completely.

Any thoughts before I attempt to write the merge code?

p.s. Note that this fix will not be stable across machines.. e.g. Two different machines can load the same .blend file, and produce different results at the ULP level.... Is this a problem?

Accept a partially working solution by reverting rBb88dd3b8e7b9, create a new report with remaining issues where disconnected UV's don't work properly.

Keeping in mind the fact that a new report is needed for the issues brought back by the revert (as opposite of linking existing one), and that the commit caused quite serious disruption in artists workflow at the studio, this proposal seems to be the best corse or actions.

Perform an additional merge-by-distance when applying modifiers (this could be done as a post-process). It's not ideal but it at least avoids disconnected UV's everywhere.

It could be a viable solution, but an investigation is needed w.r.t the merge distance.
Although, I like the sound of the idea that it will cover all possible modifiers. Also like Chris's idea of basing it on ULP.

Investigate solving this for the subdivision-surface calculations (either using a method that doesn't diverge or re-use existing calculated UV's).

I'm still not sure why subsurf is the only victim here. There are many other modifiers which potentially has the same issue.

Any thoughts before I attempt to write the merge code?

We already have "Merge By Distance" operator, so I guess the majority of the code is already in place, and "just" need to find good distance.

P.S. AFAIR, it is now possible to use double floating precision in OpenSubdiv. Maybe it will help mitigating some of the precision issues (with memory usage hit).

I'm still not sure why subsurf is the only victim here. There are many other modifiers which potentially has the same issue.

It happens also with bevel modifier, infact I use a script to merge back the UVs.

It happens also with bevel modifier, infact I use a script to merge back the UVs.

Bevel may create new and unpredicted geometry. Subdiv only divides it with a strict rules.
Yes, it will be great to have proper UVs or more solid material division after Bevel.

Update: I have a "proof-of-concept" fix that "works on my machine" by just merging UVs as we copy them out of OpenSubdiv. This seems to pass all the tests above.

It turns out even ±3 ULP was still demerging, so I had to use a full tolerance.

I think instead of just blindly merging UVs, we should first check to see if the XYZs match, as the UVs might simply be nearby because we've joined multiple original meshes into one mesh with multiple linked components.

...plus threading...

I haven't had a chance to look at the bevel modifier, but perhaps we can use a similar approach there if we can get a fix working here.

Accept a partially working solution by reverting rBb88dd3b8e7b9, create a new report with remaining issues where disconnected UV's don't work properly.

Keeping in mind the fact that a new report is needed for the issues brought back by the revert (as opposite of linking existing one), and that the commit caused quite serious disruption in artists workflow at the studio, this proposal seems to be the best corse or actions.

This would have been good to mention early on as it's the first I've heard of this causing more serious problems. I'd assumed this was a fairly rare case which users could working using weld by distance. In that case adding back a threshold seems OK even if it's not leaving Blender in an ideal state in other respects. I'll look into this.

At the risk of missing the point, is it easy for someone to compile opensubdiv with -fno-fast-math -fno-fma -debug etc and report back if the bugged script^^ still repros?

More puzzling evidence, I wanted to try and confirm that OpenSubdiv has the same output as Blender when operating on those .OBJs I posted earlier.

I built OpenSubdiv from source, passed in both SubDivRoundingSuccess.obj and SubDivRoundingProblem.obj and... to my surprise ... both outputs from OpenSubdiv had merged UVs correctly. (!)

After a little bit more digging, OpenSubdiv does indeed generate 25 unique UVs for both test cases, however those two test cases *disagree* on the U-coordinate for one of those UVs by exactly one ULP.

Keep in mind, the only difference between the test cases is the *numbering* of the vertices within the quads. Everything else is the same. Very puzzling outputs indeed.

EDIT: "25 unique UVs" was "9 unique UVs"

The XYZ coordinates are being calculated bit-perfect, so we know it is possible. The question is, why are the UVs not *also* bit-perfect?

I am not really sure how we know this.

This comment got me thinking.. I just rechecked the XYZ positions using OpenSubdiv, and have managed to construct 2 .OBJ files which are identical up to a relabeling, and now the same thing is happening with XYZ as with UV.

i.e. I have two .OBJ files (identical up to relabeling) which OpenSubdiv both subdivide to 25 unique XYZ and 25 unique UV, but they disagree on one particular XYZ and one particular UV at the significance of 1 ULP.

i.e. The XYZ are *not* being calculated bit perfect by OpenSubdiv.

Next step, can we also create a de-merging of XYZ in Blender???

A tale of two OBJ files, identical up to a relabeling of the vertices within faces.

The two files, (each with 9 unique XYZ and 9 unique UV) are each subdivided in either OpenSubdiv or Blender, then the output is compared.

The preferred outcome is each output should contain 25 unique XYZ and 25 unique UV, and they should both be identical up to a relabelling.

...however...

In OpenSubdiv, each file produces 25 unique XYZ and 25 unique UV, but the two outputs disagree in the ULP.

In Blender, one of the files produces 25 unique XYZ, and 25 unique UV.
The other file produces 25 unique XYZ, and 28 (!) unique UVs.
In Blender (as in OpenSubdiv), the two outputs disagree in both XYZ and UV in the ULP.

This is strong evidence that there is another method of copying the UV information out of OpenSubdiv in a way that keeps UVs merged in the expected way.

... the investigation shifts back to Blender.

Turns out we're not the only ones having this problem. OpenSubdiv provides an alternative interface for face varying data called a Far::PrimvarRefiner for exactly this situation.

https://graphics.pixar.com/opensubdiv/docs/doxy_html/a00081.html

It looks like we already use this interface within the Cycles renderer.

I've created a Frankenstein version of blender to use this interface and it does indeed fix the problem for my limited test case.

As an added bonus, this interface appears to be more efficient than our current evaluation strategy for face varying data. We could potentially be looking at a win-win fix here for both performance and functionality.

Next step is to work up the change to something more realistic and see if it still holds on the other test cases.

p.s. I haven't had a chance to look at the issue with the Bevel modifier. If anyone is keen to isolate a repro there, it would be useful to take a peek while I have everything open in the debugger.

Okay, I think I've taken this about as far as I can.

I think the correct solution is to use Far::PrimvarRefiner to generate the UVs so that they are correctly shared across the loops.

As an added bonus, this method should be faster than the current method of evaluating each vertex of the loop using the relevant patch.

I have a version working locally that fixes the problem for simple test cases, however it:

  • Doesn't work on medium/large meshes. (I think this is because the refiner is set to "adaptive")
  • Breaks the encapsulation in this area of the code pretty heavily.

Note that this is a subdiv-only solution, and doesn't address any other demerge situations.

I'm happy to do a handover of the testcases and code I've developed so far, but I feel I've crossed my point of diminishing returns on this task at this time.

I'm just throwing in my two cents here.

It sounds like a reasonable solution to this issue would be to do the following when one or several modifiers are applied to an object:

Merge uv's that "belong" to the same vertex and are within a small (arbitrary) distance from each other in uv space.

Campbell Barton (campbellbarton) moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.May 3 2022, 4:53 AM
Campbell Barton (campbellbarton) added a project: Restricted Project.

I'm just throwing in my two cents here.

It sounds like a reasonable solution to this issue would be to do the following when one or several modifiers are applied to an object:

Merge uv's that "belong" to the same vertex and are within a small (arbitrary) distance from each other in uv space.

Discussed with Sergey and we agree it's a reasonable option, uploaded a patch D14841.

Note that this it would still be good to resolve the issue with opensubdiv, but it's not as pressing if merging is performed.

Note that this it would still be good to resolve the issue with opensubdiv, but it's not as pressing if merging is performed.

Okay, it is partial solution. But.
Imagine overlapping islands. Their cordinates are identical.
So I suppose they will merge together if you will merge by distance without any restrictions.

Also they merge if UV the same, I suppose. It is unwanted behavior for game artists, who use overlapped uv`s very often and want o keep ability to separate islands.
For example I bought stock-model with overlapped face and I want to separate halves and paint them individually.

Here is an issue: T97858: UV: Overlapped vertices with same geometry and positions meged (unwanted merge)

@Vyacheslav (hitrpr) merging would only take place if the UV share a vertex. The issue with detecting islands as separate isn't changed by D14841 (compared to reverting rBb88dd3b8e7b9: UV: remove selection threshold for nearby coordinates).

Merge uv's that "belong" to the same vertex and are within a small (arbitrary) distance from each other in uv space.

Discussed with Sergey and we agree it's a reasonable option, uploaded a patch D14841.

Great call, nice resolution. Code looks like it will address the problem with minimal side effects. Do you want me to try run it against my test cases here?

Note that this it would still be good to resolve the issue with opensubdiv, but it's not as pressing if merging is performed.

Agreed, priority is much lower with D14841, it becomes more of a performance issue at that point.

I'll park these resources here in case someone wants to pick it up later:

The two .OBJ files:

A change to the Blender OBJ exporter to support high precision verts and uvs: