Skip to content

Commit

Permalink
Unhandled exception CURA issue :Ultimaker/Cura#18092
Browse files Browse the repository at this point in the history
  • Loading branch information
saumyaj3 committed Jan 22, 2024
1 parent f53a5c8 commit 24fd962
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 1 deletion.
1 change: 1 addition & 0 deletions conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ def generate(self):
folder_dists.append("tests")
if self.options.enable_benchmarks:
folder_dists.append("benchmark")
folder_dists.append("stress_benchmark")

for dist_folder in folder_dists:
dist_path = path.join(self.build_folder, dist_folder)
Expand Down
5 changes: 4 additions & 1 deletion include/utils/polygon.h
Original file line number Diff line number Diff line change
Expand Up @@ -1552,7 +1552,10 @@ class PolygonsPart : public Polygons
}
ConstPolygonRef outerPolygon() const
{
return paths[0];
if (! paths.empty())
{
return paths[0];
}

This comment has been minimized.

Copy link
@casperlamboo

casperlamboo Jan 22, 2024

Contributor

Requesting the outer polygon for an empty polygon is technically is technically an undefined state.

There are three solutions, all with their own downsides/upsides.

  1. Just let it crash. we have to be conscious when using outerPolygon, that we only call the function if we know that the polygon in is non empty.
  2. Changing the return type to std::optional<ConstPolygonRef>. This is the "cleanest" solution from a code base perspective but will be a bit invasive to change through out the code.
  3. Returning an empty polygon in the case when there are no polygons. Downside is that we add an if statement to a function which is called quite often (downgrade in performance but don't think it will be too impactful). However, and I think this is a bigger critique, it is masking "other errors". Empty polygons should be discarded/filtered out way before processing the polygon.

I'm not 100% settled on the best solution

This comment has been minimized.

Copy link
@casperlamboo

casperlamboo Jan 22, 2024

Contributor

@saumyaj3 what do you think?

This comment has been minimized.

Copy link
@saumyaj3

saumyaj3 Jan 24, 2024

Author Contributor

Reverting it ATM. have a ticket in sprint to fix it properly

}

/*!
Expand Down

0 comments on commit 24fd962

Please sign in to comment.