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

Support for GPU Instancing #27

Closed
strich opened this issue Jun 11, 2018 · 7 comments
Closed

Support for GPU Instancing #27

strich opened this issue Jun 11, 2018 · 7 comments
Labels

Comments

@strich
Copy link

strich commented Jun 11, 2018

Would it be possible to support GPU instancing for the meshes at each LOD level? Or do they each have unique mesh grids? Any thoughts on challenges around this? It would greatly improve the time wasted pushing the verts to the GPU.

@huwb
Copy link
Contributor

huwb commented Jun 12, 2018

Hi there,

It should not be uploading any geometry each frame - only per-tile shader params. The geometry should be uploaded once by the OceanBuilder on startup. Unless of course there's some kind of nasty bug thats causing it to upload, let me know!

(For the record, regarding GPU instancing, if my understanding is correct this would reduce the number of drawcalls - I guess once it organises what is visible and what is not, it can draw each tile type in a single call. This would probably reduce ocean draw call count from ~40 to ~5. This is interesting but closes the door to doing additive lighting in multiple forward passes to allow local point lights affecting tiles, so I decided not to act on it.)

Let me know if that answers your question and/or if you're seeing something different.

@huwb huwb added the Question label Jun 12, 2018
@strich
Copy link
Author

strich commented Jun 12, 2018

It could be even better with uniform tiles enabled too can't it? With that all tiles are the same except for their transform scale correct?

As for the shading/lighting features - Setting up GPU instancing won't remove any features. If in forward rendering mode the base pass will still benefit from instancing and further passes will automatically fall back to standard drawcalls.

@huwb
Copy link
Contributor

huwb commented Jun 12, 2018

Yes - perhaps it could be reduced to a one draw call if all tiles were uniform. There are a couple of reasons i moved away from uniform tiles:

  • They overlap and this generates not-insignificant duplicate vertex and pixel shader work
  • I generate an extra strip of tris at the boundary perimeter of the ocean geom which extends the range of the ocean geom to the horizon. This is a simple and efficient solution.

It would be possible to solve the second point by manually generating this geom, or doing away with it completely (might work in some scenarios), or by having two tile types and instancing them separately (so 2-4 draw calls in total depending on how many boundary tile types are needed).

It would be good if it doesnt affect the lighting. The main thing in my mind is that if we have local point lights we'll probably only want each light to affect surrounding tile(s). If this still works with instancing, then I am understimating Unity's instancing :) and thats good to know.

I'm not sure what other problems might be encountered when moving to instancing. Maybe none.

I'm interested in trying this out, but I have other things on my plate that are more interesting and/or pressing right now. Feel free to comment on priority/motivate this. My (possibly wrong) assumption has been that 40 drawcalls is acceptable on console/PC (at least for something that fills a vast chunk of the screen). I'm aware however that the impact of draw calls on mobile is high, or at least it used to be.

@strich
Copy link
Author

strich commented Jun 13, 2018

My (possibly wrong) assumption has been that 40 drawcalls is acceptable on console/PC

I think that is correct, but AFAIK it is a lot of polys to push to the GPU. I'll do some testing tonight to see the impact in reality.

FYI unity will automatically disable instancing on tiles that require the additional lighting. So if the solution is implemented it should be non-destructive to other features.

@huwb
Copy link
Contributor

huwb commented Jun 14, 2018

Ok let me know. I don't foresee an issue here, the only time geometry is transferred to the GPU is on startup. At runtime it uses the existing vertex/index buffers (i just double checked with renderdoc). And FWIW its a very small amount of geometry, each tile is say 64x64 verts and i think there are < 10 tile types. In my current understanding, there should not be an issue from that point of view, but let me know if I'm overlooking something.

Good to know about the lighting, thanks!

@huwb
Copy link
Contributor

huwb commented Jun 14, 2018

I just did a perf test on my laptop with a standalone build to check overall performance to make sure nothing is going horribly wrong, the total frame time is under 3.5ms at 1080p which is where i was hoping it to be, considering I've been ramping up the detail setting lately and adding loads of optional shader features which I have all turned on. (this is with quality set to 'simple' which unlocks the vsync, but this does not make the ocean cheaper).

@huwb
Copy link
Contributor

huwb commented Jun 19, 2018

I will close this for now as I don't have plans to implement instancing in the short term. If i try it i'll post back here. Thanks again for the info!

@huwb huwb closed this as completed Jun 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants