HomeWildfire Games

Refactors Camera aspect ratio and calculation of plane points.

Description

Refactors Camera aspect ratio and calculation of plane points.

Reviewed By: elexis
Commented By: wraitii
Differential Revision: https://code.wildfiregames.com/D2020

Event Timeline

Stan added inline comments.
/ps/trunk/source/graphics/Camera.h
75

Formulation is weird maybe @Gallaecio has an idea or @Nescio.

/ps/trunk/source/graphics/tests/test_Camera.h
127

*perspective.

Stan raised a concern with this commit.EditedApr 23 2020, 5:03 PM

According to https://launchpadlibrarian.net/475855591/buildlog_ubuntu-bionic-i386.0ad_0.0.24~r23598-0ubuntu1~18.04~wfg0_BUILDING.txt.gz by @ricotz, it fails to pass the tests on i386

On Ubuntu 18:04 (gcc7) doesn't work
On Ubuntu 19:04 (gcc9) works

https://launchpad.net/~wfg/+archive/ubuntu/0ad.dev/+packages

/ps/trunk/source/graphics/tests/test_Camera.h
161

This fails.

This commit now has outstanding concerns.Apr 23 2020, 5:03 PM
elexis added a subscriber: elexis.Apr 24 2020, 2:29 AM
In TestCamera::test_persepctive_plane_points:
/<<PKGBUILDDIR>>/source/graphics/tests/test_Camera.h:163: Error: Expected (quad == expectedFarQuad), found ({ 02 00 CA C2 02 00 CA C2 ... } != { 00 00 CA C2 00 00 CA C2 ... })
..Skipping map generator tests (can't find binaries/data/mods/public/maps/random/tests/)
.....................................................................................................................................................................................................................................Skipping globalscripts tests (can't find binaries/data/mods/public/globalscripts/tests/)
.Skipping component scripts tests (can't find binaries/data/mods/public/simulation/components/tests/setup.js)
.....................................................................................................
Failed 1 and Skipped 0 of 336 tests

Just a guess, but if the computation is subject to rounding errors, then those should be fixed point types in the scope of the test or rather the test not be written that is subject to rounding issues.
GUI/rendering doesnt require precise computation / is subject to rounding errors and not considered to be relevant. There are tests like the CCmpPosition test that use CVector3D, but probably not problematic operations.

Stan added a comment.Apr 24 2020, 8:34 AM

I guess camera projections do not have to be exact. But what is strange is that i seems to be fixed with a more recent version of GCC.

vladislavbelov requested verification of this commit.Apr 29 2020, 9:05 PM
This commit now requires verification by auditors.Apr 29 2020, 9:05 PM
Stan resigned from this commit.Jul 29 2020, 4:38 PM

Although the fix is correct (floats should generally not be checked for equality), I am still dubious about that GCC bug.
Camera precision doesn't matter that much, and the tests are fixed, so removing myself as auditor

This commit no longer requires audit.Jul 29 2020, 4:38 PM
Stan removed an auditor: Stan.Jul 29 2020, 4:38 PM