godotengine / tps-demo

Godot Third Person Shooter with high quality assets and lighting
Other
1.01k stars 180 forks source link

Improve rendering performance #111

Closed Calinou closed 2 years ago

Calinou commented 2 years ago

Don't merge before 3.4 is released, as this relies on occluder spheres and other 3.4-only features.

The GI setting was removed as it's no longer relevant.

Disabling shadows for non-essential lights and adding occluder spheres brings down the number of draw calls from 10k-15k to 2k-6k. (Most of the gains are from disabling shadows.)

Performance

CPU: Intel Core i7-6700K GPU: GeForce GTX 1080 (NVIDIA 470.74) OS: Fedora 34

Performance measured at start area (after starting a game and waiting a few seconds):

Settings Before After
High settings* 60 FPS (16.7 mspf) 118 FPS (8.5 mspf)
Low settings* 135 FPS (7.4 mspf) 200 FPS (5 mspf)

High settings: 2560×1440, MSAA 4×, AO High, Glow High, GI High (if relevant) Low settings: 2560×1440, MSAA Off, AO Off, Glow Off, GI Off (if relevant)

Note that in certain areas of the level, the performance gains are even greater. With AO disabled but Glow kept to high, it's uncommon to go below 144 FPS during gameplay now. This makes for a much better experience on a 144 Hz monitor :slightly_smiling_face:

Preview

Some visual cutbacks were made, but they usually aren't noticeable unless you go looking for them.

2021-10-13_19 04 08

2021-10-13_19 04 18

2021-10-13_19 03 36

Before

2021-10-17_17 26 39

After

2021-10-17_17 27 28

aaronfranke commented 2 years ago

Replace GIProbe with BakedLightmap.

Doesn't this affect lighting on dynamic objects such as the player and the enemies?

Is there a particular reason to not allow both GIProbe and BakedLightmap which can be switched between in the settings?

too large to be committed on GitHub (which imposes a 20 MB per file limit).

The limit is actually 50 MB IIRC. The repo currently has one file that's 29.1 MB. Still, smaller is better.

Calinou commented 2 years ago

Doesn't this affect lighting on dynamic objects such as the player and the enemies?

It does, but I found a decent workaround by putting "fill lights" near dynamic objects with layers configured to not affect statix objects. It's not physically correct, but it looks good enough.

Is there a particular reason to not allow both GIProbe and BakedLightmap which can be switched between in the settings?

This would require duplicating all static meshes and hiding/showing them depending on user preference. Hiding the BakedLightmap node won't hide lightmaps on the meshes. I feel it's too much effort to be worth it, especially since GIProbe doesn't always look better in all situations.

Calinou commented 2 years ago

I pushed two changes that further improve performance slightly:

These changes don't result in any noticeable visual difference, yet they improve performance. (The Lambert diffuse mode is the default in Godot 4.0 for this reason.)

On lower-end GPUs and integrated graphics, this should make it easier to hit a 60 FPS target consistently :slightly_smiling_face:

Comparison

Settings used: 2560×1440, MSAA 4×, AO Off, Glow High

Before (with old revision of this PR)

180 FPS (5.56 mspf)

2021-10-15_16 56 35

After (with current revision of this PR)

186 FPS (5.38 mspf)

2021-10-15_17 04 22

aaronfranke commented 2 years ago

Both the before and after screenshots in your latest posts are massively brighter than the current version (and, full disclaimer, I think this looks significantly worse). I think that the current aesthetic of the demo is good and shouldn't be massively changed.

Significantly changing the appearance of the demo also really shouldn't be tied to improving performance. If we have to make the demo look significantly worse to improve performance, is that actually an improvement?

Please keep in mind that this demo's purpose is to showcase that Godot can do fancy graphics. If the aesthetic changes don't stand up on their own, then the performance improvements simply do not matter.

aaronfranke commented 2 years ago

I can't reproduce the visuals in your screenshot when building the latest 3.x. Here is an ImgSli of what I see with the current version versus what I see in this PR: https://imgsli.com/NzcxNDc

Calinou commented 2 years ago

Both the before and after screenshots in your latest posts are massively brighter than the current version (and, full disclaimer, I think this looks significantly worse). I think that the current aesthetic of the demo is good and shouldn't be massively changed.

I reduced the BakedLightmapData energy from 6 to 3, please check it again.

