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

Switch from physical to logical segmentation of buffers #3461

Merged
merged 1 commit into from
Oct 26, 2016

Conversation

jfirebaugh
Copy link
Contributor

Fixes #207.

And while here, switch to indexed rendering for collision boxes. Fixes #2038.

Physical segmentation: each vertex buffer holds a maximum of 2¹⁶-1 vertices. When a layer requires more than that, create a new vertex buffer and index buffer.

Logical segmentation: no limit to the size of a vertex buffer. When reaching 2¹⁶-1 vertices, record the vertex and primitive offsets, then reset the indexing, starting a new "segment". Use the offsets when rendering each segment.

Logical segmentation has the following advantages:

  • It uses a smaller number of larger buffer objects. In theory this is more efficient for WebGL/OpenGL. However in this case the performance difference is likely negligible. We already use VAOs, which largely eliminate the cost of switching between buffers, which leaves the cost of creating them. And buffers larger than 2¹⁶ vertices are relatively rare.
  • It's what gl-native does. Having the two codebases work the same way is an advantage, as we frequently need to port changes back and forth.
  • It simplifies populatePaintArrays. With physical segmentation, populatePaintArrays needs to track both a buffer index and an index into the first buffer, and might need to populate multiple buffer segments. With logical segmentation, it just fills the single paint vertex buffer until it's the same length as the layout vertex buffer.
benchmark master e03a7b1 logical-buffer-segmentation 59f39b7
map-load 150 ms 104 ms
style-load 100 ms 104 ms
buffer 1,030 ms 993 ms
fps 60 fps 60 fps
frame-duration 7.3 ms, 1% > 16ms 7.3 ms, 1% > 16ms
query-point 1.05 ms 1.16 ms
query-box 84.95 ms 88.68 ms
geojson-setdata-small 8 ms 10 ms
geojson-setdata-large 117 ms 99 ms

Launch Checklist

  • briefly describe the changes in this PR
  • post benchmark scores
  • manually test the debug page

Physical segmentation: each vertex buffer holds a maximum of 2¹⁶-1 vertices. When a layer requires more than that, create a new vertex buffer and index buffer.

Logical segmentation: no limit to the size of a vertex buffer. When reaching 2¹⁶-1 vertices, record the vertex and primitive offsets, then reset the indexing, starting a new "segment". Use the offsets when rendering each segment.

Logical segmentation is what native uses, and is generally simpler and more efficient.

While here, switch to indexed rendering for collision debug.
Copy link
Contributor

@lucaswoj lucaswoj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes to ArrayGroup and Bucket look 👍. I didn't read the Bucket subclasses very closely. Let me know if you'd like a more careful review of those.

@@ -15,7 +16,7 @@ const fillInterfaces = {
components: 2,
type: 'Int16'
}]),
elementArrayType: new ElementArrayType(1),
elementArrayType: new ElementArrayType(3),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new ElementArrayType(3) looks correct to me. I wonder why this was 1 before.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It worked with 1 before because FillBucket#addFeature emplaced one index at a time. Now it emplaces triangles, for consistency with FillExtrudeBucket (and native).

@jfirebaugh jfirebaugh merged commit b9d9509 into master Oct 26, 2016
@jfirebaugh jfirebaugh deleted the logical-buffer-segmentation branch October 26, 2016 00:06
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