Removes CPU lighting after no FFP
ClosedPublic
Actions

Authored by vladislavbelov on Jan 13 2021, 12:30 AM.

Details

Reviewers

None

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP24599: Removes CPU lighting after no FFP

Summary

FFP removed, so we don't need to spend CPU time on that.

Test Plan

Apply the patch and compile the game
Make sure that everything works as before
Check that CPU lighting doesn't still present somewhere in changed code

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Lint Skipped

Severity	Location	Code	Message
Error	source/renderer/Renderer.cpp:172	CPPCheckBear (returnDanglingLifetime)	CPPCheckBear (returnDanglingLifetime)
Error	source/renderer/Renderer.cpp:178	CPPCheckBear (returnDanglingLifetime)	CPPCheckBear (returnDanglingLifetime)
Error	source/renderer/Renderer.cpp:184	CPPCheckBear (returnDanglingLifetime)	CPPCheckBear (returnDanglingLifetime)

Unit

Unit Tests Skipped

Build Status

Buildable 14960
Build 32296: Vulcan Build	Jenkins
Build 32295: Vulcan Build (macOS)	Jenkins
Build 32294: Vulcan Build (Windows)	Jenkins

Event Timeline

vladislavbelov created this revision.Jan 13 2021, 12:30 AM

Build is green

builderr-debug-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgraphics_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui_dbg.a(precompiled.o) has no symbols
ld: warning: text-based stub file /System/Library/Frameworks//CoreAudio.framework/CoreAudio.tbd and library file /System/Library/Frameworks//CoreAudio.framework/CoreAudio are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//AudioToolbox.framework/AudioToolbox.tbd and library file /System/Library/Frameworks//AudioToolbox.framework/AudioToolbox are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//ForceFeedback.framework/ForceFeedback.tbd and library file /System/Library/Frameworks//ForceFeedba

See https://jenkins.wildfiregames.com/job/macos-differential/2816/display/redirect for more details.

Harbormaster completed remote builds in B14960: Diff 15219.Jan 13 2021, 1:07 AM

vladislavbelov requested review of this revision.Jan 13 2021, 1:07 AM

Stan added a subscriber: Stan.Jan 13 2021, 1:12 AM

Stan added inline comments.

source/renderer/HWLightingModelRenderer.cpp
101	Maybe?
source/renderer/Renderer.cpp
172	False positives ?
553	Maybe this could be way higher in the code, so we don't waste time checking everywhere

Succeeded.
No oddities found.
Grepping for cpuLighting didn't find any occurences ;)

vladislavbelov added inline comments.Jan 13 2021, 8:25 AM

source/renderer/HWLightingModelRenderer.cpp
101	`struct`, mostly as POD.
source/renderer/Renderer.cpp
172	It'd be right if it'd return `const char*`, but it returns `CStr`. So the return value will be constructed before the `buf` will be destroyed.
553	We're already checking. It's not a regression. Just a notice to validate the place after removing RenderPath::FIXED from real code.

Stan added inline comments.Jan 13 2021, 8:33 AM

source/renderer/HWLightingModelRenderer.cpp
101	What's the advantage of the struct here since there will be only one field? Maybe we should use just the raw pointer everywhere?
source/renderer/Renderer.cpp
172	Ah makes sense
553	Makes sense.