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

Metal Port Design Proposal. #580

Merged
merged 14 commits into from
Jan 11, 2023
Merged

Conversation

sjg-wdw
Copy link
Collaborator

@sjg-wdw sjg-wdw commented Dec 9, 2022

This is an early version of the Metal Port design proposal, a follow on to the Rendering Modularization proposal.
We'll be completing this tomorrow, with figures, and then taking feedback.

@sjg-wdw sjg-wdw marked this pull request as draft December 9, 2022 00:30
@sjg-wdw sjg-wdw changed the title First cut at Metal Port Design Proposal. Metal Port Design Proposal. Dec 9, 2022
@JannikGM
Copy link
Contributor

JannikGM commented Dec 9, 2022

Thanks for the proposal!


I did a first review pass over the proposal.

Something that was missing is discussing known issues (or reports about potential improvements) in the old renderer and what other features Metal might have that facilitate new algorithms for map rendering.

For example, I brought up that deck.gl does clipping using clip-planes. This could potentially massively reduce the amount of draw calls; looking at the Metal Shading Language specification, there appears to be clip_distance so this could be done using hardware features.

Another thing I've mentioned is that at least maplibre-gl-js seems to prefer GL_TRIANGLES over (degenerated) GL_TRIANGLE_STRIP; this is a problem because some hardware will generate the strip index buffers in memory / fallback to a slower path. Vertex caching also is less efficient. This goes as far as into the earcut algorithm

So during the renderer modularization it probably makes sense to optimize these algorithms as well so Metal specific features or implementation details can be respected.


Edit:

There's also a lack of information around what the optimization target is. Some people use maplibre for static 2D top-down maps, whereas other users want to use it for 3D navigation at 60Hz. Others will dynamically update features at 60Hz but keep the camera static. Optimization / design will vastly differ between these 3 cases.

@sjg-wdw
Copy link
Collaborator Author

sjg-wdw commented Dec 9, 2022

Something that was missing is discussing known issues (or reports about potential improvements) in the old renderer and what other features Metal might have that facilitate new algorithms for map rendering.

For Metal I think the basic issue is just getting it addressed. The Modularization proposal has some of this, but again we're trying not to go too far off the path. I'm not sure how much appetite there is for work that doesn't directly address the deprecation problem.

For example, I brought up that deck.gl does clipping using clip-planes. This could potentially massively reduce the amount of draw calls; looking at the Metal Shading Language specification, there appears to be clip_distance so this could be done using hardware features.

My attitude with these things is more "If you don't want to see it, then don't load it" but there are workloads where this would be great. You should be able to try things like that!

So during the renderer modularization it probably makes sense to optimize these algorithms as well so Metal specific features or implementation details can be respected.

I'm a fan of helper classes for this sort of thing. Want to use the same ordering algorithms for two of the renderer implementations? Put that logic in a helper class.

There's also a lack of information around what the optimization target is. Some people use maplibre for static 2D top-down maps, whereas other users want to use it for 3D navigation at 60Hz. Others will dynamically update features at 60Hz but keep the camera static. Optimization / design will vastly differ between these 3 cases.

Should get faster in Metal for all three. I'd be shocked if we were any slower, except for really old hardware (iPad Air 2 is the bane of my existence). So for this work I don't think the architecture differs all that much right now. Might in the future, though. I could see that.

design-proposals/2022-11-29-metal-port.md Outdated Show resolved Hide resolved
design-proposals/2022-11-29-metal-port.md Show resolved Hide resolved
design-proposals/2022-11-29-metal-port.md Show resolved Hide resolved
design-proposals/2022-11-29-metal-port.md Show resolved Hide resolved
design-proposals/2022-11-29-metal-port.md Show resolved Hide resolved
design-proposals/2022-11-29-metal-port.md Show resolved Hide resolved
design-proposals/2022-11-29-metal-port.md Show resolved Hide resolved
design-proposals/2022-11-29-metal-port.md Show resolved Hide resolved
Copy link
Collaborator

@mwilsnd mwilsnd left a comment

Choose a reason for hiding this comment

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

Everything looks reasonable to me, approved.

Copy link
Collaborator

@alanchenboy alanchenboy left a comment

Choose a reason for hiding this comment

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

This is a test comment

design-proposals/2022-11-29-metal-port.md Outdated Show resolved Hide resolved
design-proposals/2022-11-29-metal-port.md Show resolved Hide resolved
design-proposals/2022-11-29-metal-port.md Outdated Show resolved Hide resolved
design-proposals/2022-11-29-metal-port.md Show resolved Hide resolved
design-proposals/2022-11-29-metal-port.md Show resolved Hide resolved
Copy link
Collaborator

@alanchenboy alanchenboy left a comment

Choose a reason for hiding this comment

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

Everything looks good to me

Copy link
Member

@lseelenbinder lseelenbinder left a comment

Choose a reason for hiding this comment

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

Great work. Excited to see this planned and ready for action!

@sjg-wdw sjg-wdw marked this pull request as ready for review January 9, 2023 14:36
@birkskyum birkskyum merged commit 2c59a4a into maplibre:main Jan 11, 2023
@TimSylvester TimSylvester deleted the metal-proposal branch October 4, 2023 16:17
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.

9 participants