[do not merge] Testing performance fixes #2
Conversation
|
First of all, thank you so much for opening this PR! These technical discussions are so helpful for me. ❤️ So I pulled this down, and wow it's handling all those vertices at 60FPS. This is really promising. I don't think I'm completely following where this performance boost is coming from, though. From what I can tell the buffer state logic hasn't changed in this PR. I might be missing something though 🤔 |
|
I think the perf boost is almost entirely from the switch to using split buffers - see the way I pass them to the building render function. Instead of attr.position (interleaved) I just pass position (the float array data). For whatever reason the interleaved buffer seemed to be slowing things down a lot. |
|
What that means is you could try using split buffers - all of them will be left unchanged except the state index buffer which needs to be updated in the loading state. |
| // stateTransitioner.updateLoadingState(latest.buildingIdxToMetadataList) | ||
| buffers.update({ positions, barys, randoms, buildings }, stateTransitioner.getStateIndexes()) | ||
| const attrs = buffers.getAttributes() | ||
| renderBuildings = createBuildingsRenderer(regl, positions, barys, randoms, stateTransitioner.getStateIndexes(), settings) |
There was a problem hiding this comment.
Ohhhhh - this is where I misread. I missed that you're passing these directly to createBuildingsRenderer and line 93 & 94 here are dead. Damn - so maybe there's a bug in the offset/stride stuff, like you said. 🤔 Gonna look into this a bit more tonight.
|
There are two things that still confuse me:
I'm going to go back in the commit history and checkout a version before I started interleaving the attributes to see how that functions. |
|
Alright, here's where I added the attribute interleaving (one of the few commits I actually bothered to give a helpful message). I ran the parent of this commit - and it also sucks on my work machine. Gonna have to dig into this more when I get home. |
|
Now I'm home looking at this again on my personal laptop and am confused as ever. So this branch, which gives up the incremental loading and splits the interleaved attributes back into separate I have no idea what to do with this, hahaha. All I can think is that the two different GPUs - a Radeon (work) and an Intel (home) - handle things differently and "prefer" the opposite branches. 🙈 Meanwhile, the branch connected to this PR - which drops the incremental loading, interleaved attributes, and the buildingState texture - runs at 60 FPS on both personal and work machines. Hahahaha @mattdesl Just curious - what hardware are you running this on? |
|
Thinking about this a bit more: I'm starting to suspect that this could play a role, given that the performance remained degraded when switching to a previous commit which did separate the attributes into different buffers but also had |
|
Got into work and tried switching Curious to know how this is running on your machine. |
|
Hmm doesn't seem to make a difference for me :\ |
|
Linking to this Twitter thread for posterity: https://twitter.com/mattdesl/status/988756029528793089 |
|
cc @mikolalysenko - you might find this issue interesting. A couple of us have looked at getting this up to 60 FPS but seem to be bound to 40-45 FPS. It's about 3.5M triangles with 11M vertices - rendered just as triangles - not a strip. If you end up taking a look at the code, let me know if anything stands out about the way I'm rendering the buildings. No worries if you can't get to it though! Thanks :-) |
Using multiple buffers instead of a single buffer seems to drastically improve frame rate when rendering a solid red city. Not sure why; maybe something wrong with the way stride or buffer sizes are set up?