Page MenuHomeWildfire Games

Removes unused LightingModel
ClosedPublic

Authored by vladislavbelov on Tue, Jun 25, 10:40 PM.

Details

Reviewers
elexis
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22403: Removes unused and obsolete LightingModel.
Summary

I was introduces in rP9123 as way to control shaders behaviour. Its usage in shaders was completely removed in rP11473 (before that in rP11453).

Test Plan
  1. Apply the patch and compile the game
  2. Make sure that everything is rendered as before
  3. Make sure that shaders or other code doesn't have occurrences of LIGHTING_MODEL

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

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1817/display/redirect

elexis accepted this revision.Tue, Jun 25, 11:26 PM
elexis added a subscriber: elexis.
  • No call to SetLightingModel in *.cpp, *.h, no further call to GetLightingModel and m_LightingModel
  • No occurrence of LIGHTING_MODEL in shader files

rP11453 says

Remove non-supported 'old' lighting model.

so that commit aguably forgot to remove it completely.

(rP11473 left one call there)

Observation repeated, argument accepted, I didn't test though.

source/graphics/LightEnv.h
60 ↗(On Diff #8611)

I know that above settings exist in Atlas and influence shading, although this doesn't mean that this isn't possibly duplicate dead code, I didn't check that part, you did?

Agree with moving the private parts.

64 ↗(On Diff #8611)

I don't object to removing this, since it is is unused, arguably misleading if the comment states things that don't hold, and a multi lightning model implementation might have to be revised.

This revision is now accepted and ready to land.Tue, Jun 25, 11:26 PM
source/graphics/LightEnv.h
60 ↗(On Diff #8611)

These members are used. For ex. GetRotation is used for sky.

Stan added a subscriber: Stan.Wed, Jun 26, 8:03 PM

Just tested, seems to work as expected on windows (vs2015)

This revision was automatically updated to reflect the committed changes.