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
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions mesh2hrtf/Mesh2Input/mesh2input.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
from bpy.props import StringProperty, BoolProperty, EnumProperty, \
IntProperty, FloatProperty
from bpy_extras.io_utils import ExportHelper
import re
from object_print3d_utils import report

bl_info = {
"name": "Mesh2HRTF export add-on",
Expand Down Expand Up @@ -83,6 +85,13 @@ class ExportMesh2HRTF(bpy.types.Operator, ExportHelper):
description="Render pictures of the 3D mesh",
default=True,
)

checkall: BoolProperty(
name="3D-Print Checks",
description= ("Check geometry"), #Added by Sanket
default=True,
)
Comment on lines +89 to +93
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...


# post-processing ---------------------------------------------------------
reference: BoolProperty(
name="Reference",
Expand Down Expand Up @@ -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

# post-processing
layout.label(text="Post-processing:")
row = layout.row()
Expand Down Expand Up @@ -244,6 +254,7 @@ def save(operator,
frequencyVectorType='Step size',
frequencyVectorValue=100,
pictures=True,
checkall=True, #Added by Sanket
evaluationGrids='ARI',
materialSearchPaths='None',
method='ML-FMM BEM',
Expand Down Expand Up @@ -318,6 +329,10 @@ def save(operator,
if os.path.exists(os.path.join(filepath1, "NumCalc")):
raise ValueError((f"Project folder {filepath1} already exists. "
"Choose another folder or delete files."))
if checkall:
_write_checkall_txt(filepath1) #Added by Sanket

Comment on lines +332 to +334
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.


# create sub-folders
subfolders = ["ObjectMeshes", "EvaluationGrids", "NumCalc"]
for subfolder in subfolders:
Expand Down Expand Up @@ -1160,6 +1175,48 @@ def _render_pictures(filepath1, unitFactor):
nameRender = ("%d_deg_azimuth" % azim[ii])
bpy.data.images['Render Result'].save_render(
os.path.join(dirRender, "%s.png" % nameRender))


#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)

"""check all before export"""
#check for geometry is solid
bpy.ops.mesh.print3d_check_solid()
info = report.info()
non_manifold = info[0][0]
non_manifold_num = int(re.search(r'\d+', non_manifold).group())
bad_contiguous = info[1][0]
bad_contiguous_num = int(re.search(r'\d+', bad_contiguous).group())
#check geometry for self intersections
bpy.ops.mesh.print3d_check_intersect()
info = report.info()
intersect_face = info[0][0]
intersect_face_num = int(re.search(r'\d+', intersect_face).group())
#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())
Comment on lines +1195 to +1199
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?


#If there is non-zero number after check-all then raise an error
if non_manifold_num != 0 or bad_contiguous_num != 0 or intersect_face_num != 0 or sharp_edge_num != 0:
file = open(os.path.join(filepath1, "Error_Checkall_info.txt"), "w",
encoding="utf8", newline="\n")
Comment on lines +1203 to +1204
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("##\n")
fw("##\n")
fw("## %s\n" % non_manifold)
fw("## %s\n" % bad_contiguous)
fw("## %s\n" % intersect_face)
fw("## %s\n" % sharp_edge)
Comment on lines +1212 to +1214
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?

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

file.close()
raise ValueError(("Export failed, please refer to Error_checkall_info.txt in defined folder for details"))


def _write_nc_inp(filepath1, version, title,
Expand Down