hypertidy / decido

Constrained polygon triangulation by 'ear cutting'
https://hypertidy.github.io/decido
Other
15 stars 3 forks source link

update earcut source, prep 0.3.0 release #21

Closed mdsumner closed 4 years ago

mdsumner commented 4 years ago

I'm not in a rush to do this, just lining it up. Crux is update of the earcut.h, I just copied it down from the latest commit. I will check more closely in there to compare what was fixed for CRAN before, but otherwise it's passing check.

dcooley commented 4 years ago

I've tested this PR with

library(Rcpp)

cppFunction(
  depends = "decido"
  , includes = '#include "decido/decido.hpp"'
  , code = '
    Rcpp::IntegerVector earcut( SEXP polygon ) {
      return decido::api::earcut( polygon );
    }
  '
)

poly <- list(matrix(c(0,0,0,1,1,1,1,0,0,0), ncol = 2, byrow = T))
earcut( poly )

and it compiles & runs, so from my perspective this is all good.

mdsumner commented 4 years ago

@mpadge I'm just comparing your -Wall -pedantic fixes from here:

https://github.com/hypertidy/decido/blame/master/inst/include/earcut.h

I would expect problems now that I've obtained the new Mapbox source, at least here with the trailing semicolon:

https://github.com/hypertidy/decido/blob/update-earcut.hpp/inst/include/earcut.h#L15

But, I can't trigger any warnings - what I tried was adding this in Makevars but all I get from check is a warning about this over riding user/site settings.

CFLAGS= -Wall -pedantic

Is that enough? It passes on winbuilder, but maybe won't pass on the CRAN submit debian?

mdsumner commented 4 years ago

and, of course I'll just port your changes over but I'm wondering how to approach this more systematically, and hopefully PR them back to mapbox if that's sensible :)

mdsumner commented 4 years ago

it works on Debian/rhub with '-Wall -pedantic' so I think I'll go with it.

https://builder.r-hub.io/status/decido_0.3.0.tar.gz-c0241393ea3f4223b763423b6039f3a5