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

Fix thicken normal calculation when no override is provided #695

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

danieledapo
Copy link
Contributor

Do not calculate normals if the user did not provide any override, it makes the calculation coherent with what OCCT does.

Fixes #623.

Copy link
Owner

@gumyr gumyr left a comment

Choose a reason for hiding this comment

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

The real problem here is that the normal_override parameter is being changed for subsequent faces. Here is a smaller change that just renames the variable:

    for face in to_thicken_faces:
        face_normal = (
            normal_override if normal_override is not None else face.normal_at()
        )
        for direction in [1, -1] if both else [1]:
            new_solids.append(
                face.thicken(depth=amount, normal_override=face_normal * direction)
            )

I'd like to go this this less complex change.

@danieledapo
Copy link
Contributor Author

good catch! should be ok now!

@gumyr gumyr merged commit debdf84 into gumyr:dev Sep 19, 2024
11 checks passed
@gumyr
Copy link
Owner

gumyr commented Sep 19, 2024

Thanks!

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.

Thicken normal incorrect
2 participants