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

Various bug fixes and improvements #78

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Wasupmacuz
Copy link

This branch of my fork primarily fixes #56 and fixes #77.

Fixed the stringing due to travel moves occurring right before a feature change.
Fixed the stringing travel moves included in the Hilbert curve polygons.
Moves the `modify` and `gcodeWasModified` booleans to the bottom of the loop, ensuring that if the polygon is discarded, we keep the old geometry.
Makes the checks for if GCODE was modified more robust.
Adds a comment explaining how to disable special cooling, 68.
Fixes bug where wipe moves cause invalid polygons, 445-455.
Increased specificity for faster runtime, 531.
Increased readability of conditional, 551.
Corrects some mild spelling and grammatical errors in the script header.
Removes trailing space characters from the file (to shrink file size)
Copy link
Author

@Wasupmacuz Wasupmacuz left a comment

Choose a reason for hiding this comment

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

Comments describing my changes.

for ohp in overhangs:
if poly.distance(ohp)<minDistForValidation:
if ohp.length>self.parameters.get("MinBridgeLength"):
self.validpolys.append(poly)
self.deleteTheseInfills.append(idp)
break
if self.parameters.get("PrintDebugVerification"):print(f"Layer {self.layernumber}: Poly{idp} is not close enough to overhang perimeters")
if self.parameters.get("PrintDebugVerification"):print(f"Layer {self.layernumber}: Poly{idp} is not close enough to overhang perimeters")
Copy link
Author

Choose a reason for hiding this comment

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

Changed indentation to print for every failed ohp instead of once every time after the loop.

Copy link
Author

Choose a reason for hiding this comment

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

Everything below this point is whitespace removal.

@@ -65,7 +65,7 @@ def makeFullSettingDict(gCodeSettingDict:dict) -> dict:
"aboveArcsPerimeterPrintSpeed":3*60, #Unit: mm/min
"applyAboveFanSpeedToWholeLayer":True,
"CoolingSettingDetectionDistance":5, #if the gcode line is closer than this distance to an infill polygon the cooling settings will be applied. Unit:mm
"specialCoolingZdist":3, #use the special cooling XX mm above the arcs.
"specialCoolingZdist":3, #use the special cooling XX mm above the arcs. Set to a negative value to disable (not recommended).
Copy link
Author

Choose a reason for hiding this comment

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

Tells the user how to disable Hilbert curves. Could possibly stand to be more specific...


#ARC GENERATION
if layer.validpolys:
modify=True
gcodeWasModified=True
numOverhangs += 1
Copy link
Author

Choose a reason for hiding this comment

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

We track the number of overhangs for error printing if arc generation fails. Not required, but could be useful.

#apply special cooling settings:
if len(layer.oldpolys)>0:
modify=True
gcodeWasModified=True
Copy link
Author

Choose a reason for hiding this comment

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

We moved these to the bottom of the loop so that they're only made true if arc generation was a success.

prusa_slicer_post_processing_script.py Show resolved Hide resolved
if self.lines[start - 1].startswith("G1 E") or not "E" in self.lines[start - 1]: # if a travel move came right before this feature:
start -= 1
while not "E" in self.lines[start - 1]: # include this travel move in the feature (excluding the first retraction step, if applicable)
start -= 1
Copy link
Author

Choose a reason for hiding this comment

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

This is part of the fix for #56. Including the first retraction step also removes the stringing but gets rid of the retraction and de-retraction steps. Therefore, we leave out the first retraction step.

if idf<1:
return None
lines=self.features[idf-1][1]
for line in reversed(lines):
if "G1" in line:
if line.startswith("G1 X"):
Copy link
Author

Choose a reason for hiding this comment

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

More clearly expresses our intent and runs marginally faster.

@@ -529,17 +548,18 @@ def makeExternalPerimeter2Polys(self)->None:
warnings.warn(f"Layer {self.layernumber}: Could not fetch real StartPoint.")
linesWithStart=linesWithStart+lines
extPerimeterIsStarted=True
if (idf==len(self.features)-1 and extPerimeterIsStarted) or (extPerimeterIsStarted and not ("External" in ftype or "Overhang" in ftype)) :#finish the poly if end of featurelist or different feature
if extPerimeterIsStarted and (idf==len(self.features)-1 or not ("External" in ftype or "Overhang" in ftype)) :#finish the poly if end of featurelist or different feature
Copy link
Author

Choose a reason for hiding this comment

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

Shorter and more readable.

for line in lines:
if "G1" in line and (not isWipeMove):
if (not "E" in line) and travelstr in line and splitAtTravel:
#print(f"Layer {self.layernumber}: try to split feature. No. of pts before:",len(pts))
if len(pts)>=2:#make at least 1 ls
parts.append(pts)
pts=[]# update self.features... TODO
elif "E" in line: #maybe fix error of included travel moves?
pts=[]# update self.features... TODO
Copy link
Author

Choose a reason for hiding this comment

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

The other half of the fix for #56. This one removes the included travel moves in the Hilbert curves by always clearing single points when we travel.

pts=[]# update self.features... TODO
elif "E" in line: #maybe fix error of included travel moves?
pts=[]# update self.features... TODO
elif "E" in line: #maybe fix error of included travel moves?
Copy link
Author

Choose a reason for hiding this comment

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

Do we get rid of this comment now?

@Wasupmacuz Wasupmacuz marked this pull request as ready for review April 13, 2024 22:47
Copy link

@jperry004 jperry004 left a comment

Choose a reason for hiding this comment

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

Works well for me, I've been using the patch for a few days and generated many files with it. Reduces failed generation of certain geometries and reduces stringing. No regressions noticed for me.

@Nathan22211
Copy link

did you try fixing #59 yet, or did that get patched already?

@Wasupmacuz
Copy link
Author

Wasupmacuz commented Nov 24, 2024

did you try fixing #59 yet, or did that get patched already?

@Nathan22211 That must have been related to another issue because yes, this fixes #59.
image

@Nathan22211
Copy link

nice, I'll have to see if it's easy enough to bring the fixed-up code into lousassole and Keilish's orca slicer fork of the script because orca is my main slicer

@Wasupmacuz
Copy link
Author

Wasupmacuz commented Nov 30, 2024

nice, I'll have to see if it's easy enough to bring the fixed-up code into lousassole and Keilish's orca slicer fork of the script because orca is my main slicer

You may or may not want to wait for just a few minutes. I'm just about to finalize some changes to the script on my dev branch that fixes some outstanding bugs that I found while looking at this Kyogre. The script will also run faster 👍

@Nathan22211
Copy link

yeah, I have no clue where to begin here... I've been looking at the function between lousassole's orca script and this and I have no clue what needs to be move into the orca script to bring your changes to work in orca slicer

@Wasupmacuz
Copy link
Author

@Nathan22211 I'll take a crack at it when I get a moment. I was looking at that fork and saw many changes to Nic's code that would make it not so straightforward to combine the two.

@Nathan22211
Copy link

yeah I actually can't get any Orca version script I find to work, they don't work for me it seems, also setuptools might need to be added to requirements because distutils was removed in 3.12

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