Re: [Octave-patch-tracker] [patch #9000] Polygon functions for geometry/mapping package

[Juan Pablo Carbajal]

On Sun, Oct 2, 2016 at 11:25 PM, Philip Nienhuis
<INVALID.NOREPLY@gnu.org> wrote:
> Follow-up Comment #23, patch #9000 (project octave):
>
> (just short)
> Most of Amr's work can be absorbed right away.
> Why don't you just download his 3.0.0 geometry tarball and play with it?
> Then later on we can work through my recommendations-to-come and you can
> choose what you'll do.
>
> Amr made several functions that were a bit outside the strict
> boolean-ops-on-polygons subject. Some of those might better live in of-mapping
> (due to Matlab's naming & ML-compatibility aim) but as a wrapper for similar
> functions in of-geometry. I can't think of a strong reason to not make
> of-geometry a "hard" dependency of of-mapping.
>
> Philip
>
>
>     _______________________________________________________
>
> Reply to this item at:
>
>   <http://savannah.gnu.org/patch/?9000>
>
> _______________________________________________
>   Message sent via/by Savannah
>   http://savannah.gnu.org/
>

Sorry for the delay.
I do not think AMR's work is so directly mergable.

I have been reviewing AMR's work. I really do not like the fact that
even the simplest ispolycw needs boost. I am trying to adjust it to
use the already existing isPolygonCCW. Currently i am getting the
following failure in tests

***** test
 x={[1 2],[3 4]}; y={[10 20],[30 40]};
 [xccw,yccw]=poly2ccw(x,y);
 xexp={[2 1],[4 3]};
 yexp={[20 10],[40 30]};
 assert (xccw,xexp);
 assert (yccw,yexp);
!!!!! test failed
ASSERT errors for:  assert (cond {i},expected {i},tol)

  Location  |  Observed  |  Expected  |  Reason
    (1)           1            2         Abs err 1 exceeds tol 0
    (2)           2            1         Abs err 1 exceeds tol 0

ASSERT errors for:  assert (cond {i},expected {i},tol)

  Location  |  Observed  |  Expected  |  Reason
    (1)           3            4         Abs err 1 exceeds tol 0
    (2)           4            3         Abs err 1 exceeds tol 0

Which I think it is due to the following.
It seems AMR redefined the idea of polygons passed as cells. In his
tests he writes things like

x={[1 2],[3 4]}; y={[10 20],[30 40]};

Why would somebody define a polygon with a cell in that way? The cells
in geometry are used to collect polygons, like this

px1 = [1 2 2 1].';
py1 = [1 1 2 2].';
px2 = [1 2 3 3 2 1].';
py2 = [1 1 2 3 4 4].';
pc1 = {[px1 py1], [px2 py2]}; # This is using the poly as matrix with
XY pairs, i.e. polyFunction (pc1)
pc2x = {px1, px2};                 # This using the polygon as two
arguments one for X coord
pc2y = {py1, py2};                 # and the other for Y coord, i.e.
polyFunction (pc2x, pc2y)

The latter is not working on all functions, but some do accept that.


So to include any of AMR's work I need to include the boost part,
which is not ideally currently. I would be ok with needing boost for
clipping, being that more complex, but for checking ccw-ness and
reordering I wouldn't.

@Philllip: Where is the lastet version of your work so I can merge it?
@Phillip: I would love to birg mapping and geometry closer to each other.

Cheers