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

[1.2.7-dev] Extra perimeter regression #2664

Closed
Vicious-one opened this Issue Feb 16, 2015 · 7 comments

Comments

Projects
None yet
3 participants
@Vicious-one
Contributor

Vicious-one commented Feb 16, 2015

Now it produces this result on top layer when extra perimeters are enabled:
(2 perimeters + extra)
extgap-2

http://vserv.sinp.msu.ru/temp/infill-gaps.tar.gz

May be somehow related to #2610

@alexrj alexrj added this to the 1.2.7 milestone Feb 23, 2015

alexrj added a commit that referenced this issue Feb 23, 2015

@alexrj

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj Feb 23, 2015

Member

Thank you, fixed! I hope is much more robust now.

Member

alexrj commented Feb 23, 2015

Thank you, fixed! I hope is much more robust now.

@alexrj alexrj closed this Feb 23, 2015

@Vicious-one

This comment has been minimized.

Show comment
Hide comment
@Vicious-one

Vicious-one Feb 24, 2015

Contributor

Seems fixed for this model, thanks, but "30%" logic will most probably fail on some other tricky case.

Contributor

Vicious-one commented Feb 24, 2015

Seems fixed for this model, thanks, but "30%" logic will most probably fail on some other tricky case.

@alexrj

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj Feb 24, 2015

Member

Let's see… the threshold itself might change (40%? 50%?) but I think the logic is robust… I couldn't find any other geometric definition of extra perimeters. They're very very tricky to define

Member

alexrj commented Feb 24, 2015

Let's see… the threshold itself might change (40%? 50%?) but I think the logic is robust… I couldn't find any other geometric definition of extra perimeters. They're very very tricky to define

@Vicious-one

This comment has been minimized.

Show comment
Hide comment
@Vicious-one

Vicious-one Feb 24, 2015

Contributor

IMO, the term itself is a bit ambiguous.

This feature, as I understand it, appeared as an attempt to cover the unsightly infill overlap/overextrusion so that it won't be visible from above.

If I were the one implementing the logic, I would have done it like that:
0) Assume there's a layer below the current one which has P perimeters.

  1. Generate an external perimeter for a current layer
  2. Walk the entire generated perimeter in small steps (like 1 mm) and check if there's an infill region directly below each point.
  3. If the quantity of points which have infill region below them is greater than N (N > 0), recalculate the layer below with P=P+1 perimeters.
  4. Goto step 2 and repeat this untill the infill region appears under less than N points of the external perimeter of the current layer.

Of course, in case vertical extrusions are present, the problem would be exactly the same as in this issue. In this case, the more proper solution seems to be:

  1. Calculate the infill area of the current layer
  2. Check if a tunable area delta threshold with the underlying layer is met. Maybe someone does not want to see infill overlap at all, and in his case the result which was considered erroneous is actually appropriate.
Contributor

Vicious-one commented Feb 24, 2015

IMO, the term itself is a bit ambiguous.

This feature, as I understand it, appeared as an attempt to cover the unsightly infill overlap/overextrusion so that it won't be visible from above.

If I were the one implementing the logic, I would have done it like that:
0) Assume there's a layer below the current one which has P perimeters.

  1. Generate an external perimeter for a current layer
  2. Walk the entire generated perimeter in small steps (like 1 mm) and check if there's an infill region directly below each point.
  3. If the quantity of points which have infill region below them is greater than N (N > 0), recalculate the layer below with P=P+1 perimeters.
  4. Goto step 2 and repeat this untill the infill region appears under less than N points of the external perimeter of the current layer.

Of course, in case vertical extrusions are present, the problem would be exactly the same as in this issue. In this case, the more proper solution seems to be:

  1. Calculate the infill area of the current layer
  2. Check if a tunable area delta threshold with the underlying layer is met. Maybe someone does not want to see infill overlap at all, and in his case the result which was considered erroneous is actually appropriate.
@alexrj

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj Feb 24, 2015

Member

Tuning your N or the area delta threshold is the same as my 30% :-)

The algorithmic problem is that your upper layer might be much smaller or it might have a very different shape from the lower one, but in case a part of its contour would benefit from an extra lower perimeter we need to check whether it's worth or not. Adding one or more extra perimeters just for hiding a very small quantity of infill endpoints is not worth. This is why the current logic only does that if the two layers share at least 30% of their shape… Maybe this 30% should even become 50%, but I'll wait for test cases...

Member

alexrj commented Feb 24, 2015

Tuning your N or the area delta threshold is the same as my 30% :-)

The algorithmic problem is that your upper layer might be much smaller or it might have a very different shape from the lower one, but in case a part of its contour would benefit from an extra lower perimeter we need to check whether it's worth or not. Adding one or more extra perimeters just for hiding a very small quantity of infill endpoints is not worth. This is why the current logic only does that if the two layers share at least 30% of their shape… Maybe this 30% should even become 50%, but I'll wait for test cases...

@Vicious-one

This comment has been minimized.

Show comment
Hide comment
@Vicious-one

Vicious-one Feb 24, 2015

Contributor

Anyway, in case this actually backfires, there can be a workaround: use a maximum extra perimeters threshold (instead of the magic X%).

Contributor

Vicious-one commented Feb 24, 2015

Anyway, in case this actually backfires, there can be a workaround: use a maximum extra perimeters threshold (instead of the magic X%).

@curiouspl2

This comment has been minimized.

Show comment
Hide comment
@curiouspl2

curiouspl2 May 25, 2015

once again - You did introduce very nice variable, but You are somehow reluctant to put it into EXPERT options :) it would fit VERY nicely on right side of the 'extra perimeters if needed' as simply 'threshold' with some nice hint :)
I think the setting will greatly depend on layer height users try to print with and how badly they need their model being watertight from the top (in case of models which are subjected to weathering outside - rain, snow, etc - this is actually usefull) or how much plastic they want to save .

also for very big, complex, or 'production' models this variable could be used like many other from modifier options , allowing to set different thresholds for different parts of model...

note that triggering extra perimeters is performed per layer and not per loop logic, which further complicates things - small loop with big overhang may trigger extra perimeter for large loop without one - or big loop might prevent small loop from getting extra perimeter. both cases might be undesirable . while providing variable will not prevent such border cases to fail, it at least provides user with some way to work-around the problem.

once again - You did introduce very nice variable, but You are somehow reluctant to put it into EXPERT options :) it would fit VERY nicely on right side of the 'extra perimeters if needed' as simply 'threshold' with some nice hint :)
I think the setting will greatly depend on layer height users try to print with and how badly they need their model being watertight from the top (in case of models which are subjected to weathering outside - rain, snow, etc - this is actually usefull) or how much plastic they want to save .

also for very big, complex, or 'production' models this variable could be used like many other from modifier options , allowing to set different thresholds for different parts of model...

note that triggering extra perimeters is performed per layer and not per loop logic, which further complicates things - small loop with big overhang may trigger extra perimeter for large loop without one - or big loop might prevent small loop from getting extra perimeter. both cases might be undesirable . while providing variable will not prevent such border cases to fail, it at least provides user with some way to work-around the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment