Fixes of Quads and NGons. #138

mifth · 2024-10-27T21:48:39Z

Hi, I did fixes for Quads and NGons. Now all works as a charm without crashes!

My video explanation of changes:
https://youtu.be/d9MWNfvQLtc

Some key points:

AddMesh() function was fixed
"polygon" variable was converted to an Array to have an unlimited amount of points for NGons.
indicesIDs array is added to apply ids
Fixed indices.
triangulatePolygon() was updated. It also outputs indicesIDs.
Changes were tested in Blender with many objects which had Quads and NGons. All warks like a charm!

Without these changes XAtlas will be crashing.

Pics:

Generated UVs in Blender with XAtlas:

…ices.

mifth · 2024-10-28T11:39:13Z

Update. I've changed the faceVertexCount to uint32_t as potentially an NGon can have 1000000 indices/points. It can be easily done if a geometry was generated with procedural tools like Houdiny/Blender(GeometryNodes).

69b139d

Here is a simple example how heavy the NGon can be:

…serve() line.

mifth · 2024-10-29T21:41:29Z

Update:

1 I've moved the indicesIDs array to the Triangulator struct.
45d673c

2 I've fixed zero area calculation for NGons in the AddMesh() function.
0a96bd1
#135 (comment)

Before:

After:

…le is added to ChartOptions.

mifth · 2024-11-04T19:46:21Z

Update:

I made final changes to the Quads/NGons support. Now all works without any artifacts!
My commit: 22f4d64

computeBoundaryIntersection variable is added to ChartOptions to remove artifacts (little charts). By default it's false.
lots of Quads and NGons fixes.
No changes to a generation of triangulated meshes.

I recorded a video according to this update to show and tell about new changes:
https://youtu.be/a4NF1gcR3O8

Further discussion can be here: #135

Some pictures of a final result after a generation:

Some pictures to explain a new computeBoundaryIntersection variable which I've added with the latest commit.

mifth · 2024-11-04T19:52:00Z

@jpcy could you please review my pull request. I worked hard to deliver Quads/NGons support.

castano · 2024-11-04T20:25:49Z

I think that computeBoundaryIntersection serves a valid purpose and simply disabling it is not the right solution. When using LSCM the resulting charts can sometimes self-overlap. It appears that this function tries to detect these overlaps in order to penalize these parameterizations. I think this code was @jpcy's addition, so I'm not familiar with it enough to comment what could be going on, but it appears there may be a bug that's causing some valid paramterizations to be rejected. It would be much more useful to investigate the problem in more detail to understand what's happening, or to provide a repro case so that others can take a look.

mifth · 2024-11-04T21:51:41Z

Hi @castano thank you so much for your feedback. Yeah, I agree that the computeBoundaryIntersection might have be a bug or just designed the way when it fails in some cases.

