Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Questions regarding Mtx_Translate() #55

Open
BallM4788 opened this issue Jun 16, 2021 · 11 comments
Open

Questions regarding Mtx_Translate() #55

BallM4788 opened this issue Jun 16, 2021 · 11 comments

Comments

@BallM4788
Copy link

Please be patient with me as I am extremely new to graphics libraries and the math involved in 3D rendering, and frankly have very little idea of what I'm talking about in general.

Background: ScummVM uses a modified version of TinyGL (a stripped-down version of OpenGL) to run several 3D games such as Grim Fandango and Myst 3: Exile on lower-end systems (including the 3DS), provided their ScummVM engine has alternate, TinyGL-specific graphics code. The problem is that attempting to run said 3D games crashes the system. Interestingly, the one 3D game that will run on the 3DS (Westwood's Blade Runner), does not use OpenGL nor TinyGL.

Anyway, there was speculation that the issue might be solved by having the 3DS hardware take over some functions. Since the 3DS backend for ScummVM already uses citro3d for screen rendering, I thought it might be worthwhile to try and write a version of ScummVM's TinyGL implementation that utilizes citro3d. Needless to say, due to tinyGL being column-major and citro3d being row-major, I've been in a slog from the beginning.

What I think I know: Correct me if I'm wrong on any of this. To my understanding, converting between column-major and row-major layout simply requires transposing the matrix (and in citro3d's case, additionally flipping the matrix horizontally to account for PICA200's WZYX ordering). This seemed to work out fine for matrix rotation, as doing this then performing a left-handed MTX_Rotate() produced the same values as TinyGL's glopRotate() (with value positions changed appropriately).

In TinyGL, the translation matrix in its Matrix4::translate() looks like this:

Therefore, I expected that citro3d's translation matrix would be situated like this (but flipped horizontally, of course):

However, according to citro3d's code, a left-sided MTX_Translate() changes every cell's value EXCEPT those in the bottom-row. Additionally, right-handed Mtx_Translate() uses a translation matrix that is a horizontal mirror of TinyGL's translation matrix, changing only the W cells.

Questions:

  1. Is this intentional?
  2. If so, am I not understanding something correctly?
  3. If not, how can I work around this?
  4. Am I going about this all wrong?
  5. Is there anything you think I should know before I continue on this magical journey of mathematics, personal growth, and generally banging my head against a wall?
@mtheall
Copy link
Collaborator

mtheall commented Jun 16, 2021

C3D_Mtx bRightSide=true is an OpenGL-style right-handed coordinate system where matrix multiplication is right-associative and has the "effects" from right-to-left.

C3D_Mtx bRightSide=false is an DirectX-style left-handed coordinate system where matrix multiplication is left-associative and has the "effects" from left-to-right.

@BallM4788
Copy link
Author

Thanks, that helps. In light of that, I think I may have found something wrong in Mtx_Rotate(). I made two hackjob test files (one for TinyGL, one for Citro3D) by copypasting code bits.

TinyGL test file results:
waterfox_2021-06-17_07-29-33

Citro3D test file results (matrix printouts flipped horizontally for ease of comparison):
waterfox_2021-06-17_07-24-59

While the other functions produce appropriate results in the Citro3D test, Mtx_Rotate() does not. This might be because the function does not transpose om when bRightSide = true; doing so results in the expected output:
waterfox_2021-06-17_07-49-35

Am I correct in concluding that Mtx_Rotate's current behavior for bRightSide = true is not intended?

@mtheall
Copy link
Collaborator

mtheall commented Jun 17, 2021

Have you had a look at https://github.com/devkitPro/citro3d/tree/master/test?

@BallM4788
Copy link
Author

test: main.cpp:214: void check_matrix(generator_t&, distribution_t&): Assertion `m == glm::mat4()' failed.

I probably did something wrong like install the wrong version of GLM, so I included the whole shebang in the pastebin (GLM install, build process, and test).

@mtheall
Copy link
Collaborator

mtheall commented Jun 17, 2021

Sorry looks like glm changed the way default constructors work. They are now uninitialized (or zero-initialized; didn't check) instead of identity-constructed. I created a branch https://github.com/devkitPro/citro3d/tree/update-glm that restores that behavior for the test, and also prints out the actual/expected data on failures.

@BallM4788
Copy link
Author

BallM4788 commented Jun 17, 2021

On the off chance I didn't screw up on my end, I went through and figured out which of the asserts were failing. I did this by going through and commenting out each line the test program failed at.

EDIT: Missed your reply, but this might be useful anyway.

check_matrix

check identity

  • 214: assert(m == glm::mat4());

check inverse (which you're aware of)

  • 279: assert(id == glm::mat4()); // could still fail due to rounding errors
  • 281: assert(id == glm::mat4()); // could still fail due to rounding errors

check perspective tilt

  • 345: assert(m == fix_depth*g*tilt);
  • 350: assert(m == fix_depth*glm::scale(g, z_flip)*tilt);

check perspective stereo tilt

  • 451: assert(left == fix_depth*g*left_eye*tilt);
  • 452: assert(right == fix_depth*g*right_eye*tilt);
  • 457: assert(left == fix_depth*glm::scale(g*left_eye, z_flip)*tilt);
  • 458: assert(right == fix_depth*glm::scale(g*right_eye, z_flip)*tilt);

check ortho tilt

  • 513: assert(m == tilt*fix_depth*g);
  • 518: assert(m == tilt*fix_depth*glm::scale(g, z_flip));

check lookAt

  • 548: assert(m == glm::scale(glm::mat4(), glm::vec3(-1.0f, 1.0f, -1.0f))*g);
    • preceded by I can't say for certain that this is the correct test

check translate (reversed)

  • 586: assert(m == glm::translate(glm::mat4(), v)*g);

check rotate (reversed)

  • 626: assert(m == glm::rotate(glm::mat4(), r, v)*g);

check rotate X (reversed)

  • 652: assert(m == glm::rotate(glm::mat4(), r, x_axis)*g);

check rotate Y (reversed)

  • 678: assert(m == glm::rotate(glm::mat4(), r, y_axis)*g);

check rotate Z (reversed)

  • 704: assert(m == glm::rotate(glm::mat4(), r, z_axis)*g);

check_quarternion

check identity

  • 789: assert(q == g);

check rotation

  • 918: assert(Quat_Rotate(q, FVec3_New(v.x, v.y, v.z), r, true) == glm::rotate(glm::quat(), r, v)*g);

check rotate X

  • 929: assert(Quat_RotateX(q, r, true) == glm::rotate(glm::quat(), r, x_axis)*g);

check rotate Y

  • 940: assert(Quat_RotateY(q, r, true) == glm::rotate(glm::quat(), r, y_axis)*g);

check rotate Z

  • 951: assert(Quat_RotateZ(q, r, true) == glm::rotate(glm::quat(), r, z_axis)*g);

@BallM4788
Copy link
Author

Just tried the updated test file and yeah, everything passes on that one. Guess I just need to look at all this more to familiarize myself with it.

@mtheall
Copy link
Collaborator

mtheall commented Jun 17, 2021

I would suggest inserting TinyGL into these tests and going from there

@BallM4788
Copy link
Author

That's a good idea, I'll try that.

@BallM4788
Copy link
Author

@mtheall I hope you don't mind me reusing this to avoid cluttering up the issues page. I noticed that the code in mtx_persp.c is different than that of the WolframAlpha equation it links to. Shouldn't line 16 read:
mtx->r[2].w = 0.5f*(near + far) / (near - far) - 0.5f*mtx->r[3].z;
instead of:
mtx->r[2].z = -mtx->r[3].z * near / (near - far);?

@mtheall
Copy link
Collaborator

mtheall commented Jun 26, 2021

3DS GPU clip space has z in [-1, 0] where e.g. OpenGL has it in [-1, 1]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants