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

Added "checks" before exporting project folders. Made changes in "Mesh2Input.py" #141

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sanketjain1101
Copy link

@sanketjain1101 sanketjain1101 commented Aug 20, 2024

During export of the project folder, if 3D print check- Check geometry is marked, the code will check for non-manifolds, intersections and sharp edges in the imported graded meshes. If there are any problems found, the project is not exported and the errors are reported in a text file called "Error_Checkall_info.txt" which is saved to the directory.

  • Add checks otherwise done in NumCalc (see mal from Wolfgang)
    • Face normals
    • Regularity of triangles
  • Add tests for the new feature (Fabian).

During export of the project folder, if 3D print check- Check geometry is marked, the code will check for non-manifolds, intersections and sharp edges in the imported graded meshes. If there are any problems found, the project is not exported and the errors are reported in a text file called "Error_Checkall_info.txt" which is saved to the directory.
Copy link
Contributor

@f-brinkmann f-brinkmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the effort - nice first draft. The functionaliy is good already, my comments mostly relate to code structure and makeing the output more informative for users.

Comment on lines +89 to +93
checkall: BoolProperty(
name="3D-Print Checks",
description= ("Check geometry"), #Added by Sanket
default=True,
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • A general question: should we have separate checks or only one box that always does all checks?
  • Lets find a different name for checkall this could be anything
  • Lets find a different name for '3D-Print checks' since Mesh2HRTF is not about printing
  • The description should be more specific: What is checked for which mesh?
  • There should not be any #Added by Sanket or similar comments. Git keeps track of who added which code...

@@ -200,6 +209,7 @@ def draw(self, context):
row.prop(self, "programPath")
row = layout.row()
row.prop(self, "pictures")
row.prop(self, "checkall") #Added by Sanket
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case we add separate parameters for different checks, this should get its own subcategory

Comment on lines +332 to +334
if checkall:
_write_checkall_txt(filepath1) #Added by Sanket

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to move this up, where we test for a triangular mesh. It might even make sense to include rthe triangular check only if the user wants to do the checks. Can you benchmark how long those checks take? This would help to decide if it would make sense to include separate parameters for different checks.



#Added by Sanket
def _write_checkall_txt(filepath1):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets find a more descriptive function name (see comment on checkall above)

Comment on lines +1195 to +1199
#check edges are below the sharpness preference
bpy.ops.mesh.print3d_check_sharp()
info = report.info()
sharp_edge = info[0][0]
sharp_edge_num = int(re.search(r'\d+', sharp_edge).group())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sharp edges might indeed be a problem in the BEM. Wolfgang might have an idea about how critical this is. Can the threshold for reporting a sharp edge be set?

Comment on lines +1203 to +1204
file = open(os.path.join(filepath1, "Error_Checkall_info.txt"), "w",
encoding="utf8", newline="\n")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any file operations should use the with statement. It makes sure that the file access is closed, even if an error occurs:

Suggested change
file = open(os.path.join(filepath1, "Error_Checkall_info.txt"), "w",
encoding="utf8", newline="\n")
with open(os.path.join(filepath1, "Error_Checkall_info.txt"), "w",
encoding="utf8", newline="\n") as file:
file.write('text\n')

Lets also think about a good filename.

fw = file.write
# header --------------------------------------------------------------
fw("##-------------------------------------------\n")
fw("## This file was created by mesh2input, export failed because of non-zero values during check-all\n")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be more illustrative. Maybe add line breaks for readibility:

Suggested change
fw("## This file was created by mesh2input, export failed because of non-zero values during check-all\n")
fw("## This file was created by the Mesh2HRTF export plug-in. The export failed because the Reference mesh might not be suitable for BEM computations. Try to fix the errors listed below or disable the checks if you are sure that the mesh can be used regardless of the errors.\n")

fw("## %s\n" % sharp_edge)
fw("##\n")
fw("##\n")
fw("Please make sure the object doesn't have non manifold edges/bad contiguous edges/intersect faces/sharp edges before export")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line can be deleted if following the comment above

Comment on lines +1212 to +1214
fw("## %s\n" % bad_contiguous)
fw("## %s\n" % intersect_face)
fw("## %s\n" % sharp_edge)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two suggestions:

  • only write lines if there is a specific problem. E,g,: if bad_contiguous:
  • It would be best if we gave hints on how to fix this. What would be best: add text here or link to the wiki, where we can add pictures?

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

Successfully merging this pull request may close these issues.

2 participants