Skip to content

Fix mid calculation for #173.#176

Merged
mourner merged 2 commits intomapbox:mainfrom
mlptownsend:fix-173
Oct 5, 2023
Merged

Fix mid calculation for #173.#176
mourner merged 2 commits intomapbox:mainfrom
mlptownsend:fix-173

Conversation

@mlptownsend
Copy link
Copy Markdown
Contributor

Fix the mid index calculation.

image

@mourner
Copy link
Copy Markdown
Member

mourner commented Oct 5, 2023

Huh, interesting — looks like this change makes one of the tests fail. Needs investigation before we can merge then...

@mlptownsend
Copy link
Copy Markdown
Contributor Author

Yeah, it's over here.

"z6-11-23": [
    {
      "geometry": [
        [
          [
            3302,
            -64
          ],
          [
            3315,
            -40
          ],
          [
            3403,
            33
          ],
          [
            3399,
            230
          ],
          [
            3483,
            315
          ],
          [
            3634,
            326
          ],
          [
            3730,
            657
          ],
          [
            3814,
            712
          ],
          [
            3890,
            617
          ],
          [
            4117,
            623
          ],
          [
            4160,
            600
          ],
          [
            4160,
            3109
          ],
          [
            799,
            3104
          ],
          [
            799,
            1285
          ],
          [
            895,
            952
          ],
          [
            835,
            868
          ],
          [
            695,
            852
          ],
          [
            643,
            712
          ],
          [
            791,
            349
          ],
          [
            867,
            315
          ],
          [
            942,
            163
          ],
          [
            931,
            67
          ],
          [
            1014,
            -57
          ],
          [
            1016,
            -64
          ],
          [
            3302,
            -64
          ]
        ]
      ],
      "type": 3,
      "tags": {
        "name": "Idaho",
        "density": 19.15
      },
      "id": "16"
    },...

vs

"z6-11-23": [
    {
      "geometry": [
        [
          [
            3302,
            -64
          ],
          [
            3315,
            -40
          ],
          [
            3403,
            33
          ],
          [
            3399,
            230
          ],
          [
            3483,
            315
          ],
          [
            3634,
            326
          ],
          [
            3730,
            657
          ],
          [
            3814,
            712
          ],
          [
            3890,
            617
          ],
          [
            4117,
            623
          ],
          [
            4160,
            600
          ],
          [
            4160,
            3109
          ],
          [
            2972,
            3109
          ],
          [
            799,
            3104
          ],
          [
            799,
            1285
          ],
          [
            895,
            952
          ],
          [
            835,
            868
          ],
          [
            695,
            852
          ],
          [
            643,
            712
          ],
          [
            791,
            349
          ],
          [
            867,
            315
          ],
          [
            942,
            163
          ],
          [
            931,
            67
          ],
          [
            1014,
            -57
          ],
          [
            1016,
            -64
          ],
          [
            3302,
            -64
          ]
        ]
      ],
      "type": 3,
      "tags": {
        "name": "Idaho",
        "density": 19.15
      },
      "id": "16"
    },...

The new one has another point in it.

[
            2972,
            3109
          ],

That's the only difference.

@mourner
Copy link
Copy Markdown
Member

mourner commented Oct 5, 2023

The result visually looks fine to me. I guess there's a chance that choosing a different pivot might lead to a slightly different outcome, even if it's not very noticeable... Let's update the test fixture then.

@mlptownsend
Copy link
Copy Markdown
Contributor Author

Sure, can do.

Here's what the new bottom of Idaho looks like in the debug visualizer:
image
vs
image

The red line is a wee bit darker in the old one.

@mourner mourner merged commit 6eb1731 into mapbox:main Oct 5, 2023
@mlptownsend mlptownsend deleted the fix-173 branch October 10, 2023 16:42
district10 pushed a commit to cubao/headers that referenced this pull request Oct 17, 2023
district10 added a commit to cubao/headers that referenced this pull request Oct 17, 2023
Co-authored-by: TANG ZHIXIONG <zhixiong.tang@momenta.ai>
maxammann added a commit to maxammann/geojson-vt-cpp that referenced this pull request Aug 13, 2024
mourner pushed a commit to mapbox/geojson-vt-cpp that referenced this pull request Aug 13, 2024
* Fix mid point calculation

see mapbox/geojson-vt#176

* Update us-states-tiles.json from JS version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants