Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

queryRenderedFeatures returning incorrect fill-extrusion-base and fill-extrusion-height #9632

Open
danvk opened this issue Apr 27, 2020 · 5 comments
Labels

Comments

@danvk
Copy link

danvk commented Apr 27, 2020

mapbox-gl-js version: 1.9.1

browser: Chrome

Steps to Trigger Behavior

I'm using fill extrusion layers with LineString geometries to color the sides of buildings. Before #9282, the queryRenderedFeatures would return objects with a layer.paint containing style expressions that I could evaluate to determine the base and height for each feature.

After #9282, these are evaluated to the incorrect values.

> const features = map.queryRenderedFeatures(point);
> features.length
11
> console.log(features[0])
{
  "geometry": {
    "type": "LineString",
    "coordinates": [
      [ -113.79264436662197, 40.828627564726645 ],
      [ -113.7926483899355, 40.82872346056524 ]
    ]
  },
  "type": "Feature",
  "properties": {
    "out:Bldg-ID": "6ed979b6-8d2a-4ce0-9a70-a425c3696388",
    "my-metric": 65.760886,
    "min_height": 122.5,
    "height": 136
  },
  "layer": {
    "id": "my-source-extrusion",
    "type": "fill-extrusion",
    "source": "my-source",
    "layout": {
      "visibility": null
    },
    "paint": {
      "fill-extrusion-base": 0,
      "fill-extrusion-height": 11,
      "fill-extrusion-color": {
        "r": 0.351652772012709,
        "g": -0.05106863724142475,
        "b": 0.8435576652191649,
        "a": 1
      },
      "fill-extrusion-opacity": 1,
      "fill-extrusion-vertical-gradient": false
    }
  },
  "source": "my-source",
  "state": {}
}

This is what the style for that layer looks like:

{
  "fillExtrusionPaint": {
    "fill-extrusion-base": [ "get", "min_height" ],
    "fill-extrusion-height": [ "get", "height" ],
    "fill-extrusion-color": [
      "case",
      [ "has",  "building_id" ],
      "white",
      [
        "==",
        [ "get", "my-metric" ],
        0
      ],
      "yellow",
      [
        "interpolate-hcl",
        [ "linear" ],
        [ "get", "my-metric" ],
        0,
        "blue",
        100,
        "red"
      ]
    ],
    "fill-extrusion-opacity": 1,
    "fill-extrusion-vertical-gradient": false
  }
}

The fill-extrusion-base and fill-extrusion-height properties should evaluate to 122.5 and 136, not 0 and 11.

Link to Demonstration

I can try to put together a smaller repro if it's helpful.

Expected Behavior

Actual Behavior

@arindam1993
Copy link
Contributor

@danvk We actually don'y support fill-extrusions with LineStrings only Polygon geometry type is supported. This is a hole in our documentation that needs to be addressed.

Its captured here.

//Other feature types (e.g. LineString) do not have area, so triangulation is pointless / undefined

This is a part of the code that process's fill extrusion geometry and it completely skips LineStrings The bug here is that it shows up in queryRendereredFeatures at all in the first place.

@danvk
Copy link
Author

danvk commented Apr 28, 2020

A LineString geometry does have area if it's a fill extrusion: length x height = area.

For not being supported, LineStrings with fill-extrusions work spectacularly well! I've been using them for at least a year and this is the first problem I've run into. Please don't fix the bug by removing them from the results completely!

image

@arindam1993
Copy link
Contributor

aah, I misunderstood the code there, looks like that code only skips triangulating the top "roof", but we still process the side walls for linestrings.

@ryanhamley , can you please take a look at this? seems like a regression

@ryanhamley
Copy link
Contributor

@danvk Can you create a minimal example showing this problem? Definitely looks like a bug but it's not obvious what's going wrong just looking at the output.

@danvk
Copy link
Author

danvk commented Apr 30, 2020

@ryanhamley here's a repro: https://codepen.io/danvk/pen/WNQZQGG?editors=1000

image

whichever bit of the side you click, you'll see this in the console:

image

The fill-extrusion-{base,height} properties always evaluate to 0 and 50, whereas they should vary depending on what you click. I believe the fill-extrusion-color is evaluated correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants