Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Number of objects in FreeCAD file cause face already exisits error #4522

Closed
macdroid53 opened this issue Jun 10, 2022 · 39 comments
Closed

Number of objects in FreeCAD file cause face already exisits error #4522

macdroid53 opened this issue Jun 10, 2022 · 39 comments
Labels

Comments

@macdroid53
Copy link

Problem statement

The attached zip has 2 FreeCAD files. One has 4 solids, one has 5 solids.
Using the following connected nodes:

  • File path
  • Read FCStd
  • Solid to Mesh
  • Mesh viewer

Specifying RaspPi4random4.FCStd the Mesh viewer reports no error.
Specifying RaspPi4random5.FCStd the Mesh viewer reports error:
ValueError('faces.new(verts): face already exists)

Note: the original RaspPi4 file has 19 solids. The files attached can include any random selection of 4 or 5 solids and the same result is seen.

Sverchok version

v1.1.0-alpha.2
ObjectCount45.zip

@zeffii
Copy link
Collaborator

zeffii commented Jun 10, 2022

can you copy/paste the terminal printout ? in add-on preferences for sverchok you can set Sverchok into Debug mode, it should print out a lot more information.

@macdroid53
Copy link
Author

`Traceback (most recent call last):
File "/home/mac/.config/blender/2.93/scripts/addons/sverchok-master/core/update_system.py", line 369, in main_update
node.process()
File "/home/mac/.config/blender/2.93/scripts/addons/sverchok-master/nodes/viz/mesh_viewer.py", line 184, in process
me_data.regenerate_mesh(self.base_data_name, v, e, f, m, self.fast_mesh_update)
File "/home/mac/.config/blender/2.93/scripts/addons/sverchok-master/utils/nodes_mixins/generating_objects.py", line 206, in regenerate_mesh
add_mesh_to_bmesh(bm, verts, edges, faces, update_indexes=False, update_normals=False)
File "/home/mac/.config/blender/2.93/scripts/addons/sverchok-master/utils/sv_bmesh_utils.py", line 133, in add_mesh_to_bmesh
[new_face([bm_verts[i] for i in face]) for face in faces or []]
File "/home/mac/.config/blender/2.93/scripts/addons/sverchok-master/utils/sv_bmesh_utils.py", line 133, in
[new_face([bm_verts[i] for i in face]) for face in faces or []]
ValueError: faces.new(verts): face already exists
2022-06-10 11:07:11,811 [DEBUG] sverchok.core.tasks 112: Global update - 1204ms

`

@zeffii
Copy link
Collaborator

zeffii commented Jun 10, 2022

i'm not able to replicate the error

@zeffii
Copy link
Collaborator

zeffii commented Jun 10, 2022

for sv_mesh_utils add_mesh_to_bmesh to give that error

File "/home/mac/.config/blender/2.93/scripts/addons/sverchok-master/utils/sv_bmesh_utils.py", line 133, in
[new_face([bm_verts[i] for i in face]) for face in faces or []]

does narrow down the search :)

@macdroid53
Copy link
Author

Does it show Solid. 5 and read all as shown here:
image

@zeffii
Copy link
Collaborator

zeffii commented Jun 10, 2022

oh.. i see. no i was not displaying the 5 objects simultaneously

@zeffii
Copy link
Collaborator

zeffii commented Jun 10, 2022

i'm a little confused. but it's time for dinner :)

image

@zeffii
Copy link
Collaborator

zeffii commented Jun 10, 2022

there are definitely duplicate polygons in that output from Solid To Mesh. Actually, there appear to be 400 duplicates ( either rotated, or reversed ) polygons in the output of Solid To Mesh. for that one object.

@zeffii
Copy link
Collaborator

zeffii commented Jun 10, 2022

we could add a "Filter Degenerates" Node / API , but it does slow down things a little. The saving grace of this geometry is that it is tris all the way, lending itself nicely to numba/numpy for speedup

@macdroid53
Copy link
Author

Hmm..."there appear to be 400 duplicates ( either rotated, or reversed ) polygons in the output of Solid To Mesh.."

That means those solids aren't manifold, right?

@zeffii
Copy link
Collaborator

zeffii commented Jun 10, 2022

degenerate_filter (redundant faces)

"""
>in Verts v
>in Faces s
out Verts_out v
out Faces_out s
"""
# from sverchok.utils.ascii_print import str_color

def filter_degenerate(verts, faces):
    reference_faces = set()
    good_faces = []
    add_face = good_faces.append
    discarded = 0
    for idx, face in enumerate(faces):
        polygon_shell = tuple(sorted(face))
        if not polygon_shell in reference_faces:
            reference_faces.add(polygon_shell)
            add_face(face)
        else:
            discarded += 1
    # print(str_color(f"{discarded = }", 32))
    return verts, good_faces

for verts, faces in zip(Verts, Faces):
    v, f = filter_degenerate(verts, faces)
    Verts_out.append(v)
    Faces_out.append(f)

image

image

@zeffii
Copy link
Collaborator

zeffii commented Jun 10, 2022

That means those solids aren't manifold, right?

not necessarily, the duplicate faces "bridge" vertices which are already bridged by one other face, which makes those elements "degenerate" (or at the very least redundant). Take for example

[0, 1, 2] = first come first serve.
[2, 1, 0] = reversed
[1, 0, 2] = rotated reversed
[0, 2, 1] = rotated reversed
... etc

the snippet i posted checks if there are existing faces that contain all of the vertex indices of the face i'm about to add, it doesn't distinguish between duplicates/reversed/rotated. The issue appears to be in the conversion algorithm used in Solid to Mesh, it shouldn't be outputting degenerate data :)

MeshViewer shouldn't have to check for degenerates. but @Durman might agree that it could be more informative. If the add_mesh_to_bmesh did a degenerate analysis if it encounters an exception. while populating the bmesh

@zeffii
Copy link
Collaborator

zeffii commented Jun 11, 2022

@macdroid53 there is a mesh cleanup node .

image

@zeffii
Copy link
Collaborator

zeffii commented Jun 11, 2022

i've called it "degenerate" but perhaps redundant is more appropriate, as degenerate may cover a few other scenarios aswel

import numpy as np

def remove_degenerate_triangles(verts, faces):

    F = np.array(faces)
    M = np.sort(F, axis=-1, order=None)

    # from https://stackoverflow.com/a/16973510/1243487
    K = np.ascontiguousarray(M).view(np.dtype((np.void, M.dtype.itemsize * M.shape[1])))
    _, idx = np.unique(K, return_index=True)

    return verts, F[idx]
  • could include a check to remove faces that feature indices which exceed len(verts)-1
  • remove unreferenced verts (and thus shifting the valid indices)

@zeffii
Copy link
Collaborator

zeffii commented Jun 11, 2022

image

