Page MenuHomeWildfire Games
Differential D971

Adds perspective matrix
ClosedPublic
Actions

Authored by vladislavbelov on Oct 21 2017, 11:43 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Sep 13, 12:09 PM
Unknown Object (File)
Mon, Sep 2, 8:16 AM
Unknown Object (File)
Mon, Sep 2, 8:16 AM
Subscribers
Stan
Vulcan
wraitii
Restricted Owners Package
Restricted Owners Package
Tokens
"Yellow Medal" token, awarded by Stan.

Details

Reviewers
wraitii
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Owners Package(Owns No Changed Paths)
Commits
rP20377: Adds a perspective matrix, refracts a matrix projection setup to prepare for an…
Summary

Refracted matrix projection setup, preparing for isometric view.

Test Plan
  1. Run the game
  2. Test, that all looks the same

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

vladislavbelov created this revision.Oct 21 2017, 11:43 PM
vladislavbelov updated this revision to Diff 3928.Oct 21 2017, 11:45 PM
Stan added a subscriber: Stan.Oct 22 2017, 12:29 AM

Hey thanks for working on this. May I suggest a few things ?

Using explicit static casts instead https://stackoverflow.com/questions/103512/why-use-static-castintx-instead-of-intx

And adding spaces after and prior to mathematical operators ?

source/maths/Matrix3D.cpp
38 ↗(On Diff #3928)

Should either be nuked or replace with doxygen.

56 ↗(On Diff #3928)

Maybe we can save some computation as the second part is also computed above. I don't know if that code is speed critical.

Vulcan added a subscriber: Vulcan.Oct 22 2017, 2:02 AM

Build is green

Updating workspaces...
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimation]’:
FCollada/FCDocument/FCDLibrary.cpp:149:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
  const T* cptr = ((const FCDLibrary<T>*)l1)->GetEntity(0);
           ^
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimationClip]’:
FCollada/FCDocument/FCDLibrary.cpp:150:34:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDCamera]’:
FCollada/FCDocument/FCDLibrary.cpp:151:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDController]’:
FCollada/FCDocument/FCDLibrary.cpp:152:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEffect]’:
FCollada/FCDocument/FCDLibrary.cpp:153:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEmitter]’:
FCollada/FCDocument/FCDLibrary.cpp:154:28:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDForceField]’:
FCollada/FCDocument/FCDLibrary.cpp:155:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDGeometry]’:
FCollada/FCDocument/FCDLibrary.cpp:156:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDImage]’:
FCollada/FCDocument/FCDLibrary.cpp:157:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDLight]’:
FCollada/FCDocument/FCDLibrary.cpp:158:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:159:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDSceneNode]’:
FCollada/FCDocument/FCDLibrary.cpp:160:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsModel]’:
FCollada/FCDocument/FCDLibrary.cpp:161:33:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:162:36:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsScene]’:
FCollada/FCDocument/FCDLibrary.cpp:163:33:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
In file included from FCollada/FUtils/FUSemaphore.cpp:10:0:
FCollada/FUtils/FUSemaphore.h:36:2: warning: #warning "FUSemaphore: Semaphore not implemented for non Windows" [-Wcpp]
 #warning "FUSemaphore: Semaphore not implemented for non Windows"
  ^
FCollada/FUtils/FUStringConversion.cpp: In function ‘void TrickLinkerFUStringConversion()’:
FCollada/FUtils/FUStringConversion.cpp:281:8: warning: variable ‘f’ set but not used [-Wunused-but-set-variable]
  float f = FUStringConversion::ToFloat(&c);
        ^
FCollada/FUtils/FUStringConversion.cpp:283:7: warning: variable ‘b’ set but not used [-Wunused-but-set-variable]
  bool b = FUStringConversion::ToBoolean(c);
       ^
FCollada/FUtils/FUStringConversion.cpp:285:8: warning: variable ‘i32’ set but not used [-Wunused-but-set-variable]
  int32 i32 = FUStringConversion::ToInt32(&c);
        ^
FCollada/FUtils/FUStringConversion.cpp:287:9: warning: variable ‘u32’ set but not used [-Wunused-but-set-variable]
  uint32 u32 = FUStringConversion::ToUInt32(&c);
         ^
In file included from FCollada/FUtils/FUThread.cpp:10:0:
FCollada/FUtils/FUThread.h:30:2: warning: #warning "Threads not yet implemented for non Windows." [-Wcpp]
 #warning "Threads not yet implemented for non Windows."
  ^
FCollada/FCDocument/FCDAnimationCurve.cpp: In member function ‘float FCDAnimationCurve::Evaluate(float) const’:
FCollada/FCDocument/FCDAnimationCurve.cpp:396:13: warning: ‘inTangent.FMVector2::<anonymous>.FMVector2::<anonymous union>::x’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   FMVector2 inTangent;
             ^
FCollada/FCDocument/FCDAnimationCurve.cpp:396:13: warning: ‘inTangent.FMVector2::<anonymous>.FMVector2::<anonymous union>::y’ may be used uninitialized in this function [-Wmaybe-uninitialized]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimation]’:
FCollada/FCDocument/FCDLibrary.cpp:149:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
  const T* cptr = ((const FCDLibrary<T>*)l1)->GetEntity(0);
           ^
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimationClip]’:
FCollada/FCDocument/FCDLibrary.cpp:150:34:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDCamera]’:
FCollada/FCDocument/FCDLibrary.cpp:151:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDController]’:
FCollada/FCDocument/FCDLibrary.cpp:152:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEffect]’:
FCollada/FCDocument/FCDLibrary.cpp:153:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEmitter]’:
FCollada/FCDocument/FCDLibrary.cpp:154:28:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDForceField]’:
FCollada/FCDocument/FCDLibrary.cpp:155:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDGeometry]’:
FCollada/FCDocument/FCDLibrary.cpp:156:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDImage]’:
FCollada/FCDocument/FCDLibrary.cpp:157:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDLight]’:
FCollada/FCDocument/FCDLibrary.cpp:158:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:159:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDSceneNode]’:
FCollada/FCDocument/FCDLibrary.cpp:160:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsModel]’:
FCollada/FCDocument/FCDLibrary.cpp:161:33:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:

http://jenkins-master:8080/job/phabricator/2144/ for more details.

Harbormaster completed remote builds in B3439: Diff 3927.Oct 22 2017, 2:02 AM
Vulcan added a comment.Oct 22 2017, 2:19 AM

Build is green

Updating workspaces...
Build (release)...
../../../source/graphics/Camera.cpp:51:35: warning: unused parameter ‘nearp’ [-Wunused-parameter]
 void CCamera::SetProjection(float nearp, float farp, float fov)
                                   ^
../../../source/graphics/Camera.cpp:51:48: warning: unused parameter ‘farp’ [-Wunused-parameter]
 void CCamera::SetProjection(float nearp, float farp, float fov)
                                                ^
../../../source/graphics/Camera.cpp:51:60: warning: unused parameter ‘fov’ [-Wunused-parameter]
 void CCamera::SetProjection(float nearp, float farp, float fov)
                                                            ^
Build (debug)...
../../../source/graphics/Camera.cpp:51:35: warning: unused parameter ‘nearp’ [-Wunused-parameter]
 void CCamera::SetProjection(float nearp, float farp, float fov)
                                   ^
../../../source/graphics/Camera.cpp:51:48: warning: unused parameter ‘farp’ [-Wunused-parameter]
 void CCamera::SetProjection(float nearp, float farp, float fov)
                                                ^
../../../source/graphics/Camera.cpp:51:60: warning: unused parameter ‘fov’ [-Wunused-parameter]
 void CCamera::SetProjection(float nearp, float farp, float fov)
                                                            ^
Running release tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/2145/ for more details.

Harbormaster completed remote builds in B3440: Diff 3928.Oct 22 2017, 2:19 AM
vladislavbelov added a comment.Oct 22 2017, 9:58 AM
In D971#37857, @Stan wrote:

Hey thanks for working on this. May I suggest a few things ?

Sure.

Using explicit static casts instead https://stackoverflow.com/questions/103512/why-use-static-castintx-instead-of-intx

I don't sure that it's really helpful, because it's simple cast for a compiler. But it's not hard to change.

And adding spaces after and prior to mathematical operators ?

Yep.

vladislavbelov added inline comments.Oct 22 2017, 9:59 AM
source/maths/Matrix3D.cpp
38 ↗(On Diff #3928)

Not sure, that it should be done now.

56 ↗(On Diff #3928)

I think, compiler can do these things, because this is a simple expression enough.

vladislavbelov updated this revision to Diff 3930.Oct 22 2017, 10:05 AM

Added casts and spaces.

Vulcan added a comment.Oct 22 2017, 10:25 AM

Build is green

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

http://jenkins-master:8080/job/phabricator/2146/ for more details.

Harbormaster completed remote builds in B3441: Diff 3930.Oct 22 2017, 10:25 AM
vladislavbelov retitled this revision from Added perspective matrix to Adds perspective matrix.Oct 22 2017, 11:21 AM
wraitii accepted this revision.Oct 29 2017, 3:13 PM
wraitii added a subscriber: wraitii.

Works OK from what I can see and not a performance hotspot -> commit.

This revision is now accepted and ready to land.Oct 29 2017, 3:13 PM
Stan awarded a token.Oct 29 2017, 3:16 PM
Closed by commit rP20377: Adds a perspective matrix, refracts a matrix projection setup to prepare for an… (authored by vladislavbelov). · Explain WhyOct 30 2017, 12:42 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
ps/
trunk/
source/
graphics/
24 lines
maths/
6 lines
33 lines

Diff 4023

View Options

ps/trunk/source/graphics/Camera.cpp

Loading...
View Options

ps/trunk/source/maths/Matrix3D.h

Loading...
View Options

ps/trunk/source/maths/Matrix3D.cpp

Loading...
Wildfire Games · Phabricator