Filter out None element from vertices for clipping_polygon

[kkkkkom]

Dear @mozman , could you please review this minor merge request?

During viewport drawing in paperspace, the None element from clipped.copy() is raising errors for later processing, thus filtering them out to avoid the errors.

It produces same rendering results as compared to autodesk online viewer at https://viewer.autodesk.com/

Could you please review and see if this is a safe filtering and can be merged?

[kkkkkom]

Actually, never mind, I think this is not the correct way to handle the error.
I'll mark as draft and submit another merge request for proper handling

[kkkkkom]

Hi, @mozman , I've updated the merge request to add a more proper handling of the error case. Could you please review now?

Basically, if clipping edge and polygon edge are in parallel and overlapping, given the default absolute tolerance, is_inside() will say the point is out side of clipping edge, but edge_interaction() will return None, as it cannot find an intersection point.

In such case, the modification conservatively preserve the polygon vertex during clipping instead of None.

[mozman]

@kkkkkom Please add test cases to the test_618_clipping.py file to demonstrate exactly what this PR does.

[kkkkkom]

@kkkkkom Please add test cases to the test_618_clipping.py file to demonstrate exactly what this PR does.

Thanks for reviewing, yes, I will add testcase and fix the pipeline

[kkkkkom]

Hi, @mozman , actually, never mind, I think regarding my specific issue, there is no need for code change, but needs to provide a smaller abs_tol

However, I would like to set a smaller default TOLERANCE globally. Is there such way to do so?
From my understanding, the global TOLERANCE is compiled in cython, so we won't be able to dynamically change it during runtime.
If this is true, I think we can add something like below in construct.pyx:

def set_tolerance(tolerance):
    global TOLERANCE
    TOLERANCE = tolerance

Is that correct? If so, could you please let me know how to rebuild the cython extension so I can test the changes locally?
I was not able to figure out which script does the cython build ..

[mozman]

@kkkkkom The tolerances should be passed to the functions, if this is not possible, that's the issue to fix. I will not accept a change of the (arbitrary) global tolerance - what is best for you is maybe not ideal for the next user, there is no perfect value. Therefor all functions (should) have a tolerance argument.

This link shows how to build ezdxf from source - you need the build-tools from Microsoft on Windows (link is in the docs).

https://ezdxf.mozman.at/docs/setup.html#build-and-install-from-source

There is also a makefile.template - you need a make command on Windows (MSYS), read the comments in the template.

[kkkkkom]

Hi, @mozman , thanks for reviewing. Let me post the original issue that I encountered, so I can describe the here in a more clear way.

So the issue came from a very high level API (render a viewport in paperspace) with a deep trace like below:

    Frontend(ctx, out).draw_layout(layout, finalize=True)
  ezdxf\addons\drawing\frontend.py", line 304, in draw_layout
    self.draw_entities(
  ezdxf\addons\drawing\frontend.py", line 342, in draw_entities
    _draw_entities(self, self.ctx, entities, filter_func=filter_func)
  ezdxf\addons\drawing\frontend.py", line 1027, in _draw_entities
    _draw_viewports(frontend, viewports)
  ezdxf\addons\drawing\frontend.py", line 1055, in _draw_viewports
    frontend.draw_viewport(viewport)
  ezdxf\addons\drawing\frontend.py", line 679, in draw_viewport
    self.pipeline.draw_viewport(vp, self.ctx, self._bbox_cache)
  ezdxf\addons\drawing\pipeline.py", line 275, in draw_viewport
    self.draw_entities(
  ezdxf\addons\drawing\frontend.py", line 347, in draw_entities_callback
    _draw_entities(self, ctx, entities)
  ezdxf\addons\drawing\frontend.py", line 1024, in _draw_entities
    frontend.draw_entity(entity, properties)
  ezdxf\addons\drawing\frontend.py", line 369, in draw_entity
    draw_method(entity, properties)
  ezdxf\addons\drawing\frontend.py", line 626, in draw_hatch_entity
    self.pipeline.draw_filled_paths(ignore_text_boxes(paths), properties)
  ezdxf\addons\drawing\pipeline.py", line 405, in draw_filled_paths
    self._pipeline.draw_filled_paths(list(map(BkPath2d, paths)), properties)
  ezdxf\addons\drawing\pipeline.py", line 479, in draw_filled_paths
    paths = clipping_portal.clip_filled_paths(paths, max_sagitta)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ezdxf\tools\clipping_portal.py", line 214, in clip_filled_paths
    self.foreach_stage(do)
  ezdxf\tools\clipping_portal.py", line 126, in foreach_stage
    if not command(stage):
           ^^^^^^^^^^^^^^
  ezdxf\tools\clipping_portal.py", line 210, in do
    result.extend(stage.portal.clip_filled_paths(paths, max_sagitta))
  ezdxf\tools\clipping_portal.py", line 311, in clip_filled_paths
    for part in clipper.clip_polygon(
                ^^^^^^^^^^^^^^^^^^^^^
  ezdxf\math\clipping.py", line 201, in clip_polygon
    return self._clipping_polygon.clip_polygon(polygon)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ezdxf\math\clipping.py", line 152, in clip_polygon
    if len(vertices) > 1 and vertices[0].isclose(vertices[-1]):
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Then I did some initial debug, and figured out that the function edge_intersection() is returning None which caused the error. To easily reproduce the error, I've narrowed it down to a more lower level call. So you can try like below:

from ezdxf.math.clipping import ConvexClippingPolygon2d
from ezdxf.math import Vec2

clipper = ConvexClippingPolygon2d(
    [Vec2(8.000000000000455, 9.000000000000165), Vec2(15.000000000000887, 9.000000000000165),
     Vec2(15.000000000000887, 11.000000000000834), Vec2(8.000000000000455, 11.000000000000834)])
polygon = [Vec2(8.000000000000435, 9.000000000000169), Vec2(15.000000000000874, 9.000000000000169),
           Vec2(15.000000000000895, 11.00000000000084), Vec2(8.000000000000464, 11.00000000000084),
           Vec2(8.000000000000435, 9.000000000000377), Vec2(8.000000000000435, 9.000000000000169)]

clipper.clip_polygon(polygon)

Above should raise an error.

If we pass in a smaller abs_tol into the edge_intersection() function, it can avoid the error. But as you can see, there is a deep call stack to expose the abs_tol all the way up to the initial call of Frontend(ctx, out).draw_layout(layout, finalize=True)

Then on a second thought, I think a more proper way is just to use the consistent abs_tol for both is_inside() and edge_intersection() function (which I have already updated in this PR). Could you please let me know if this makes sense?

[mozman]

The fix for this is to change the algorithm and check for None values!

[mozman]

You should report such issues as bug report.

[kkkkkom]

Thanks, will report a bug issue.

[kkkkkom]

Actually I saw you already updated the fix, thanks!