see the new filter button. if committed this may be available to those algorithms that output only triangles (i've seen some output ngons and quads..and that's a mixed bag the np version can't handle)

@macdroid53
Copy link
Author

In the image I posted above, the Read FCStd node is different (note: I didn't mod it, I understood it was provided by someone involved with the sverchok project...).
Are you referring to the funnel icon on the Solid to Mesh node?

I see this, or similar, being more of an issue as more complex assemblies are used. The FC file in the OP is just a extracted portion of this file.
RaspPi4-all.FCStd.zip

And even that is a reduction from the original:
RaspPi4_initial.FCStd.zip

This would be a typical application. In fact the various pieces would probably be subsequently used in Blender to do rendering and animation.

@zeffii
Copy link
Collaborator

zeffii commented Jun 11, 2022

Are you referring to the funnel icon on the Solid to Mesh node?

yes, i haven't decided if or how to include that in the node in the master-branch, but the image shows the experiment works.

In the image I posted above, the Read FCStd node is different

well, you may want to modify it in a similar way that I modified sverchok's version. Not much I can do about versions that aren't in Sverchok's distribution.

the various pieces would probably be subsequently used in Blender to do rendering

yeah i'm sure excellent work can be done that way.

@zeffii
Copy link
Collaborator

zeffii commented Jun 11, 2022

i accidentally unzipped those FCStd files and see that they carry an xml with material information (color..etc) , sverchok can set up materials too..

image

@macdroid53
Copy link
Author

I don't think they do have that sort of info at this point...haven't gotten that far in my exploration.

As for the version of FCStd_read.py...I though Haas got it from someone involved with the sverchok project...

@zeffii
Copy link
Collaborator

zeffii commented Jun 11, 2022

OK. well. this might be useful.
https://gist.github.com/yorikvanhavre/680156f59e2b42df8f5f5391cae2660b

@macdroid53
Copy link
Author

Yes, I think Yorik wrote that for his architectural work. It seems to have never been fleshed out, but it may have info to help read the color info. In theory, there is material info possible though.
The source of the FCStd file/s I've uploaded here is originally geometry from a step file, in this particular case, those colors may have originated in the step file.

@zeffii
Copy link
Collaborator

zeffii commented Jun 11, 2022

There's a considerable divergence between your version of the FCStd_read.py and the one that ships with Sverchok. I'm inclined to add this larger version as a MK2 (different classname, similar node) - and then update the code inside that node too. But at the same time update the enum as described earlier.

@macdroid53
Copy link
Author

macdroid53 commented Jun 11, 2022

That code came from rastart.

But, I think it is experimental.

@zeffii
Copy link
Collaborator

zeffii commented Jun 12, 2022

ok! the topic of this issue has drifted a little from the original intent; the bad geometry produced by some algos.

  • a problem was identified
  • the cause is now known
  • a partial solution can be implemented for some modes of the Solid To Mesh node (some modes don't even work on my machine)

@macdroid53
Copy link
Author

i accidentally unzipped those FCStd files and see that they carry an xml with material information (color..etc) , sverchok can set up materials too..

image

Having the colors would be nice when available.

But, at this point I'm not sure I know where this stands. Is there code here I should attempt to try (i.e. cobbling on my system, copy from this thread, etc.) Does it make sense for me to even tinker with it, or just wait...

@zeffii
Copy link
Collaborator

zeffii commented Jun 12, 2022

#4525 see the description for details. This is one step of many.

but I too hope @rastart can keep an eye on these nodes, in-case i do something stupid.

@zeffii zeffii closed this as completed Jun 12, 2022
@macdroid53
Copy link
Author

Given I have his version of Read_FCStd.py with the FName changes, I'm trying to determine what files I need to get to use this.

It appears these 3 files were changed for the merge:

nodes/exchange/FCStd_read.py
nodes/solid/solid_to_mesh_mk2.py
utils/sv_mesh_utils.py

But is my version Read_FCStd.py compatible?

@zeffii
Copy link
Collaborator

zeffii commented Jun 12, 2022

solid_to_mesh_mk2 is a self contained change, but i added a function to sv_mesh_utils... so those are necessary for fixing triangles.

as for the FCStd_read node,

  • the Fname stuff is necessary to change
  • if you want a names socket, you can make those changes locally too probably. :)
    • i can add a FCStd_read_experiment.py with all these fixes merged into your "experimental" version of the node (it will have a different class name and maybe be called FCStd Read Mod

@macdroid53
Copy link
Author

solid_to_mesh_mk2 is a self contained change, but i added a function to sv_mesh_utils... so those are necessary for fixing triangles.

as for the FCStd_read node,

* the `Fname` stuff is necessary to change

* if you want a names socket, you can make those changes locally too probably. :)
  
  * i can add a `FCStd_read_experiment.py`  with all these fixes merged into your "experimental" version of the node  (it will have a different class name and maybe be called `FCStd Read Mod`

If you do the FCStd Read Mod, maybe it then may make more sense for me to just do a git clone?

@zeffii
Copy link
Collaborator

zeffii commented Jun 12, 2022

yeah, probably. The script made by yorik shows yet another way to extract geometry including face colors (other visual properties too), allowing for an automated way to extract materials.

what i'm just not sure about is how this node-based workflow has any benefit over a one-off operator import. one thing i do know is that doing this will teach me about freecad :)

@macdroid53
Copy link
Author

yeah, probably. The script made by yorik shows yet another way to extract geometry including face colors (other visual properties too), allowing for an automated way to extract materials.

So, should I do a clone now?

what i'm just not sure about is how this node-based workflow has any benefit over a one-off operator import. one thing i do know is that doing this will teach me about freecad :)

Hmm...could you elaborate?

@macdroid53
Copy link
Author

macdroid53 commented Jun 14, 2022

yeah, probably. The script made by yorik shows yet another way to extract geometry including face colors (other visual properties too), allowing for an automated way to extract materials.

So, should I do a clone now?

what i'm just not sure about is how this node-based workflow has any benefit over a one-off operator import. one thing i do know is that doing this will teach me about freecad :)

Hmm...could you elaborate?

@zeffii
Don't mean to be bothersome...but, if cloning it will help by letting me exercise things, I'd like to help. Same with the workflow. I have a pretty good handle on the FC workflow and would be happy to explain what I can.

@zeffii
Copy link
Collaborator

zeffii commented Jun 15, 2022

Hmm...could you elaborate? .... Don't mean to be bothersome

sure, questions welcome! it's nice to know this isn't happening in a vacuum. it's not clear how this would be a very pleasant experience due to the slowness of the importing code. My goal is to speed the importer code up ( it seems rather expensive ), while keeping it correct and generate materials. In my scripted node experiment i've added caching to save my nerves a little.

i spent most of my free time yesterday dissecting yorik's fcstd->blender importer script , i'm about 30% way into that process (geometry and transforms work now). Materials per face next. Then after that figure out what to do with the geometry issue (ie faces appear to contain all or some of a previous face's indices (when shared indices are 3 or more )

image

  • some faces produced by the importer are not accepted by bmesh (like Solid_to_Mesh node in Mefisto and trivial mode) but permitted by blender's Object data Mesh storage type,
    • The ViewerDraw node above is intentionally made as lenient as possible to allow it to be used to debug data like this visually.

@zeffii
Copy link
Collaborator

zeffii commented Jun 15, 2022

the pull request to try the read_mod node is #4529 , my changes are mostly subtle internal stuff and the names output socket.

@zeffii
Copy link
Collaborator

zeffii commented Jun 15, 2022

@macdroid53
Copy link
Author

I need to reread your post. But a side note; apparently there is a version of sverchok that reads material from the Label2 property. No idea what that means for what you are doing...

@zeffii
Copy link
Collaborator

zeffii commented Jun 15, 2022

yorik confirms that colors can be derived per face, that's part of the importer code i've still not read with much focus.

@macdroid53
Copy link
Author

macdroid53 commented Jun 15, 2022

So, when is a good time for me to do a clone?

Hmm...I did a clone, but, now I don't know what to do with it...obviously I just can't copy to:
~.config/blender//scripts/addons/sverchok-master/

And, I suppose I should move this to Discussions...

@zeffii
Copy link
Collaborator

zeffii commented Jun 16, 2022

continuing on the topic of " ValueError: faces.new(verts): face already exists "
here's a snippet that shows it

import bmesh
from sverchok.utils.console_print import console_print as prin

bm = bmesh.new()
bm_verts = bm.verts
new = bm_verts.new
new((0.0, 1.0, 2.0))
new((1.0, 1.0, 2.0))
new((2.0, 1.0, 2.0))
new((3.0, 1.0, 2.0))
new((4.0, 1.0, 2.0))

bm_verts.index_update()
bm_verts.ensure_lookup_table()

desired_faces = [
    [0, 1, 2],
    [0, 1, 2, 3, 4],   # <-- this is allowd
    [2, 1, 0],       # <-- this is not.
    [1, 2, 3, 3],  
]
for f in desired_faces:
    try:
        bm.faces.new([bm_verts[i] for i in f])
    except Exception as err:
        prin(f"{f} -- {err}")

outputs

[2, 1, 0] -- faces.new(verts): face already exists
[1, 2, 3, 3] -- faces.new(...): found the same (BMVert) used multiple times

it's not as hopeless as i thought. it appears to literally mean any similar length faces which share identical indices with any existing face will be prohibited

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants