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

Sort point features by y-axis #470

Closed
tmcw opened this issue Jun 18, 2014 · 16 comments
Closed

Sort point features by y-axis #470

tmcw opened this issue Jun 18, 2014 · 16 comments

Comments

@tmcw
Copy link
Contributor

tmcw commented Jun 18, 2014

img_0624

For overlays and possibly for built-in layer data, we need to sort by y-axis before displaying overlapping features that look like 'markers' - otherwise things look weird. This will need to be recalculated based on rotation if we want to really nail it.

@tmcw tmcw added the rendering label Jun 18, 2014
@incanus
Copy link
Contributor

incanus commented Jun 18, 2014

This needs to happen in native, too. Rules, borrowing from iOS SDK current behavior (modeled on Apple's MapKit):

  • Markers above shapes #
  • Markers ascending in z as relates to ascending in y (top of screen down) #
  • Ability to bring a selected marker forward #

This all needs to happen each frame, especially for rotation.

@tmcw
Copy link
Contributor Author

tmcw commented Jun 18, 2014

Markers above shapes

I'd assume this happens at the style level, right? Like, for a low-level interface, all that we're implementing is sorting in a single layer.

@incanus
Copy link
Contributor

incanus commented Jun 18, 2014

Ah, yes, correct. Lines and points would presumably belong to separate layers, which are then ordered in the style.

@incanus
Copy link
Contributor

incanus commented Jun 18, 2014

So in thinking about approaches to this:

  • We don't have access to gl_FragDepth in WebGL/OpenGL ES.
  • We can't (?) modify gl_FragCoord in the pixel shader.
  • We need a viewport-normalized a value based on a_pos.y for something like gl_Position = u_matrix * vec4(a_pos, mix(-0.2, -0.1, a, 1); in the vertex shader. FWIW values in the range of -0.2 to -0.1 seem to work alright with our other shaders' non-zero depth values (e.g. text and lines).

That's where I'm at so far... still trying to figure out a good approach.

@ansis
Copy link
Contributor

ansis commented Jun 19, 2014

Yep, depth buffer values can't be set from the fragment shader, so solutions using depth aren't going to work. Even if we could set the depth value per fragment I think we'd still have problems in semi-transparent areas.

Features are layered in the order they appear in the buffers, so sorting the points before adding them should work. They would need to be resorted after every rotation though.

@incanus
Copy link
Contributor

incanus commented Jun 19, 2014

Why does depth have to be set in the fragment shader? Why can't we adjust the a_pos in the vertex shader?

They would need to be resorted after every rotation though.

Isn't this going to be have to be on the CPU though?

@incanus
Copy link
Contributor

incanus commented Jun 19, 2014

@ansis
Copy link
Contributor

ansis commented Jun 19, 2014

Why does depth have to be set in the fragment shader? Why can't we adjust the a_pos in the vertex shader?

Setting the depth doesn't automatically arrange the features in the right z-order. It just allows you to write to the depth buffer and compare against the depth buffer. Setting the depth on the vertices will set the interpolated depth for each fragment. You can then later test fragments against the previously written depth value, and choose to drop the fragments if they fail some condition. You could use this to prevent drawing over higher z-order markers, but it will prevent drawing for the marker's entire box including transparent pixels.

I might be missing something big though

Isn't this going to be have to be on the CPU though?

yes

@incanus
Copy link
Contributor

incanus commented Jun 19, 2014

Got it. Yeah we don't want to drop, not just because of boxing but also alpha values in marker imagery themselves -- these should interleave smoothly.

@tmcw
Copy link
Contributor Author

tmcw commented Jun 24, 2014

@ansis what's the easiest way to get a testcase for this? should I hack debug to add some points to the geojson?

@incanus
Copy link
Contributor

incanus commented Jun 24, 2014

Now that #477 is done, one way would be to do what I've been doing over on native. Here's my diff using a vector point source:

https://gist.github.com/incanus/485f6e63528124c98f2e

@ansis
Copy link
Contributor

ansis commented Jun 24, 2014

@incanus's way seems good, but geojson points would also be fine and easier to make into a testcase.

Also, I think this should be opt-in.

@incanus
Copy link
Contributor

incanus commented Jun 24, 2014

Also, I think this should be opt-in.

Agreed. Likely defaulting to on for iOS, but we should open up the API to hit both POI (non-collide) and overlay (probably want to collide) use cases.

@ansis
Copy link
Contributor

ansis commented Apr 7, 2015

This is easy to implement with the new labeling: 5bf094f

How should this be specified in the style? spec issue here: mapbox/mapbox-gl-style-spec#274

@incanus
Copy link
Contributor

incanus commented Apr 7, 2015

This is mostly designed for annotations, so the style is hardcoded such as in https://github.com/mapbox/mapbox-gl-native/blob/0a2ee766dbe65e3d23d0e5cba79776560ff29275/src/mbgl/style/style_parser.cpp#L40-L69, then would have the ability to turn a feature like this on or off via API.

@ansis ansis closed this as completed in 6514de7 Apr 30, 2015
bensleveritt pushed a commit to bensleveritt/mapbox-gl-js that referenced this issue Oct 24, 2016
Fire draw.update on mouseout when dragging
lucaswoj pushed a commit that referenced this issue Jan 11, 2017
@ChrisLoer
Copy link
Contributor

#5150 partially reverts this behavior (by allowing sort order inconsistencies at tile boundaries) in order to fix #2706 along with other issues. See discussion at #5150 (comment).

stepankuzmin pushed a commit that referenced this issue Jun 21, 2023
#470)

* Tree shadows, casting and receiving

Basic implementation, no instancing nor frustums optimization yet.

* Update dynamic style

Still unsupported:
- icon-emissive-strength using measure-light
- model-height-based-emissive-strength-multiplier (waiting on landmarks)
- model-emissive-strength using measure-light (waiting on landmark)

* Shadow frustum culling for trees

* Remove u_cascade_distances and fade between cascades. Extend shadow map far to fill the top of tilted shadow map camera

Set dynamic style (rebase to latest dynamic.json) as default. Enable usage or original style  fill extrusions (not adding from 3d-playground)

* render tests

* setEventedParent for modelManager

* load lights from style if present

* Fix shader after rebase

* revert back end fade range behavior (was using cascade[1].far * 0.75 on GPU earlier

* shadow map extend frustum far on terrain. shadow cast and receive

terrain raster shadows use non biased ground version to prevent peter panning.
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

No branches or pull requests

5 participants