"normalize" and "offset" now working

Other changes: - added preliminary documentation for "offset" functionality - added tests for "offset" functionality - de-duplicated test images
Rouslan · Oct 2, 2023 · 87c18cf · 87c18cf
1 parent 788b6c6
commit 87c18cf
Show file tree

Hide file tree

Showing 25 changed files with 276 additions and 150 deletions.
diff --git a/README.md b/README.md
@@ -12,6 +12,13 @@ mature library, I recommend
 [Documentation](https://rouslan.github.io/polyops/cpp/index.html) |
 [Python Binding](https://github.com/Rouslan/polyops/tree/master/py)
 
+When splitting lines in integer coordinates, the new points need to be rounded
+to integer coordinates. This rounding causes the lines to be offset slightly,
+which can cause lines to intersect where they previously did not intersect.
+Unlike other polygon clipping libraries, polyops will catch 100% of these new
+intersections, thus the output is guaranteed not to have any lines that cross
+(lines may still touch at endpoints or overlap completely).
+
 Basic usage:
 ```c++
 using namespace poly_ops;

diff --git a/doc/CMakeLists.txt b/doc/CMakeLists.txt
@@ -18,6 +18,7 @@ if(Python3_FOUND)
             cpp/clip.rst
             cpp/large_ints.rst
             cpp/index.rst
+            cpp/offset.rst
             py/index.rst
             py/polyops.rst
         VERBATIM)

diff --git a/doc/cpp/index.rst b/doc/cpp/index.rst
@@ -230,4 +230,5 @@ Reference
 .. toctree::
     base
     clip
+    offset
     large_ints
diff --git a/doc/cpp/offset.rst b/doc/cpp/offset.rst
@@ -0,0 +1,41 @@
+poly_ops/offset.hpp
+=====================
+
+.. cpp:namespace:: poly_ops
+
+
+Functions
+----------------
+
+.. cpp:function:: template<typename Coord,typename Index=std::size_t,typename Input>\
+    void add_offset_loops(\
+        clipper<Coord,Index> &n,\
+        Input &&input,\
+        bool_set set,\
+        real_coord_t<Coord> magnitude,\
+        std::type_identity_t<Coord> arc_step_size)
+
+.. cpp:function:: template<typename Coord,typename Index=std::size_t,typename Input>\
+    void add_offset_loops_subject(\
+        clipper<Coord,Index> &n,\
+        Input &&input,\
+        real_coord_t<Coord> magnitude,\
+        std::type_identity_t<Coord> arc_step_size)
+
+.. cpp:function:: template<typename Coord,typename Index=std::size_t,typename Input>\
+    void add_offset_loops_clip(\
+        clipper<Coord,Index> &n,\
+        Input &&input,\
+        real_coord_t<Coord> magnitude,\
+        std::type_identity_t<Coord> arc_step_size)
+
+.. cpp:function:: template<bool TreeOut,typename Coord,typename Index=std::size_t,typename Input>\
+    std::conditional_t<TreeOut,\
+        temp_polygon_tree_range<Coord,Index>,\
+        temp_polygon_range<Coord,Index>>\
+    offset(\
+        Input &&input,\
+        real_coord_t<Coord> magnitude,\
+        Coord arc_step_size,\
+        point_tracker<Index> *pt=nullptr,\
+        std::pmr::memory_resource *contig_mem=nullptr)
diff --git a/include/poly_ops/clip.hpp b/include/poly_ops/clip.hpp
@@ -794,7 +794,6 @@ template<typename Index,typename Coord> class line_balance {
             }
             return line_state_t::discard;
         case bool_op::normalize:
-            if(w < 0) w = -1 - w;
             return w % 2 == 0 ? line_state_t::keep : line_state_t::keep_rev;
         }
 

diff --git a/include/poly_ops/offset.hpp b/include/poly_ops/offset.hpp
@@ -162,67 +162,86 @@ void add_offset_point(
 
 } // namespace detail
 
-template<typename Coord,typename Index=std::size_t,point_range<Coord> R>
-void add_offset_loop(
+template<typename Coord,typename Index=std::size_t,typename Input>
+void add_offset_loops(
     clipper<Coord,Index> &n,
-    R &&input,
+    Input &&input,
     bool_set set,
     real_coord_t<Coord> magnitude,
     std::type_identity_t<Coord> arc_step_size)
 {
-    auto sink = n.add_loop(set);
-    Index orig_i = sink.last_orig_i();
-    auto itr = std::ranges::begin(input);
-    auto end = std::ranges::end(input);
-    if(itr == end) return;
-
-    point_t<Coord> prev2(*itr++);
-    point_t<Coord> first = prev2;
-
-    if(itr == end) {
-        /* A size of one can be handled as a special case. The result is a
-        circle around the single point. For now, we just ignore such "loops". */
-        sink.last_orig_i() = orig_i + 1;
-        return;
-    }
+    static_assert(point_range_or_range_range<Input,Coord>);
+    static_assert(coordinate<Coord>);
+    static_assert(std::integral<Index>);
+
+    if constexpr(point_range_range<Input,Coord>) {
+        for(auto &&loop: input) add_offset_loops(n,std::forward<decltype(loop)>(loop),set,magnitude,arc_step_size);
+    } else {
+        auto sink = n.add_loop(set);
+        Index orig_i = sink.last_orig_i();
+        auto itr = std::ranges::begin(input);
+        auto end = std::ranges::end(input);
+        if(itr == end) return;
+
+        point_t<Coord> prev2(*itr++);
+        point_t<Coord> first = prev2;
+
+        if(itr == end) {
+            /* A size of one can be handled as a special case. The result is a
+            circle around the single point, but for now, we just ignore such
+            "loops". */
+            sink.last_orig_i() = orig_i + 1;
+            return;
+        }
 
-    point_t<Coord> prev1(*itr++);
+        point_t<Coord> prev1(*itr++);
+        point_t<Coord> second = prev1;
 
-    if(itr == end) {
-        detail::add_offset_point(sink,magnitude,arc_step_size,orig_i,prev2,prev1,prev2);
-        detail::add_offset_point(sink,magnitude,arc_step_size,orig_i,prev1,prev2,prev1);
-        return;
-    }
+        if(itr == end) {
+            detail::add_offset_point(sink,magnitude,arc_step_size,orig_i,prev2,prev1,prev2);
+            detail::add_offset_point(sink,magnitude,arc_step_size,orig_i,prev1,prev2,prev1);
+            return;
+        }
 
-    for(auto &&p : std::ranges::subrange(itr,end)) {
-        detail::add_offset_point(sink,magnitude,arc_step_size,orig_i,prev2,prev1,p);
-        prev2 = prev1;
-        prev1 = p;
+        for(auto &&p : std::ranges::subrange(itr,end)) {
+            detail::add_offset_point(sink,magnitude,arc_step_size,orig_i,prev2,prev1,p);
+            prev2 = prev1;
+            prev1 = p;
+        }
+        detail::add_offset_point(sink,magnitude,arc_step_size,orig_i,prev2,prev1,first);
+        detail::add_offset_point(sink,magnitude,arc_step_size,orig_i,prev1,first,second);
     }
-    detail::add_offset_point(sink,magnitude,arc_step_size,orig_i,prev2,prev1,first);
 }
 
-template<typename Coord,typename Index=std::size_t,point_range<Coord> R>
-void add_offset_loop_subject(
+template<typename Coord,typename Index=std::size_t,typename Input>
+void add_offset_loops_subject(
     clipper<Coord,Index> &n,
-    R &&input,
+    Input &&input,
     real_coord_t<Coord> magnitude,
     std::type_identity_t<Coord> arc_step_size)
 {
-    add_offset_loop(n,std::forward<R>(input),bool_set::subject,magnitude,arc_step_size);
+    static_assert(point_range_or_range_range<Input,Coord>);
+    static_assert(coordinate<Coord>);
+    static_assert(std::integral<Index>);
+
+    add_offset_loops(n,std::forward<Input>(input),bool_set::subject,magnitude,arc_step_size);
 }
 
-template<typename Coord,typename Index=std::size_t,point_range<Coord> R>
-void add_offset_loop_clip(
+template<typename Coord,typename Index=std::size_t,typename Input>
+void add_offset_loops_clip(
     clipper<Coord,Index> &n,
-    R &&input,
+    Input &&input,
     real_coord_t<Coord> magnitude,
     std::type_identity_t<Coord> arc_step_size)
 {
-    add_offset_loop(n,std::forward<R>(input),bool_set::clip,magnitude,arc_step_size);
+    static_assert(point_range_or_range_range<Input,Coord>);
+    static_assert(coordinate<Coord>);
+    static_assert(std::integral<Index>);
+
+    add_offset_loops(n,std::forward<Input>(input),bool_set::clip,magnitude,arc_step_size);
 }
 
-template<bool TreeOut,coordinate Coord,std::integral Index=std::size_t,point_range_range<Coord> Input>
+template<bool TreeOut,typename Coord,typename Index=std::size_t,typename Input>
 std::conditional_t<TreeOut,
     temp_polygon_tree_range<Coord,Index>,
     temp_polygon_range<Coord,Index>>
@@ -233,8 +252,12 @@ offset(
     point_tracker<Index> *pt=nullptr,
     std::pmr::memory_resource *contig_mem=nullptr)
 {
+    static_assert(point_range_or_range_range<Input,Coord>);
+    static_assert(coordinate<Coord>);
+    static_assert(std::integral<Index>);
+
     clipper<Coord,Index> n{pt,contig_mem};
-    for(auto &&loop : input) add_offset_loop(n,std::forward<decltype(loop)>(loop),bool_set::subject,magnitude,arc_step_size);
+    add_offset_loops(n,std::forward<Input>(input),bool_set::subject,magnitude,arc_step_size);
     return std::move(n).template execute<TreeOut>(bool_op::union_);
 }
 

diff --git a/include/poly_ops/polydraw.hpp b/include/poly_ops/polydraw.hpp
@@ -1,4 +1,4 @@
-/* A basic polygon rasterizer */
+/* A very basic polygon rasterizer */
 
 #ifndef polydraw_hpp
 #define polydraw_hpp
@@ -657,7 +657,7 @@ template<typename Coord> struct scan_line_sweep_state {
                         winding += s_itr->value.a_is_main(rast.lpoints) ? 1 : -1;
 
                         Coord x_start = x;
-                        x = to_coord(x_intercept(s_itr->value.pa,s_itr->value.pb,y),0,static_cast<Coord>(width));
+                        x = to_coord(x_intercept(s_itr->value.pa,s_itr->value.pb,y),0,static_cast<Coord>(width-1));
 
                         if(prev_winding) {
                             sc = {static_cast<unsigned int>(x),static_cast<unsigned int>(x_start),static_cast<unsigned int>(y),prev_winding};

diff --git a/py/README.md b/py/README.md
@@ -9,6 +9,13 @@ interface may change drastically between versions.
 [Documentation](https://rouslan.github.io/polyops/py/index.html) |
 [C++ Version](https://github.com/Rouslan/polyops)
 
+When splitting lines in integer coordinates, the new points need to be rounded
+to integer coordinates. This rounding causes the lines to be offset slightly,
+which can cause lines to intersect where they previously did not intersect.
+Unlike other polygon clipping libraries, polyops will catch 100% of these new
+intersections, thus the output is guaranteed not to have any lines that cross
+(lines may still touch at endpoints or overlap completely).
+
 Basic usage:
 ```python
 loop_a = [

diff --git a/py/tests/btest.py b/py/tests/btest.py
@@ -14,12 +14,12 @@
 # Technically, comments can appear anywhere, including inside 'tokens', not just
 # around tokens, but we only need to open our own files, so this is good enough.
 # The 'P4' at the start is omitted because it is checked separately.
-RE_PBM_HEADER = re.compile(b"""
-    (?:#.*[\n\r])*\\s(?:#.*[\n\r]|\\s)*
-    (\\d+)
-    (?:#.*[\n\r])*\\s(?:#.*[\n\r]|\\s)*
-    (\\d+)
-    (?:#.*[\n\r])*\\s""",re.VERBOSE)
+RE_PBM_HEADER = re.compile(
+    br"(?:#.*[\n\r])*\s(?:#.*[\n\r]|\s)*"
+    + br"(\d+)"
+    + br"(?:#.*[\n\r])*\s(?:#.*[\n\r]|\s)*"
+    + br"(\d+)"
+    + br"(?:#.*[\n\r])*\s")
 
 
 test_data_files = importlib.resources.files(test_data)
@@ -64,7 +64,7 @@ def has_square(data):
 class TestCase:
     def __init__(self):
         self.set_data = ([],[])
-        self.op_files = [None] * len(polyops.BoolOp)
+        self.op_files = [{} for i in range(len(polyops.BoolOp))]
 
 class ParseError(Exception):
     pass
@@ -82,7 +82,7 @@ def parse_tests():
 
     re_array_assign_start = re.compile(r'[ \t]*:[ \t]*\[')
     re_array_assign_end = re.compile(r'([0-9 +\-]*)(\])[ \t\n\r]*(?:$|#)')
-    re_op_assign = re.compile(r'[ \t]*:[ \t]*([0-9a-zA-Z]+)[ \t\n\r]*(?:$|#)')
+    re_op_assign = re.compile(r'([+-][0-9]+|)[ \t]*:[ \t]*([0-9a-zA-Z]+)[ \t\n\r]*(?:$|#)')
 
     def end_array_assign(m,lnum):
         nonlocal touched, partial_line
@@ -114,7 +114,7 @@ def end_array_assign(m,lnum):
                             m = re_op_assign.match(line,len(op.name))
                             if not m:
                                 raise ParseError(i)
-                            tests[-1].op_files[op] = m[1]
+                            tests[-1].op_files[op][int(m[1],10) if m[1] else 0] = m[2]
                             touched = True
                             break
                     else:
@@ -145,32 +145,33 @@ def setUpClass(cls):
         cls.clip = polyops.Clipper()
         cls.rast = polydraw.Rasterizer()
 
-    def _run_op(self,op):
+    def _run_op(self,op,nonzero_offset=False):
         for item in self.data:
-            if item.op_files[op] is None: continue
-
-            file_id = item.op_files[op]
-
-            with self.subTest(file=file_id):
-                imgfilename = file_id + '.pbm'
-                target_img = read_pbm(imgfilename,(test_data_files/imgfilename).read_bytes())
-                test_img = np.zeros_like(target_img)
-
-                self.clip.add_loops_subject(item.set_data[polyops.BoolSet.subject])
-                self.clip.add_loops_clip(item.set_data[polyops.BoolSet.clip])
-
-                self.rast.reset()
-                self.rast.add_loops(self.clip.execute_flat(op))
-                for x1,x2,y,wind in self.rast.scan_lines(test_img.shape[1],test_img.shape[0]):
-                    if wind > 0:
-                        test_img[y,x1:x2+1] = 1
-
-                diff = test_img != target_img
-                if has_square(diff):
-                    if DUMP_FAILURE_DIFF:
-                        write_pbm(file_id + "_mine.pbm",test_img)
-                        write_pbm(file_id + "_diff.pbm",diff)
-                    self.fail('output does not match file')
+            for offset,file_id in item.op_files[op].items():
+                if (offset == 0) == nonzero_offset: continue
+                if offset:
+                    raise Exception('not implemented yet')
+
+                with self.subTest(file=file_id):
+                    imgfilename = file_id + '.pbm'
+                    target_img = read_pbm(imgfilename,(test_data_files/imgfilename).read_bytes())
+                    test_img = np.zeros_like(target_img)
+
+                    self.clip.add_loops_subject(item.set_data[polyops.BoolSet.subject])
+                    self.clip.add_loops_clip(item.set_data[polyops.BoolSet.clip])
+
+                    self.rast.reset()
+                    self.rast.add_loops(self.clip.execute_flat(op))
+                    for x1,x2,y,wind in self.rast.scan_lines(test_img.shape[1],test_img.shape[0]):
+                        if wind > 0:
+                            test_img[y,x1:x2+1] = 1
+
+                    diff = test_img != target_img
+                    if has_square(diff):
+                        if DUMP_FAILURE_DIFF:
+                            write_pbm(file_id + "_mine.pbm",test_img)
+                            write_pbm(file_id + "_diff.pbm",diff)
+                        self.fail('output does not match file')
 
     def test_union(self):
         self._run_op(polyops.BoolOp.union)
@@ -184,9 +185,8 @@ def test_xor(self):
     def test_difference(self):
         self._run_op(polyops.BoolOp.difference)
 
-    # this doesn't work yet
-    #def test_normalize(self):
-    #    self._run_op(polyops.BoolOp.normalize)
+    def test_normalize(self):
+        self._run_op(polyops.BoolOp.normalize)
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -59,9 +59,8 @@ add_test(NAME UnionBitmapTest COMMAND btest ${test_input_file} union)
 add_test(NAME IntersectionBitmapTest COMMAND btest ${test_input_file} intersection)
 add_test(NAME XorBitmapTest COMMAND btest ${test_input_file} xor)
 add_test(NAME DifferenceBitmapTest COMMAND btest ${test_input_file} difference)
-
-# doesn't work yet
-#add_test(NAME NormalizeBitmapTest COMMAND btest ${test_input_file} normalize)
+add_test(NAME NormalizeBitmapTest COMMAND btest ${test_input_file} normalize)
+add_test(NAME OffsetBitmapTest COMMAND btest ${test_input_file} offset)
 
 add_executable(whitebox white_box.cpp)
 add_test(NAME WhiteBox COMMAND whitebox -t2000 -n50000 -p50)