google / s2-geometry-library-java

Automatically exported from code.google.com/p/s2-geometry-library-java
Apache License 2.0
533 stars 230 forks source link

CellID lost when S2RegionCoverer.getCovering() #22

Closed JiZhenfei1211 closed 3 years ago

JiZhenfei1211 commented 4 years ago

Dear list,

I want to implement a function which could get all cellids within a circle of specific radius. it looks like:

    public static String getLnglatradius(String position,double radius,int accuracy) {
        double lon= Double.parseDouble(position.split(",")[0]);
        double lat= Double.parseDouble(position.split(",")[1]);
        double capHeight = (2 * S2.M_PI) * (radius / 40075017);
        S2LatLng s2LatLng= S2LatLng.fromDegrees(lat, lon);
        String cellidlist = getCapCellidList(accuracy, capHeight, s2LatLng);
        return  cellidlist;
    }
 private static String getCapCellidList(int accuracy, double capHeight, S2LatLng s2LatLng) {
        String cellidlist="";
        double v = (capHeight * capHeight) / 2;
        S2Cap cap = S2Cap.fromAxisHeight(s2LatLng.toPoint(), v );
        S2RegionCoverer coverer = new S2RegionCoverer();
        coverer.setMaxLevel(accuracy);
        coverer.setMinLevel(accuracy);
        S2CellUnion covering = coverer.getCovering(cap);
        System.out.println(covering.size());

        ArrayList<S2CellId> s2CellIds = coverer.getCovering(cap).cellIds();
        for (S2CellId s2CellId : s2CellIds) {
            long id = s2CellId.id();
            if (cellidlist.length() == 0) {
                cellidlist = cellidlist + id;
            } else {
                cellidlist = cellidlist + "," + id;
            }
        }
        return cellidlist;
    }

but when I print the cellidlist on the map, it obviously loses some cells. The following image is the result when I want to get all cellids within a circle of 5km radius, as you can see there are some cells lost around the central of the circle. (please ignore the color of the small dots) image

Is there any problems in my code or is it a bug?
Thanks a lot!

lokkju commented 4 years ago

You might display the bounds of each cellId, instead of it's center point. I think you'll see that there are various cell sizes (levels). You can avoid this feature - if you want to - by specifying the min/max cell levels that the S2RegionCoverer is allowed to return.

JiZhenfei1211 commented 4 years ago

@lokkju Thanks for reply!
I do want to keep one same level within the circle area and have set the min/max levels to be same, but it seems that it will still automatically replace four child cells with their parent as annotated by author for getCovering().

/**

  • Return a normalized cell union that covers the given region and satisfies
    • the restrictions EXCEPT for min_level() and level_mod(). These criteria
  • cannot be satisfied using a cell union because cell unions are
    • automatically normalized by replacing four child cells with their parent
    • whenever possible. (Note that the list of cell ids passed to the cell union
    • constructor does in fact satisfy all the given restrictions.) */ public S2CellUnion getCovering(S2Region region) { S2CellUnion covering = new S2CellUnion(); getCovering(region, covering); return covering; }

Is there any alternatives to avoid this feature?

lokkju commented 4 years ago

you might try:

1) getSimpleCovering 2) loop through the S2CellUnion and emit children elements for any parent element above your desired level 3) use reflection to call getCoveringInternal and get the raw S2CellIds 4) a custom S2CellUnion that doesn't normalize the results (this code isn't at all tested, just gives you an idea):

public strictfp class S2CellUnionDenormalized extends S2CellUnion {
  public void initSwap(ArrayList<S2CellId> cellIds) {
    initRawSwap(cellIds);
    // skip normalization
    // normalize();
  }
}
S2CellUnionDenormalized s2cud = new S2CellUnionDenormalized();
regionCoverer.getCovering(region, s2cud)

I'd probably suggest (1) or (2) as the best options.

JiZhenfei1211 commented 4 years ago

@lokkju Get it. Appreciate for your help!

eengle commented 3 years ago

I'd suggest #5: use denormalize, to get the cells at the particular cell level you want.

eengle commented 3 years ago

(Feel free to reopen if that doesn't work well for you)