educelab / volume-cartographer

Volumetric processing toolkit and C++ libraries for the recovery and restoration of damaged cultural materials
GNU General Public License v3.0
60 stars 20 forks source link

[Bug] Resolved edge pair already paired during flattening #36

Open hariseldon137 opened 1 year ago

hariseldon137 commented 1 year ago

What happened?

Some segments made with VC fail to render, without a clear way to fix them. There are a couple different error messages that appear, and I will attempt to collate them here, however the most common is:

[volcart] [error] Resolved edge pair already paired. Edge (17246, 17214) is not 2-manifold.

Steps to reproduce

It is unclear what exactly produces the errors, we think it may be one dot on top of another.

Version

any version

How did you install the software?

None

On which operating systems have you experienced this issue?

Relevant log output

[volcart] [error] Resolved edge pair already paired. Edge (17246, 17214) is not 2-manifold.

Code of Conduct

csparker247 commented 1 year ago

Thanks. That error message comes from our OpenABF flattening library and indicates a non-manifold surface condition detected on the mesh. Specifically, it means that an edge is shared by more than two faces, which would produce a surface shaped like a T. Obviously, this sort of geometry doesn't make sense for our virtual unwrapping purposes and ABF can't actually handle this condition, so it's a hard error for the program.

As you suggest, this is almost definitely caused by points being moved too close to each other. The default geometric pipeline when you run vc_render with -s SEGID is ordered meshing -> ACVD mesh resampling -> flattening. When you do the first part, the mesh connectivity looks something like this:

|-----|-----|
| \   | \   |
|  \  |  \  |
|   \ |   \ |
|-----|-----|

But that doesn't say anything about the distances between points. If the points in the segmentation are really close, you end up with a mesh that looks more like this in 3D:

|-|-|

But all of those faces are still there geometrically, they're now just compacted into a small area and are overlapping in weird ways. When ACVD comes through and resamples the mesh, it ends up connecting faces in weird, non-manifold ways because everything is overlapping strangely in those regions.

So ultimately, this isn't a flattening bug but is a condition of the input data that we don't handle well. In the short term, the unfortunate solution is just don't squish the points too close to each other while segmenting. Think of the points you're drawing as defining a line of more or less fixed length. If that line's length expands or contracts too much, you'll have problems (too much expansion produces things like the #33).

Long term, I can think of some possible solutions to this problem:

  1. Implement non-ordered pointset meshing: Ordered pointset meshing is simple and effective, but assumes a lot about how the user has constructed the pointset. Someone could implement a meshing algorithm which better meshes the implied surface. There are many out there and I have frequently discussed my interest in getting some form of filtered Poisson surface reconstruction implemented.
  2. Remove zero-area faces and duplicate vertices before resampling: Someone could add a filter between the meshing and resampling step which filters out faces with zero area and points which are spatial duplicates (i.e. super close). Merging the vertices is easy. Ensuring the mesh remains manifold may not be.
  3. Change the surface representation: We're trying to model a continuous, manifold surface using discretized pointsets. Why not use a continuous surface representation like NURBS or something? This would be a lot of work.
janpaul123 commented 1 year ago

Which one of these would be the easiest for someone to do? I'm thinking probably (2)? We could perhaps put that in the feature requests doc.

hariseldon137 commented 1 year ago

Instead of merging then duplicating, would it make sense to take the offending points and offset them from each other a minimum distance? Unless the entire line is contracting past a critical threshold, this may keep the mesh manifold and not affect ink detection in any meaningful way.

csparker247 commented 1 year ago

@janpaul123 Yeah, probably 2. There's probably a pretty predictable collapsing scheme for intelligently remeshing the merged region, but it's hard to imagine all of the edge cases without coding it up.

@hariseldon137 It's possible, but I don't know what a reasonable threshold would be to keep it from ultimately generating non-manifold surfaces while also not messing up the surface shape. Someone could test it, I suppose. A twist on that idea would be to add a distance constraint inside the VC GUI keep the user from moving points too close together. That's probably safer in terms of guaranteeing the segmentation follows what you want.

hariseldon137 commented 1 year ago

The points as a whole are naturally moving together if the papyrus goes from lots of curvature to very little curvature. Is this something you encountered in VC historically, or would this be a result of the new segmentation algorithm from Richi we are using?

csparker247 commented 1 year ago

Points getting closer together is expected if there's something like a bubble on the surface, but I've never seen this cause points to get so close together that they result in non-manifold geometry.

In my experience, severe point overlapping occurs when the surface you're following is eroded on the edges and the total visible length gets shorter. In this circumstance, users tend to try to keep all segmentation points on the surface rather than maintaining a fixed segmentation length. I didn't think OFS was doing something like that, but I haven't worked with the V2 one yet.

hariseldon137 commented 1 year ago

dots_1 dots_2

This would be an example of where the dots en masse are coming closer together. The entire line, maybe 8x the length of what you see here, has contracted.

hariseldon137 commented 11 months ago

Seth, quick question on this issue:

When we have an error saying "Edge (17246, 17214) is not 2-manifold", can we use that info to locate where in the segmentation (roughly which slice range) the pathology occurs?

We now have the capacity to re-segment any part of our work without erasing the segmentation above it, so knowing where the pathology occurs would let us fix this easily.

skyward7187 commented 11 months ago

Hi Seth, another segmenter here.

I just wanted to add that these particular issues are currently affecting my two very large segments (50+ and 25+ cm^2), so identifying where the pathology occurs via inspection is quite difficult.

Also, they're too large to simply abandon! We need to fix the error for these particular segments one way or another.

csparker247 commented 11 months ago

Sorry, I don't know why GitHub didn't notify me of these comments.

The two numbers listed are the zero-indexed vertex IDs that are the endpoints of the edge. So in MeshLab, you can select vertices by those indices (may need to add 1? Can't remember if MeshLab is zero-indexed).

Note that these are indices in the ACVD mesh, so you'll need to save that intermediate mesh using vc_render --intermediate-mesh foo.obj .... You can repair the intermediate mesh in MeshLab, then give it back to vc_render with --input-mesh.