Ply Exporter broken with use_uv=True if uv non-unique #99249

New Issue

Ömercan Yazici · 2022-06-28T20:54:34+02:00

Ömercan Yazici commented

2022-06-28 20:54:34 +02:00

Description of problem:
Exporting a bmesh with the ply plugin while setting use_uv=True can produce broken meshes if the vertices of a face share the same loop and the same uv. The vertices are flagged as "non-unique" as the uv attribute match, but position and other attributes are completely ignored. See 94bd481980 (especially around line 105 in io_mesh_ply/export_ply.py)

On my humble opinion this should not be the intended behavior as the uv attribute can be whatever the user wants it to be. I would rather recommend to base the "uniqueness" of a vertex on all exported attributes.

Steps to reproduce
A simple reproduction of the problem is to select all the uvs of the default cube, scale it with 0 and export it via ply. The resulting ply file will be like this:

ply
format ascii 1.0
comment Created by Blender 3.2.0 - www.blender.org
element vertex 1
property float x
property float y
property float z
property float nx
property float ny
property float nz
property float s
property float t
element face 6
property list uchar uint vertex_indices
end_header
1.000000 1.000000 1.000000 0.577350 0.577350 0.577350 0.500000 0.500000
4 0 0 0 0
4 0 0 0 0
4 0 0 0 0
4 0 0 0 0
4 0 0 0 0
4 0 0 0 0

Possible solution:
This should be easy to fix. Just integrate all exported attributes into the ply_vert_map key.

Attached data:
See attached blend file for the actual scene:
PlyExportError.blend
The resulting ply file. It is in ASCII format for readability:
PlyExportError.ply

Operating system and hardware information do not matter here, but for completeness:
System Information
Operating system: Ubuntu 22.04, Windows 11
Graphics card: Nvidia RTX 3080, Nvidia RTX 2080 Super and Nvidia Quadro blender/blender#2000

Blender Version
Broken: 3.2.0 stable and Blender 3.1.x stable

**Description of problem:** Exporting a bmesh with the ply plugin while setting `use_uv=True` can produce broken meshes if the vertices of a face share the same loop and the same `uv`. The vertices are flagged as "non-unique" as the `uv` attribute match, but position and other attributes are completely ignored. See 94bd481980 (especially around line 105 in `io_mesh_ply/export_ply.py`) On my humble opinion this should not be the intended behavior as the `uv` attribute can be whatever the user wants it to be. I would rather recommend to base the "uniqueness" of a vertex on all exported attributes. **Steps to reproduce** A simple reproduction of the problem is to select all the `uv`s of the default cube, scale it with 0 and export it via ply. The resulting ply file will be like this: ``` ply format ascii 1.0 comment Created by Blender 3.2.0 - www.blender.org element vertex 1 property float x property float y property float z property float nx property float ny property float nz property float s property float t element face 6 property list uchar uint vertex_indices end_header 1.000000 1.000000 1.000000 0.577350 0.577350 0.577350 0.500000 0.500000 4 0 0 0 0 4 0 0 0 0 4 0 0 0 0 4 0 0 0 0 4 0 0 0 0 4 0 0 0 0 ``` **Possible solution:** This should be easy to fix. Just integrate all exported attributes into the `ply_vert_map` key. **Attached data:** See attached blend file for the actual scene: [PlyExportError.blend](https://archive.blender.org/developer/F13235122/PlyExportError.blend) The resulting ply file. It is in ASCII format for readability: [PlyExportError.ply](https://archive.blender.org/developer/F13235125/PlyExportError.ply) *Operating system and hardware information do not matter here, but for completeness:* **System Information** Operating system: Ubuntu 22.04, Windows 11 Graphics card: `Nvidia RTX 3080`, `Nvidia RTX 2080 Super` and `Nvidia Quadro blender/blender#2000` **Blender Version** Broken: 3.2.0 stable and Blender 3.1.x stable

Ömercan Yazici commented

2022-06-28 20:54:34 +02:00

Added subscriber: @PearCoding

Pratik Borhade commented

2022-06-29 09:14:49 +02:00

Added subscriber: @PratikPB2123

Omar Emara commented

2022-07-05 13:51:08 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Ömercan Yazici commented

2022-07-08 10:57:59 +02:00

As I need a workaround for the problem asap, I fixed the bug myself.

Here I share my solution with the following patch: fix-t99249.patch
It might be a good starting point. It fixes the issue above and is - on my understanding - a complete solution.

As I need a workaround for the problem asap, I fixed the bug myself. Here I share my solution with the following patch: [fix-t99249.patch](https://archive.blender.org/developer/F13270473/fix-t99249.patch) It might be a good starting point. It fixes the issue above and is - on my understanding - a complete solution.

Ömercan Yazici commented

2022-07-09 23:07:03 +02:00

To support non-smooth faces - which the above patch does not - it would make sense to change the lines:

if use_normals:
    normal = v.normal

to something similar like this:

if use_normals:
    normal = v.normal if f.smooth else f.normal

To support non-smooth faces - which the above patch does not - it would make sense to change the lines: ``` if use_normals: normal = v.normal ``` to something similar like this: ``` if use_normals: normal = v.normal if f.smooth else f.normal ```

Chris Blackbourn commented

2023-01-12 09:24:52 +01:00

Added subscribers: @MikhailRachinskiy, @Chris_Blackbourn

Chris Blackbourn commented

2023-01-12 09:24:52 +01:00

Hi @MikhailRachinskiy , Any thoughts on this one? Looks like two potential fixes, one for UVs and a seperate fix for smoothing?

Mikhail Rachinskiy commented

2023-01-12 12:48:12 +01:00

Basing uniqueness of a vertex on corner data basically means splitting edges and duplicating geometry. Which in my opinion is an awful way to export mesh.

I plan to rewrite PLY exporter and importer #103230, so it would use face corner domain to store UVs.
Vertex color data will be taken from vertex domain instead of corner domain.
And after that I will investigate better ways of exporting normals.

This wouldn't work without splitting edges:
normal = v.normal if f.smooth else f.normal

Basing uniqueness of a vertex on corner data basically means splitting edges and duplicating geometry. Which in my opinion is an awful way to export mesh. I plan to rewrite PLY exporter and importer #103230, so it would use face corner domain to store UVs. Vertex color data will be taken from vertex domain instead of corner domain. And after that I will investigate better ways of exporting normals. This wouldn't work without splitting edges: `normal = v.normal if f.smooth else f.normal`

Chris Blackbourn commented

2023-01-14 22:52:41 +01:00

Hey @MikhailRachinskiy, Just trying to understand this one a little more.

Is the situation with UVs and the situation with normals different or the same? (i.e. will the same fix / rewrite work for both, or are there unique issues that are different between UVs and normals?)

In #99249#1473130, @MikhailRachinskiy wrote:
Basing uniqueness of a vertex on corner data basically means splitting edges and duplicating geometry. Which in my opinion is an awful way to export mesh.

Can you expand on this one a little more? I understand that creating additional duplicate geometry is "bad", but at the same time, being unable to export UVs makes the PLY exporter unusable for certain workflows.

How would you feel about having an exporter option where the user can choose to duplicate geometry to support splitting UVs and normals? (I'd be happy to make the change...)

It seems to me that could add that relatively easily in the interim before the rewrite (to use the face corner domain) comes online.

Sorry for the confusion here, I know very little about the PLY exporter and have ended up here coming in a bit sideways.

Hey @MikhailRachinskiy, Just trying to understand this one a little more. Is the situation with UVs and the situation with normals different or the same? (i.e. will the same fix / rewrite work for both, or are there unique issues that are different between UVs and normals?) > In #99249#1473130, @MikhailRachinskiy wrote: > Basing uniqueness of a vertex on corner data basically means splitting edges and duplicating geometry. Which in my opinion is an awful way to export mesh. Can you expand on this one a little more? I understand that creating additional duplicate geometry is "bad", but at the same time, being unable to export UVs makes the PLY exporter unusable for certain workflows. How would you feel about having an *exporter option* where the user can choose to duplicate geometry to support splitting UVs and normals? (I'd be happy to make the change...) It seems to me that could add that relatively easily in the interim before the rewrite (to use the face corner domain) comes online. Sorry for the confusion here, I know very little about the PLY exporter and have ended up here coming in a bit sideways.

Mikhail Rachinskiy commented

2023-01-15 16:06:50 +01:00

The difference between UVs and normals, is that UVs can be stored in corner domain, for normals I don't know yet. If there is no such possibility, then they only way of exporting corner normals is splitting edges.

creating additional duplicate geometry is "bad", but at the same time, being unable to export UVs makes the PLY exporter unusable for certain workflows.

There is no issue with exporting UVs at the moment. The author of this task experiences problems with UVs because they use old Blender version without this fix 60299fcc23

How would you feel about having an exporter option where the user can choose to duplicate geometry to support splitting UVs and normals?

It already works that way and always did, if user clicks export UVs option, then edges get split.

The difference between UVs and normals, is that UVs can be stored in corner domain, for normals I don't know yet. If there is no such possibility, then they only way of exporting corner normals is splitting edges. > creating additional duplicate geometry is "bad", but at the same time, being unable to export UVs makes the PLY exporter unusable for certain workflows. There is no issue with exporting UVs at the moment. The author of this task experiences problems with UVs because they use old Blender version without this fix 60299fcc23 > How would you feel about having an exporter option where the user can choose to duplicate geometry to support splitting UVs and normals? It already works that way and always did, if user clicks export UVs option, then edges get split.

Chris Blackbourn commented

2023-01-15 21:58:37 +01:00

In #99249#1474447, @MikhailRachinskiy wrote:

There is no issue with exporting UVs at the moment. The author of this task experiences problems with UVs because they use old Blender version without this fix 60299fcc23

I've just checked the repro steps here in master in everything seems fine.

In that case, can we go ahead and mark #99249 as a duplicate of #103203 and close it as resolved?

(Or is there more here still to do?)

> In #99249#1474447, @MikhailRachinskiy wrote: > There is no issue with exporting UVs at the moment. The author of this task experiences problems with UVs because they use old Blender version without this fix 60299fcc23 I've just checked the repro steps here in master in everything seems fine. In that case, can we go ahead and mark #99249 as a duplicate of #103203 and close it as resolved? (Or is there more here still to do?)

Mikhail Rachinskiy commented

2023-01-16 19:22:34 +01:00

This issue can be closed as resolved.
If anyone has additional input, they could provide it in #103230

This issue can be closed as resolved. If anyone has additional input, they could provide it in #103230

Chris Blackbourn commented

2023-01-16 22:25:06 +01:00

Changed status from 'Confirmed' to: 'Resolved'

Chris Blackbourn closed this issue

2023-01-16 22:25:06 +01:00

Chris Blackbourn self-assigned this 2023-01-16 22:25:06 +01:00

Chris Blackbourn commented

2023-01-16 22:25:06 +01:00

Closed as duplicate of #103203, 60299fcc23

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Ply Exporter broken with use_uv=True if uv non-unique #99249