Actually, I tried XAtlas with computeBoundaryIntersection=false on many models and all worked perfectly. As far as I get the LSCM unwrapping is set by default so I had no issues with this mode. I think having a boolean parameter to switch on/off the BoundaryIntersection feature is a good way too to have a possibility to change it (especially if it's buggy :) ).

I've attached a model where you could test the issue:
Monkey_smart_triangulation.zip

mifth · 2024-11-04T22:01:59Z

Sorry, forgot to mention that to reproduce the artifacts just run this simple code:

xatlas::Atlas* atlas = xatlas::Create();
xatlas::AddMeshError meshError = xatlas::AddMesh(atlas, meshDecl);  // Add your Mesh
xatlas::ChartOptions chartOptions = xatlas::ChartOptions();
xatlas::PackOptions packOptions = xatlas::PackOptions();
xatlas::Generate(atlas, chartOptions, packOptions);

siliconvoodoo · 2024-11-05T05:59:57Z

source/xatlas/xatlas.cpp

@@ -5175,14 +5205,22 @@ struct PlanarCharts
 		m_regionFirstFace.clear();
 		m_nextRegionFace.resize(faceCount);
 		m_faceToRegionId.resize(faceCount);
+		const bool hasQuadsOrNGons = m_data.mesh->trianglesToPolygonIDs.size() > 0 ? true : false;
+		Array<bool> parsedFaces;  // For Quads/NGons


miiiiight be beneficial to use std::vector since it'll use that weirdo pseudo bitfield implementation that will reduce memory by a factor 8. Memory consumption by geometry is revealing to be a problem in some scene cases I'm encountering with 100 millions triangles. Maybe less bandwidth can also improve caching and speed, even if it sounds in contradiction with the bit masking and shifts necessary. In GPU talks we say "ALU is free". probably can apply here.

siliconvoodoo · 2024-11-05T06:11:31Z

source/xatlas/xatlas.cpp

@@ -7280,7 +7385,8 @@ class Chart
 			// Computing charts checks for flipped triangles and boundary intersection. Don't need to do that again here if chart is planar.
 			if (m_type != ChartType::Planar && m_generatorType != segment::ChartGeneratorType::OriginalUv) {
 				XA_PROFILE_START(parameterizeChartsEvaluateQuality)
-				m_quality.computeBoundaryIntersection(m_unifiedMesh, boundaryGrid);
+				if (options.computeBoundaryIntersection)  // intersect edges. It can create artifacts (little peeces).


little pieces ?

siliconvoodoo · 2024-11-05T06:25:13Z

Nice that you put so many code captures and the video walkthrough to explain your changes. 👍

castano · 2024-11-05T06:32:32Z

Actually, I tried XAtlas with computeBoundaryIntersection=false on many models and all worked perfectly.

I tried this on the model you provided and noticed that it produced a self-intersection.

This is the kind of error that the boundary intersection code is intended to detect.

I think the charts that have an invalid parameterization may fall back to the "piecewise" parameterization method and that may be causing the artifacts that you have detected.

A better solution may be to add an option to avoid the piecewise parameterization fallback. An even better solution would be to figure out why the piecewise parameterization method is producing these results and try to improve them. Eliminating the self-intersection tests is not a good solution.

I have not reviewed the code changes that add support for n-gons, but at the very least, I think this should be addressed in a separate PR.

mifth · 2024-11-05T09:29:46Z

@siliconvoodoo thank you so much for your feedback.

As far as I know the point of the library is to use only internal classes/structs without any dependencies. So, std::vector is nowhere used in the code. Only internal clases/structs are allowed.

What would you suggest to use instead of internal::Array<bool> parsedFaces; ? Typically, a boolean is represented as 1 byte (8 bits). Is there something less weighter?

I'll fix the typo.

@castano about your latest post:

1 About the green chart 52: This overlap (on your picture) is exaclty the LSCM unwrapping which doesn't count complex charts. This is an expected behaviour when we have smooth/round shapes like a subdivided monkey. The computeBoundaryIntersection() will not fix this issue as it has a simple/primitive process which serves for a different purpose (I'll explain below). This is a common behaviour for such smooth models.

2 As far as I get the computeBoundaryIntersection() function compares Chart1 and Chart2 and intersects their overlapped polygons to Chart3. Did I get it right?

3 It seems you tested my model with not default parameters of ChartOption. Please use default ChartOption and set computeBoundaryIntersection=true. Then you will see gaps/holes like this:

4 About a separate PR: unfortunately, quads and ngons depend on the computeBoundaryIntersection parameter. The computeBoundaryIntersection concerns them directly because it breaks them. So, it should be all in one PR.

mifth · 2024-11-05T14:36:40Z

@castano I checked out the computeBoundaryIntersection() function again. You are right. The function tries to remove intersected faces of a uv map. It works per mesh. But it looks like its pretty complicated...

mifth · 2024-11-05T19:58:37Z

@castano I'm looking at the issue further. It seems the problem is not with computeBoundaryIntersection.
m_isInvalid affects this issue. I've been investigating the issue further.

mifth · 2024-11-05T20:05:12Z

It seems it's somewhere there:

…atches.

mifth · 2024-11-05T23:30:11Z

Hi @castano @siliconvoodoo it seems I was able to fix the issue with my latest commit. A problem was in this commented code:

mifth · 2024-11-05T23:30:50Z

Here is a video with my creepy explanation:
https://youtu.be/FBvfjz8ciHA

mifth · 2024-11-05T23:50:33Z

Just some screenshots of quads and triangles:

I'm very happy with the result!

siliconvoodoo · 2024-11-06T08:08:40Z

source/xatlas/xatlas.cpp

+				if (m_faceInAnyPatch.get(oface))
+					continue;


just this part if @jpcy can give an opinion he'd be the most relevant person for analysis.

@siliconvoodoo yeah, I understand your concern. I've added this check because a triangle/face can be added multiple times to an array. But sure, @jpcy knows better if it makes sense.

xatlas/source/xatlas/xatlas.cpp

Line 6879 in f01768a

m_patch.push_back(face);

Apologies I've deleted my last comment because there were different checks.

Ah, right! I'll delete this check with a new commit.

siliconvoodoo · 2024-11-06T13:11:57Z

I merged your changes locally and tested on some of my scenes with no particular adverse of negative behaviors compared to original master. (edit: still need to check again with last commits)

mifth · 2024-11-06T13:16:57Z

I merged your changes locally and tested on some of my scenes with no particular adverse of negative behaviors compared to original master. (edit: still need to check again with last commits)

Thanks a lot. There are no any big changes for triangulated meshes. Have you tried meshes which have quads/ngons?

mifth · 2024-11-06T20:44:51Z

I've tested 2 versions of monkeys. With quads and with triangles.
Quads: 1.37sec, 31000 quads.
Triangles: 2.2sec, 62000 triangles.

mifth · 2024-11-06T20:50:10Z

And another one test. I made the same amount of quads(duplicated monkeys one time) and the same amount of triangles.

Quads: 1.44sec, 62000 quads.
Triangles: 2.2sec, 62000 triangles.

It looks pretty cool and means that my work for quads and ngons was done pretty well.
But I can't test memory. I guess the Quads version will take more memory.

mifth · 2024-11-07T09:26:38Z

I've removed "#IF 0" fix and will make it as a separate PR as @castano suggested. The Quads/NGons don't depend on it after removing the computeBoundaryIntersection variable.

mifth · 2024-11-07T10:10:58Z

New pull request is here #141

… leave it commented to make an option in future.

…s/NGons and I wasn't be able to fix it. So I commented my fixes in the PiecewiseParam::computeChart() function and set "m_isInvalid" to work only for Triangles. I hope someone will fix the PiecewiseParam::computeChart() function in future.

mifth · 2024-11-08T16:39:43Z

Hi all,

I've made commit here: 2f47d81

Unfortunately, the PiecewiseParam::computeChart() function still makes artifacts for Quads/NGons so I had to switch it off. The code is designed for triangles only and I failed to fix this part. I hope someone will fix the PiecewiseParam::computeChart() function in future.

Without the the PiecewiseParam::computeChart() function all still works well.

…d for flipped faces and zero faces.

mifth · 2024-11-09T09:39:36Z

Update: I uncommented code for flipped polygons only:
a4bb139

siliconvoodoo · 2024-11-11T03:39:08Z

I was going to ask if it wouldn't be simpler to triangulate first then go through triangle parameterization. But I recalled you're doing a blender plugin so you need to preserve the original geometry as much as possible.

Fixes for Quads and NGons.

e74bbbc

This was referenced Oct 27, 2024

Possible Fix for faceVertexCount logic #136

Closed

Wrong calculation with faceVertexCount? #135

Open

support for quad meshes #119

Open

mifth added 2 commits October 27, 2024 23:22

Removed an empty line

83c6c45

Set faceVertexCount=uint32_t as potentially NGon can have 1000000 ind…

69b139d

…ices.

mifth added 2 commits October 29, 2024 21:58

The indicesIDs array is moved to the Triangulated struct and has a re…

45d673c

…serve() line.

Fix for a zero area calculation for NGons. jpcy#135 (comment)

0a96bd1

mifth added 2 commits October 30, 2024 00:46

fix: initialised "area" variable.

710357a

Final changes for Quads and NGons. computeBoundaryIntersection variab…

22f4d64

…le is added to ChartOptions.

siliconvoodoo reviewed Nov 5, 2024

View reviewed changes

Fixed typo

c4703c5

Removed computeBoundingIntersection variable. Fixed quads/ngons for p…

f01768a

…atches.

siliconvoodoo reviewed Nov 6, 2024

View reviewed changes

Delete unneeded check.

2d44719

Reverted "#IF 0" as it will be as a different PR. Little tweaks.

5ac6a5d

Cleanup.

49ce073

mifth mentioned this pull request Nov 7, 2024

Fixes of little patches which look like UV artifacts. #141

Open

mifth added 3 commits November 7, 2024 22:57

Wrong number for a quad.

98806d6

Commented a simple case for "quads to triangles". But I would like to…

15a9d20

… leave it commented to make an option in future.

mifth added 2 commits November 8, 2024 21:48

Uncommented code for the PiecewiseParam::computeChart() as it's neede…

a4bb139

…d for flipped faces and zero faces.

Missed vertices are added.

d92f849

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes of Quads and NGons. #138

Fixes of Quads and NGons. #138

mifth commented Oct 27, 2024 •

edited

Loading

mifth commented Oct 28, 2024

mifth commented Oct 29, 2024 •

edited

Loading

mifth commented Nov 4, 2024 •

edited

Loading

mifth commented Nov 4, 2024

castano commented Nov 4, 2024

mifth commented Nov 4, 2024

mifth commented Nov 4, 2024 •

edited

Loading

siliconvoodoo Nov 5, 2024

siliconvoodoo Nov 5, 2024

siliconvoodoo commented Nov 5, 2024

castano commented Nov 5, 2024

mifth commented Nov 5, 2024 •

edited

Loading

mifth commented Nov 5, 2024 •

edited

Loading

mifth commented Nov 5, 2024

mifth commented Nov 5, 2024

mifth commented Nov 5, 2024

mifth commented Nov 5, 2024

mifth commented Nov 5, 2024

siliconvoodoo Nov 6, 2024

mifth Nov 6, 2024

mifth Nov 6, 2024

mifth Nov 6, 2024

siliconvoodoo commented Nov 6, 2024 •

edited

Loading

mifth commented Nov 6, 2024

mifth commented Nov 6, 2024

mifth commented Nov 6, 2024 •

edited

Loading

mifth commented Nov 7, 2024

mifth commented Nov 7, 2024

mifth commented Nov 8, 2024

mifth commented Nov 9, 2024

siliconvoodoo commented Nov 11, 2024

Fixes of Quads and NGons. #138

Are you sure you want to change the base?

Fixes of Quads and NGons. #138

Conversation

mifth commented Oct 27, 2024 • edited Loading

mifth commented Oct 28, 2024

mifth commented Oct 29, 2024 • edited Loading

mifth commented Nov 4, 2024 • edited Loading

mifth commented Nov 4, 2024

castano commented Nov 4, 2024

mifth commented Nov 4, 2024

mifth commented Nov 4, 2024 • edited Loading

siliconvoodoo Nov 5, 2024

Choose a reason for hiding this comment

siliconvoodoo Nov 5, 2024

Choose a reason for hiding this comment

siliconvoodoo commented Nov 5, 2024

castano commented Nov 5, 2024

mifth commented Nov 5, 2024 • edited Loading

mifth commented Nov 5, 2024 • edited Loading

mifth commented Nov 5, 2024

mifth commented Nov 5, 2024

mifth commented Nov 5, 2024

mifth commented Nov 5, 2024

mifth commented Nov 5, 2024

siliconvoodoo Nov 6, 2024

Choose a reason for hiding this comment

mifth Nov 6, 2024

Choose a reason for hiding this comment

mifth Nov 6, 2024

Choose a reason for hiding this comment

mifth Nov 6, 2024

Choose a reason for hiding this comment

siliconvoodoo commented Nov 6, 2024 • edited Loading

mifth commented Nov 6, 2024

mifth commented Nov 6, 2024

mifth commented Nov 6, 2024 • edited Loading

mifth commented Nov 7, 2024

mifth commented Nov 7, 2024

mifth commented Nov 8, 2024

mifth commented Nov 9, 2024

siliconvoodoo commented Nov 11, 2024

mifth commented Oct 27, 2024 •

edited

Loading

mifth commented Oct 29, 2024 •

edited

Loading

mifth commented Nov 4, 2024 •

edited

Loading

mifth commented Nov 4, 2024 •

edited

Loading

mifth commented Nov 5, 2024 •

edited

Loading

mifth commented Nov 5, 2024 •

edited

Loading

siliconvoodoo commented Nov 6, 2024 •

edited

Loading

mifth commented Nov 6, 2024 •

edited

Loading