Note that in general, I found the demo's current appearance to be too dark. It made it difficult to make out details that are away from light, and generally makes for poor readability (especially if you're not in a dark environment yourself). This will also be exacerbated by https://github.com/godotengine/tps-demo/pull/106 if it's merged, since ACES tonemapping often looks darker and more contrasted.

Large areas of darkness also don't look great on monitors with a poor contrast ratio, so they should be avoided for aesthetic reasons as well.

I can't reproduce the visuals in your screenshot when building the latest 3.x. Here is an ImgSli of what I see with the current version versus what I see in this PR: imgsli.com/NzcxNDc

I made sure to commit the new .mesh and UV2 unwrap cache files – maybe my local .import/ folder has obsolete data that is still being taken into account by Godot?

Edit: I removed my .import/ folder then reimported everything. The demo's appearance is still correct on 3.4.beta.custom_build.f6784e18d.

aaronfranke commented 2 years ago

Here is what I see after building the latest 3.x, clearing .import/, and opening this PR: https://imgsli.com/NzcyNjk

I can reproduce the bright ground, but it still looks significantly worse.

TuxTheAstronaut commented 2 years ago

i think everything is too bright and seems artificial also i don't think this demo is meant to be the most performant its supposed to show how realistic you can make graphics look in godot

SeaFishelle commented 2 years ago

What's the purpose behind making the visual demo more performant? Isn't the whole point to show the graphical capabilities of the engine? Why actively make the demo look worse to improve its performance? That seems kind of backwards to me. Especially since it's just a demo, not a template or asset pack or anything like that.

And if the argument is so that users on lower-end hardware can see the demo... it's a visual demo. They can watch videos and get the same point across.

Calinou commented 2 years ago

Here is what I see after building the latest 3.x, clearing .import/, and opening this PR: imgsli.com/NzcyNjk

I can reproduce the bright ground, but it still looks significantly worse.

I can see that an added bonus of not relying on GIProbe for reflections is that specular aliasing is lessened. Which I guess is a good thing :slightly_smiling_face:

(Since we don't have temporal antialiasing yet, reducing the sources of specular aliasing is all we can do for now.)

i think everything is too bright and seems artificial also i don't think this demo is meant to be the most performant its supposed to show how realistic you can make graphics look in godot

This demo was never about creating realistic visuals. In fact, the current tonemapping in use doesn't behave in a realistic manner for highlights. (https://github.com/godotengine/tps-demo/pull/106 resolves this.) Creating a demo which aims for realism is an interesting idea, but it's not what's being done here.

It's clear that performance was not a focus when the TPS demo was initially designed, but I believe this backfired in the long run. Game engines are all about balancing visuals with performance, since one is useless without the other.

I can make the lightmap darker a second time, but I don't want to replicate the original issue this demo had when it comes to screen brightness.

What's the purpose behind making the visual demo more performant? Isn't the whole point to show the graphical capabilities of the engine? Why actively make the demo look worse to improve its performance? That seems kind of backwards to me. Especially since it's just a demo, not a template or asset pack or anything like that.

And if the argument is so that users on lower-end hardware can see the demo... it's a visual demo. They can watch videos and get the same point across.

The TPS demo is currently not quite smooth on high-end hardware from a few years ago. A GTX 1080 is still significantly faster than the most popular GPU on Steam as of writing, which is a GTX 1060. ("Smooth" refers to keeping up 60 FPS at all times, which definitely doesn't happen with the current demo. An average of 60 FPS is not a minimum of 60 FPS.)

This gives a bad image of Godot to people who try it on mid-range hardware, and then dismiss it as a low-performance option compared to other engines. This is something I've seen happen time and time again on various community platforms (inside and outside active Godot users). Overall, I've seen more people complain about Godot's poor 3D performance rather than poor visuals.

Also, given the current GPU shortages, I think it's better to focus on making the demo fast while still looking relatively good rather than go all-out on visuals. These GPU shortages aren't going to end soon – they'll likely end after Godot 4.0 is released, maybe even 4.1.

For the record, doing visual cutbacks after releasing a trailer or initial game release is common practice. It's been done to an extreme (Ubisoft games for instance), but even popular games like Team Fortress 2 have done that. It's no longer relevant on today's hardware in TF2's case, but it very much is relevant here. I prefer following the "make it work, make it fast, make it pretty" saying (in that order) :slightly_smiling_face:

Whimfoome commented 2 years ago

I agree with the performance improvements, as it shows good optimization practices, and I will finally be able to run this demo without massive lag.

But I think the scene now is too bright, I actually prefer the darker setting, gives more atmospheric feeling.

Calinou commented 2 years ago

I updated OP to include a Before/After comparison.

Before

2021-10-17_17 26 39

After

2021-10-17_17 27 28

After (BakedLightmapData energy set to 1)

2021-10-17_17 29 16

The last screenshot is closer to the Before state. It looks good in the starting area, but it's not very appealing in the main corridor and the core:

2021-10-17_17 32 16

2021-10-17_17 32 46

2021-10-17_17 33 06

This is why I think what we have currently in this PR is the best we can do for now without compromising on performance (or heavily reworking lighting, which would take a long time to do by hand).

aaronfranke commented 2 years ago

@Calinou With the before images, the scene looks like the lighting is artificially increased for all surfaces. With these new screenshots, it's very clear that what this PR does is vastly turn down the lighting effects.

Take a look at this ImgSli: https://imgsli.com/Nzc0MTM

The dark areas in the left image now look mostly the same, but the light areas in the left image are now also dark on the right. It looks like there is nearly zero light coming out of the lights. The lighting in this pull request looks terrible regardless of the brightness setting (I think it's BakedLightmapData energy that you're adjusting).

Here's another: https://imgsli.com/Nzc0MTU

golddotasksquestions commented 2 years ago

This looks a lot worse on all accounts. I would not merge it (removing the original demo) as the original TPS demo serves the purpose to show what Godot can visually do.

Why not upload it as a separate demo? something like "TPS demo - performance optimized"? Having both would allow to keep the original TPS demo a visual showcase demo, and have this as a demo on what steps can be taken to optimize such a project for lower spec target audience.

Calinou commented 2 years ago

Why not upload it as a separate demo? something like "TPS demo - performance optimized"?

To avoid confusion, we can't have too many similar-looking demos lying around. We also can't make lightmaps optional without duplicating all static meshes, since hiding the BakedLightmap node does not hide lightmaps on static meshes.

golddotasksquestions commented 2 years ago

To avoid confusion, we can't have too many similar-looking demos lying around

I don't see this as a problem. I see it as a benefit. There are many platformer demos in the Asset Library all called "Platformer" using the same visuals to highlight different approaches. I can check out those differences and have the opportunity to learn by comparison.

If your mind is set on only having one TPS demo, I would strongly vote against merging the changes you made.

Nukiloco commented 2 years ago

Looking at the after images, the entire quality seem to be much worse than it was before. I'm also going to recommend not to merge or change this at all and keep it the same. In my opinion this demo is supposed to show how good Godot is at visual stuff, lowering down the quality of it is going to make it look worse and might get some new comers not interested at all.

This gives a bad image of Godot to people who try it on mid-range hardware, and then dismiss it as a low-performance option compared to other engines. This is something I've seen happen time and time again on various community platforms (inside and outside active Godot users). Overall, I've seen more people complain about Godot's poor 3D performance rather than poor visuals.

The engine in this case is what needs to be optimized here. Lowering down the quality of the game will make the situation even worse than it is before and doesn't fix the overall problem that your explaining here.

In my opinion and I think that we as a community, should take step back and look at the whole graphical part of the engine to see what can be optimized and fixed before making any changes to the demo itself. When I think of optimizations, I don't think of major graphical stuff that shows the engine's potential being cut just to fix a problem.

Nukiloco commented 2 years ago

And even if those engine optimizations aren't enough, I think a better solution is to bring in graphical options into the demo to allow the user to toggle between high and low graphics for computers with low end or high end specs.

Jamsers commented 2 years ago

Why does switching from GI Probe to Baked Lightmap reduce quality so much? Shouldn't Baked Lightmap actually produce higher quality results, albeit at a bigger cost to storage? I've tried it myself with my own copy of the TP demo and it does produce flat, bad lighting

Calinou commented 2 years ago

Why does switching from GI Probe to Baked Lightmap reduce quality so much? Shouldn't Baked Lightmap actually produce higher quality results, albeit at a bigger cost to storage? I've tried it myself with my own copy of the TP demo and it does produce flat, bad lighting

Baked lightmaps in 3.x only store diffuse lighting, not specular lighting. They also don't store directional information in 3.x, although GIProbes don't do that either for indirect lighting.

Baked lightmaps are also much less prone to leaks, but GIProbe leaking is barely noticeable in the TPS demo since it's an indoor scene.