We define the Camera as a single object that contains only orientation (position + rotation) and projection at some moment. We use this information to render a scene and additional things, like a shadow map. So the Camera doesn't contain any animation, event handling and so on, purely graphics object.
Details
Details
- Reviewers
elexis - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rP22404: Refactors camera projections - makes projection functions of camera more clear.
- Apply the patch and compile the game with tests
- Make sure that all tests are passed
- Make sure that everything looks as before
Diff Detail
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
| source/graphics/Camera.h | ||
|---|---|---|
| 49 ↗ | (On Diff #8612) | I didn't use SetPerspectiveProjection, because I think it's too long and it doesn't help much (because perspective camera is a known word collocation). |
| 109 ↗ | (On Diff #8612) | I didn't move it under private too, because there're many occurrences of its using and this patch is only about projections. |
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues:
Executing section Source...
source/tools/atlas/GameInterface/View.cpp
| 211| » » g_Game->Interpolate(actualFrameLength,·realFrameLength);
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character '{' when these macros are defined: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...Link to build: https://jenkins.wildfiregames.com/job/differential/1818/display/redirect
| source/graphics/Camera.h | ||
|---|---|---|
| 49 ↗ | (On Diff #8612) | (One could add comments for the functions) I wouldn't disagree with SetPerspectiveProjection, even if it's a bit longer, so that it's clear from the name that this in the end sets the projection, so that the reader sees both Set*Projection functions have this in common. SetProjection, SetPerspective sound more like two functions changing something different. (I'm speaking on the naming because the naming is the only thing changed in this diff) I won't object if you prefer SetPerspective. |
| source/graphics/Camera.h | ||
|---|---|---|
| 49 ↗ | (On Diff #8612) | Probably SetPerspectiveProjection is more clear at the moment. I'm not sure about comments, functions do what their names tell. |
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues:
Executing section Source...
source/tools/atlas/GameInterface/View.cpp
| 211| » » g_Game->Interpolate(actualFrameLength,·realFrameLength);
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character '{' when these macros are defined: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...Link to build: https://jenkins.wildfiregames.com/job/differential/1825/display/redirect
Comment Actions
The rename is acceptable for me. Not compiled, except for seeing Jenkins suceed and this not doing more than a rename and one private-flagging.
A quick search for m_ProjMat
graphics/Camera.cpp: m_ProjMat.SetPerspective(m_FOV, aspect, m_NearPlane, m_FarPlane); graphics/Camera.cpp: m_ProjMat.SetPerspectiveTile(m_FOV, aspect, m_NearPlane, m_FarPlane, tiles, tile_x, tile_y); graphics/Camera.cpp: MatFinal = m_ProjMat * MatView; graphics/Camera.cpp: CMatrix3D transform = m_ProjMat * m_Orientation.GetInverse(); renderer/Renderer.cpp: camera.m_ProjMat = scaleMat * camera.m_ProjMat; renderer/Renderer.cpp: camera.m_ProjMat = scaleMat * camera.m_ProjMat;
shows that it seems that the change is complete too.
| source/tools/atlas/GameInterface/View.cpp | ||
|---|---|---|
| 225 ↗ | (On Diff #8621) | (I guess most compilers are smart enough to inline) |
Comment Actions
Tested it on windows. Noticed no apparent glitches, nor errors, rotated camera zoomed in and out, and did both at the same time. Parallax look the same.
EDIT: Sorry didn't run the tests.