Page MenuHomeWildfire Games

Add shadow map quality
ClosedPublic

Authored by vladislavbelov on Jul 14 2017, 10:27 AM.

Details

Reviewers
elexis
wraitii
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP20001: Shadow map quality option.
Trac Tickets
#4351
Summary

Adds a dropbox to select a shadow quality. It gives shadow map resolution, multiplied by two's degree.

Test Plan
  1. Run game without shadow filtering
  2. Patch game
  3. Run game without shadow filtering again on different quality values
  4. Compare results

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Vulcan added a subscriber: Vulcan.Jul 14 2017, 11:14 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1746/ for more details.

elexis edited edge metadata.Jul 14 2017, 2:09 PM

https://trac.wildfiregames.com/ticket/4351#comment:12 (slider?)
https://trac.wildfiregames.com/ticket/4351#comment:13 (formula instead of switch?)

I expect players to set settings to maximum quality by default while this feature drains the FPS significantly. I hope that won't be a problem (players reporting our code to be bad when they chose too high settings).

Calling RecreateTexture for all actors when changing the settings would take too long?

binaries/data/mods/public/gui/options/options.json
169 ↗(On Diff #2909)

Only a match restart is required

In D745#29266, @elexis wrote:

Yeah, it could be good.

https://trac.wildfiregames.com/ticket/4351#comment:13 (formula instead of switch?)

Will it be more readable?

I expect players to set settings to maximum quality by default while this feature drains the FPS significantly. I hope that won't be a problem (players reporting our code to be bad when they chose too high settings).

I think medium is enough while, because it's the same as now.

Calling RecreateTexture for all actors when changing the settings would take too long?

No need to do so, I'll add dynamic changing in the next patch.

vladislavbelov added inline comments.Jul 14 2017, 2:43 PM
binaries/data/mods/public/gui/options/options.json
169 ↗(On Diff #2909)

Why other options have to restart a game?

elexis added inline comments.Aug 13 2017, 10:12 PM
binaries/data/mods/public/gui/options/options.json
169 ↗(On Diff #2909)

Depends on the option. It's ok to not rebuild all shadowmaps here, but we shouldn't shout in the string and it only requires a match restart, not a game restart as far as I can see (perhaps that difference is negligible)

fabio added a subscriber: fabio.Aug 14 2017, 10:31 AM

Is it just me that I found the low quality with its blurred edges more realistic? Maybe very low it is too much blurred, eventually something between the two extremes.

Depends on the clouds I guess. We can see sharp shadows in reality too depending on the situation.

(Just for the information, this patch was also for the cinematic a22 trailer scenes (the ones with the weird camera angle, black sky and horizontal bars): https://www.youtube.com/watch?v=xQI8ce43vXk )

elexis accepted this revision.Aug 19 2017, 2:44 PM

With -2 I get 70 FPS on anatolian plateau,
With +1 it drops to 25 fps (my graphics card is extremely crappy).
With one instance running with -2 and trying to launch another with +2 freezes the second instance. So red color might be useful for that tooltip.

binaries/data/config/default.cfg
72 ↗(On Diff #2909)

This should get a comment so people know what they can set here. Just reusing that comment from C++

binaries/data/mods/public/gui/options/options.json
169 ↗(On Diff #2909)

High values on low computers may lead to out of video memory.

The english should be improved before committing.
low computers sounds like desktop towers standing on the floor :P

(Was wondering about the terms http://www.differencebtw.com/difference-between-graphics-card-and-video-card/ but I regret to have checked for it)

177 ↗(On Diff #2909)

They totally don't fit the dropdown width. But that's not the only dropdown affected. (Guess I can increae the size of that dropdown very soon)

source/renderer/ShadowMap.cpp
99 ↗(On Diff #2909)

Ultra -> Very High, as seen in the JSON file

(using the word exponent would be nice, but seems to overcomplicate the comment)
(the filename contains Shadow map, so everything is about the Shadow map, so maybe no need to mention "shadow map" explicitly).

101 ↗(On Diff #2909)

Should be grouped with the variables (don't cut off the functions).
Should be grouped with Width, Height as they are the most related ones.

127 ↗(On Diff #2909)

Unneeded comment. Group.

421 ↗(On Diff #2909)

Not sure why we have both Width and Height if we know that the texture has to be square. The duplicate can be nuke somewhere else though.

This revision is now accepted and ready to land.Aug 19 2017, 2:44 PM
This revision was automatically updated to reflect the committed changes.