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

[Merged by Bors] - Implement non-indexed mesh rendering #3415

Closed

Conversation

nicopap
Copy link
Contributor

@nicopap nicopap commented Dec 22, 2021

Objective

Instead of panicking when the indices field of a mesh is None, actually manage it.

This is just a question of keeping track of the vertex buffer size.

Notes

  • Relying on this change to improve performance on bevy_debug_lines using the new renderer
  • I'm still new to rendering, my only expertise with wgpu is the learn-wgpu tutorial, likely I'm overlooking something.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Dec 22, 2021
@nicopap nicopap force-pushed the implement-non-indexed-drawing branch from 11257db to 5dcdc2a Compare December 22, 2021 13:43
nicopap added a commit to nicopap/bevy_debug_lines that referenced this pull request Dec 22, 2021
With bevyengine/bevy#3415 it is now possible to upload meshes to GPU
without `indices` set. Before, it would panic. Removing `indices`
enables us to simply remove the `ImmLinesStorage::fill_indices` code,
since the ordering is exactly as the GPU would interpret the mesh
otherwise.
@nicopap nicopap force-pushed the implement-non-indexed-drawing branch from 5dcdc2a to 5e71a30 Compare December 23, 2021 09:09
@cart
Copy link
Member

cart commented Dec 23, 2021

Seems like a pretty clear win to me. Nice work!

@cart
Copy link
Member

cart commented Dec 23, 2021

bors r+

bors bot pushed a commit that referenced this pull request Dec 23, 2021
# Objective

Instead of panicking when the `indices` field of a mesh is `None`, actually manage it.

This is just a question of keeping track of the vertex buffer size.

## Notes

* Relying on this change to improve performance on [bevy_debug_lines using the new renderer](Toqozz/bevy_debug_lines#10)
* I'm still new to rendering, my only expertise with wgpu is the learn-wgpu tutorial, likely I'm overlooking something.
@bors bors bot changed the title Implement non-indexed mesh rendering [Merged by Bors] - Implement non-indexed mesh rendering Dec 23, 2021
@bors bors bot closed this Dec 23, 2021
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled labels Dec 23, 2021
nicopap added a commit to nicopap/bevy_debug_lines that referenced this pull request Dec 23, 2021
With bevyengine/bevy#3415 it is now possible to upload meshes to GPU
without `indices` set. Before, it would panic. Removing `indices`
enables us to simply remove the `ImmLinesStorage::fill_indices` code,
since the ordering is exactly as the GPU would interpret the mesh
otherwise.
Toqozz pushed a commit to Toqozz/bevy_debug_lines that referenced this pull request Jan 11, 2022
* Rework to use new bevy rendering pipeline

Regressions:
* Does not work in 2d anymore
* The depth check option is now always on
* Removed user Lines (not sure what their purpose was tbh)

Improvements:
* Does work with the new renerer
* Uses wgsl

I wrote TODOs laying out improvement paths. There are a few performance
low-hanging fruits available.

The depth check is kind of a monster. I wanted to have something out
before trying it, because I'm strongly considering a uniform for this,
so that the depth test can be modified at run time and is probably going
to tank performance.

Uploading uniforms to GPU under the new renderer requires an awful lot
of additional code, due to the nature of wgpu, but it seems completely
possible, just check the `shader_material` example in the bevy
repository.

* Separate retained and immediate lines

The persisting lines now are treated and stored separately from the
temporary ones. This simplifies the logic for the temporary ones, and
since it's the general case, it should improve performance.

* Remove indices of immediate mod line meshes

With bevyengine/bevy#3415 it is now possible to upload meshes to GPU
without `indices` set. Before, it would panic. Removing `indices`
enables us to simply remove the `ImmLinesStorage::fill_indices` code,
since the ordering is exactly as the GPU would interpret the mesh
otherwise.

* follow bevy main HEAD

* Add back depth check configuration

It is configured through the plugin constructor now. It makes more sense
than through the `DebugLines` resource, since it's configured at the
beginning and won't change in the middle of running the plugin.

This was already true before, the difference is that it is now made
explicit to the user.

* Refactor LineStorage to use line over vertex index

At the cost of making the `RetLineStorage::fill_indices` and
`mark_expired` methods a bit more complex, it is possible to only store
the line index rather than two point indices of expired lines.

This should encapsulate neatly the complex underlying storage that
mirrors the mesh attribute buffers. Other side benefit is that
`timestamp` and `expired` vectors have their size divided by two.

I also added the `Line<T>` struct to make the arguments to mutating
methods of LineStorage more understandable.

* Remove references to thickness in examples

* Refactor and document

Various name changes and extra doc string. Also fixes a likely issue
with `mark_expired`, which would add duplicates to the `expired` stack.

* Add back 2d renderning

This however, requires adding a feature flag :/

* Update bevy dependency to 0.6

* Revert to exposing DebugLines as a bevy Resource

* Fix minor spelling errors

* Do not bump already-bumped version
@nicopap nicopap deleted the implement-non-indexed-drawing branch September 5, 2023 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants