X3D Exporter generates duplicate edges and splits the mesh when exporting with Triangulate #49420

New Issue

Michael Stahre · 2016-09-21T16:16:35+02:00

Michael Stahre commented

2016-09-21 16:16:35 +02:00

System Information
Windows 10, NVidia GTX 780

Blender Version
Broken: 2.76, 2.78 RC1, 2.78 RC2
Worked: Unknown

Short description of error
When running the x3d exporter with Triangulate enabled some of the vertices in the mesh get saved multiple times resulting in duplicate vertices and edges. The result is a "cut" in the surface that can be opened up by moving one of the duplicate vertices or edges.
The attached screenshot shows the original mesh (to the left, with one of the edges that get duplicated selected) and the mesh after exporting and then reimporting the mesh with Triangulate enabled (to the right) with the "cut" opened to show which edges are being duplicated.

Exact steps for others to reproduce the error

Open the .blend file.
Export to .x3d, with Triangulate enabled
Reimport the .x3d file into blender or open it in any other software that supports .x3d files.

duplicate_edges_repro.blend

**System Information** Windows 10, NVidia GTX 780 **Blender Version** Broken: 2.76, 2.78 RC1, 2.78 RC2 Worked: Unknown **Short description of error** When running the x3d exporter with Triangulate enabled some of the vertices in the mesh get saved multiple times resulting in duplicate vertices and edges. The result is a "cut" in the surface that can be opened up by moving one of the duplicate vertices or edges. The attached screenshot shows the original mesh (to the left, with one of the edges that get duplicated selected) and the mesh after exporting and then reimporting the mesh with Triangulate enabled (to the right) with the "cut" opened to show which edges are being duplicated. **Exact steps for others to reproduce the error** 1. Open the .blend file. 2. Export to .x3d, with Triangulate enabled 3. Reimport the .x3d file into blender or open it in any other software that supports .x3d files. [duplicate_edges_repro.blend](https://archive.blender.org/developer/F365822/duplicate_edges_repro.blend) ![duplicate_edges_repro.png](https://archive.blender.org/developer/F365823/duplicate_edges_repro.png)

Michael Stahre commented

2016-09-21 16:16:35 +02:00

Changed status to: 'Open'

Michael Stahre commented

2016-09-21 16:16:35 +02:00

Added subscriber: @MichaelStahre

Michael Stahre commented

2016-09-21 16:26:59 +02:00

I have already done some analysis of the bug and discovered the following:

In io_scene_x3d/export_x3d.py down by line 750, when the vertices that get duplicated are being stored in vert_tri_list, the key that is stored in vertex_hash is a tuple containing the uv coordinates of that vertex. From my observation there seems to be a risk of some of the faces that are enumerated over that contain the same edges having slightly different UV coordinates stored (which seems like it might be a float precision problem.)
The solution I am using to get around this which seems to work is to replace line 736:

                                  mesh_faces_uv[fidx].uv[f_cnr_idx][:],

with

                                  tuple([round(val, 7) for val in mesh_faces_uv[fidx].uv[f_cnr_idx][:]]),

Which means I'm really just rounding it down to 7 decimal points, at which point the precision error seems to go away on the meshes I have gotten the problem in.
This might not be the root cause (as it might be a bug that the UV coordinates differ) which is why I am submitting this as a bug rather than a patch.

I am also not sure if there is a reason why the UV coordinates are stored as the key instead of the vertex index (v_idx), it is possible that using that index instead of relying on the UV coordinates might be a better solution.

I have already done some analysis of the bug and discovered the following: In io_scene_x3d/export_x3d.py down by line 750, when the vertices that get duplicated are being stored in vert_tri_list, the key that is stored in vertex_hash is a tuple containing the uv coordinates of that vertex. From my observation there seems to be a risk of some of the faces that are enumerated over that contain the same edges having slightly different UV coordinates stored (which seems like it might be a float precision problem.) The solution I am using to get around this which seems to work is to replace line 736: ``` mesh_faces_uv[fidx].uv[f_cnr_idx][:], ``` with ``` tuple([round(val, 7) for val in mesh_faces_uv[fidx].uv[f_cnr_idx][:]]), ``` Which means I'm really just rounding it down to 7 decimal points, at which point the precision error seems to go away on the meshes I have gotten the problem in. This might not be the root cause (as it might be a bug that the UV coordinates differ) which is why I am submitting this as a bug rather than a patch. I am also not sure if there is a reason why the UV coordinates are stored as the key instead of the vertex index (v_idx), it is possible that using that index instead of relying on the UV coordinates might be a better solution.

Sergey Sharybin commented

2016-09-27 12:31:28 +02:00

Added subscribers: @mont29, @Sergey

Bastien Montagne was assigned by Sergey Sharybin

2016-09-27 12:31:28 +02:00

Sergey Sharybin commented

2016-09-27 12:31:28 +02:00

@mont29, see you in the addon maintainers! ;) Mind having a look here? :)

Sebastian Ullrich commented

2016-11-03 09:19:34 +01:00

Added subscriber: @souljedi

Michael Stahre commented

2016-11-04 10:00:56 +01:00

The workaround I described turned out to not work for cases where a split is actually intended, since any vertices that are at the same coordinates will end up being merged.
My new approach, which so far seems to work, was to still round the coordinates down to the 7th decimal but to also add the vertex id to the key when matching on uv.

So

                elif is_uv:
                    slot_uv = 0

                    def vertex_key(fidx, f_cnr_idx):
                        return (
                            mesh_faces_uv[fidx].uv[f_cnr_idx][:],
                        )

instead becomes

                elif is_uv:
                    slot_uv = 0

                    def vertex_key(fidx, f_cnr_idx, v_idx):
                        return (
                            tuple([round(val, 7) for val in mesh_faces_uv[fidx].uv[f_cnr_idx][:]] + [v_idx]),
                        )

And I then just made sure to pass in v_idx to vertex_key when looping through the faces right below in the code.

The workaround I described turned out to not work for cases where a split is actually intended, since any vertices that are at the same coordinates will end up being merged. My new approach, which so far seems to work, was to still round the coordinates down to the 7th decimal but to also add the vertex id to the key when matching on uv. So ``` elif is_uv: slot_uv = 0 def vertex_key(fidx, f_cnr_idx): return ( mesh_faces_uv[fidx].uv[f_cnr_idx][:], ) ``` instead becomes ``` elif is_uv: slot_uv = 0 def vertex_key(fidx, f_cnr_idx, v_idx): return ( tuple([round(val, 7) for val in mesh_faces_uv[fidx].uv[f_cnr_idx][:]] + [v_idx]), ) ``` And I then just made sure to pass in v_idx to vertex_key when looping through the faces right below in the code.

Sebastian Ullrich commented

2017-01-24 17:24:19 +01:00

It's been a while since this was commited, anyone how can find the time to review this? Thanks

Sebastian Ullrich commented

2017-05-11 08:43:30 +02:00

sorry to bother. Any updates on this?

any feedback would be greatly appreciated so that we can move forward with other exporter contributions for the blender community.

sorry to bother. Any updates on this? any feedback would be greatly appreciated so that we can move forward with other exporter contributions for the blender community.

Sebastian Ullrich commented

2018-07-04 16:29:50 +02:00

Sorry to bother again. Any news on this? We submitted it over a year ago.

blender-admin commented

2019-03-17 17:30:16 +01:00

This issue was referenced by 4ecd2f7a5f

This issue was referenced by 4ecd2f7a5f13f170ff578e8e0e59f271dd8e5ccd

Bastien Montagne commented

2019-03-17 17:44:35 +01:00

Changed status from 'Open' to: 'Resolved'

Bastien Montagne closed this issue

2019-03-17 17:44:35 +01:00

Bastien Montagne commented

2019-03-17 17:45:12 +01:00

Pushed a fix in 2.8's version of the add-on now, based on your solution. No need to use the vertex index though, keys are used in a per-vertex context already.

Sign in to join this conversation.