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

Wrong bridge detection result (FOR TESTING ONLY) #4678

Merged
merged 1 commit into from
Apr 7, 2024

Conversation

SoftFever
Copy link
Owner

@SoftFever SoftFever commented Mar 24, 2024

Quick and dirty.
Haven't got time to dive deep to the original code logic yet.
Initial testing result seems fine.
image

@igiannakas
OrcaCube.zip

@igiannakas
Copy link
Contributor

Makes sense, subtract top area from the bridging area, good one! The only thing I can think off is whether we need to add an expansion margin to the top area unless that is covered elsewhere? To ensure there is enough expansion basically underneath the solid infill lines above the Z.

Will also need to regression test it with the sloped models to make sure bridging is still performed properly there.

Will test a bit when back home.

@SoftFever
Copy link
Owner Author

Yeah, bridge margin is usually handled in perimeter generator
image

@igiannakas
Copy link
Contributor

igiannakas commented Mar 24, 2024

Initial tests show no regressions. Sloped surfaces look good, no impact there. Standard models look also OK.

To contain potential impact for defects in the 2.0 release, maybe push this in 2.1 after more testing by the community?

I've merged it in my dev branch and use it as my daily just in case any issues pop up :)

@SoftFever
Copy link
Owner Author

Initial tests show no regressions. Sloped surfaces look good, no impact there. Standard models look also OK.

To contain potential impact for defects in the 2.0 release, maybe push this in 2.1 after more testing by the community?

I've merged it in my dev branch and use it as my daily just in case any issues pop up :)

I think so, too

@SoftFever SoftFever merged commit ba414bb into main Apr 7, 2024
12 checks passed
@SoftFever SoftFever deleted the feature/wrong_internal-bridge-type branch April 7, 2024 14:43
@vgdh
Copy link
Contributor

vgdh commented Apr 7, 2024

something maybe wrong
sudden flow change in the bridge
1
2
Cube.zip

@igiannakas
Copy link
Contributor

something maybe wrong sudden flow change in the bridge 1 2 Cube.zip

Is this also happening with the 2.0 release or not?

@vgdh
Copy link
Contributor

vgdh commented Apr 7, 2024

same behavior

@igiannakas
Copy link
Contributor

So it’s not this PR then, right?

@vgdh
Copy link
Contributor

vgdh commented Apr 7, 2024

So it’s not this PR then, right?

yeah

powpingdone pushed a commit to powpingdone/OrcaSlicer that referenced this pull request Apr 10, 2024
Fixed an issue that top surface was treated as internal bridge
powpingdone pushed a commit to powpingdone/OrcaSlicer that referenced this pull request Apr 11, 2024
Fixed an issue that top surface was treated as internal bridge
SoftFever added a commit that referenced this pull request Apr 13, 2024
Fixed an issue that top surface was treated as internal bridge
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.

3 participants