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

Remove primitive restart #7397

Closed

Conversation

weihuoya
Copy link
Contributor

@weihuoya weihuoya commented Sep 4, 2018

Is there any reason to use such feature? it just make code complex.

@degasus
Copy link
Member

degasus commented Sep 4, 2018

It reduces the generate index buffer size. And its hard code is well separated. But you might argue about its speedup...

@stenzek
Copy link
Contributor

stenzek commented Sep 4, 2018

Is there a reason not to use it? My suspicion would be that fewer transfers across PCIe could lead to better performance on low-end systems. But I'm happy to be corrected if the numbers say it's not any slower...

@weihuoya
Copy link
Contributor Author

weihuoya commented Sep 4, 2018

the mapped gpu memory is constant size, not baseed on data size. and is it really reduces generate index buffer size? use 4 indices per triangle.

@stenzek
Copy link
Contributor

stenzek commented Sep 4, 2018

Yes, the mapping is a constant size, but it's a persistent (and potentially coherent) mapping. There is no implicit copy of the entire buffer, the only bytes copied are the ones which are written.

edit: yes, it's fewer indices for triangle lists, but for triangle strips or quads, it'll cause additional indices.

@degasus
Copy link
Member

degasus commented Sep 4, 2018

The GC GPU does not support indexed vertices. So to avoid redundant transforming & lighting, high polygon games use triangle_strips for their big meshes. This fits better to the primitive_restart path, and it was the reason to implement it many years ago.
But honestly, I was told that the strips generator with primitive restart was slower for some other developer by a small margin. So I wonder if you are able to confirm the same behavior. And if so, I wonder if the strip merger has an implementation issue or if the algorithmn is slower in general.

@weihuoya
Copy link
Contributor Author

weihuoya commented Sep 4, 2018

oh, i'm not familiar with this.:laughing:

with this changes, run stereo mode in vulkan backend, game will crash in some place, don't known where going wrong.

@weihuoya weihuoya closed this Sep 4, 2018
@weihuoya weihuoya deleted the remove-primitive-restart branch July 15, 2019 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants