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

Generic Matrices Using OpenGL Mathematics (GLM) #1396

Merged
merged 39 commits into from Oct 1, 2018

Conversation

Projects
None yet
3 participants
@RobertBColton
Member

RobertBColton commented Sep 28, 2018

So this is pretty much the last of the big cleanups that needed to take place and I'm very happy to have it done. Basic idea we've been discussing is that there's no need for us to use 4 separate solutions for matrix transformations and we can just make a single generalized implementation of the matrices. That's what this pull request hopes to accomplish by making use of the OpenGL Mathematics (GLM) library. This should result in more consistent rendering across all of the graphics systems and address outstanding bugs in the transform functions.

This pull request obviously adds a new dependency on GLM. I intend to make an announcement once we decide to merge this and let everyone know about the changes.

# Ubuntu
sudo apt-get install libglm-dev

# MSYS2 32-bit
pacboy -S glm:i

# MSYS2 64-bit
pacboy -S glm:x

There are several reasons for choosing GLM over the other solutions.

  • D3DX is deprecated, outdated, and not generally available for D3D11 or GL
  • OpenGL fixed-function pipeline is also deprecated and not available in newer GL or D3D
  • DirectXMath is not as friendly to GL or GLES as GLM is to D3D
  • GLM is a project that stands on its own, meaning it should have less bugs and actually save us maintenance work
  • GLM is very popular, optimized, and efficient on mobile and OpenGL ES
  • GLM already supports a myriad of additional and miscellaneous graphics math functionality which does not exist in TheExDeus's math headers
  • TheExDeus is not as active anymore or generally available to maintain or extend his math headers if we needed or wanted him to
  • GLM is generally available on most package managers for us to install it on AppVeyor/Travis

MinGW32/MinGW64 Deps

Fundies added two jobs to our Travis matrix to test cross compilation. The two jobs rely on a zip of deps he uploaded to GitHub. Since I am adding GLM as a dependency, I obviously had to update this zip. I uploaded it here to this issue and used that URL in the Travis yaml. Not sure what else to do or if we should move this zip somewhere safer.
Deps zip: enigma-libs.zip

Changes in this pull request

  • The wheels of the car in Wild Racing are now properly rotated
  • The world sphere in Geosphere example is now correctly scaled and not ellipsoidal
  • d3d_start in d3d9 now correctly turns on hidden surface removal
  • All of the GMS2 matrix functions have been added, but may not work exactly yet because they are untested
  • The 3D Cubes demo now has a correct transformation in all 4 systems, including GL1 where it was broken
  • The 3D Cubes demo now has correct lighting in GL1 thanks to discoveries of D3D<->OGL differences and new clarity to the problem
  • The House Effects demo has more correct lighting now wrt the ceiling in GL1 thanks to the former
  • The character in the Animation Platform Example sent to me by DarkAceZ is now correctly transformed in all 4 systems, including GL1 where it was broken
  • The ceiling tiles in House Effects Demo are correctly transformed in GL1 now and not sideways
  • d3d_set_projection_perspective now rotates the projection by the angle parameter instead of using it as the field of view, and the field of view is 40 by default, just like GM8
  • Because of the former change, view_angle now works correctly for perspective views

Other things

  • I removed all of the code that would be for updating light positions in D3D9 and D3D11 because it never applied to those systems. D3D's fixed-function lighting defined lights in world space, and thus so did GM classic and GameMaker: Studio v1.4. OpenGL ffp is the only case where lights are defined in "eye coordinates" which means that we need to set the view matrix with no world matrix when updating lights in that system according to Khronos.

When a light is positioned in the world it should be positioned when the viewing matrix is on the MODELVIEW stack.

https://docs.microsoft.com/en-us/windows/desktop/direct3d9/light-properties
https://www.khronos.org/opengl/wiki/Viewing_and_Transformations#I_put_my_gluLookAt.28.29_call_on_my_Projection_matrix_and_now_fog.2C_lighting.2C_and_texture_mapping_don.27t_work_correctly._What_happened.3F

  • I decided to keep the d3d_projection_stack_* set of functions introduced by TheExDeus because they are simple to add and maintain, were provided by legacy OpenGL, and are friendly to novice programmers
  • I decided to keep Harri's d3d_transform_add_rotation and d3d_transform_set_rotation functions because they make it easier to rotate objects and are also easy for novice programmers to understand
  • I decided to keep Harri's d3d_set_projection_ortho_lookat because while it was possible to do that in GM8 and GMS already, this variant of the function is more efficient to call because it does a single matrix multiplication in a single function call, whereas doing it in GM requires two function calls and two matrix multiplications
  • I did remove and not reimplement Harri's d3d_transform_set_look_at and d3d_transform_add_look_at because I do not see any need to do that to the world transform, and I also feel like that's inconsistent with GM's API that is designed for novices. It can still be done through the GMS matrix functions though.
  • I did remove and not reimplement Harri's d3d_transform_force_update because we no longer cache the matrix update for later as of this pull request. The reasoning is simple, if we want to allow extensions to our graphics systems, then our transforms must be applied at the time of construction so they affect extensions.
  • I did remove and not reimplement Harri's functions for accessing and mutating the transform and projection matrices using raw pointers because I do not see a need for them, I feel they are too advanced for novices, and generally unsafe to work with. The GMS matrix functions will still allow users to build custom matrices using var arrays. Another issue with them is that they are a bit redundant, whereas the matrix functions combine all of this into a single function matrix_get very efficiently. With the way our pipeline is set up now maybe later we can bring them back under the matrix_ family of functions as matrix_get_raw for example. I do understand that raw pointers may be quicker than working with var arrays, I just mainly do not see it as a good idea to mix this with the classic GM transforms API. I feel we should keep the classic API simple and more oriented at novices, and then extend newer APIs with advanced stuff.
  • Even though I have basically completely rewritten all of this transform code, I have kept everyone on the copyright headers and updated myself to 2018 because I referenced the old code a lot while rewriting it

Animation Platform Example

This was provided to me by DarkAceZ and I am just using it show the consistency of the transforms at startup now.

OpenGL1 (transform fixed)

Animation Platform Example GL1

OpenGL3 (basically the same)

Animation Platform Example GL3

Direct3D9 (hidden surface removal fixed)

Animation Platform Example D3D9

Direct3D11 (transform and projection fixed)

Animation Platform Example D3D11

@RobertBColton RobertBColton force-pushed the generic-glm-matrix branch from b47f372 to 43054a2 Sep 28, 2018

@RobertBColton RobertBColton force-pushed the generic-glm-matrix branch 4 times, most recently from 00ac470 to 78be09f Sep 28, 2018

@codecov

This comment has been minimized.

codecov bot commented Sep 29, 2018

Codecov Report

Merging #1396 into master will decrease coverage by 0.2%.
The diff coverage is 18.67%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1396      +/-   ##
==========================================
- Coverage   16.94%   16.74%   -0.21%     
==========================================
  Files         166      164       -2     
  Lines       17310    17095     -215     
==========================================
- Hits         2933     2862      -71     
+ Misses      14377    14233     -144
Impacted Files Coverage Δ
...system/SHELL/Graphics_Systems/General/GSmatrix.cpp 12.77% <12.77%> (+12.77%) ⬆️
...GMAsystem/SHELL/Graphics_Systems/OpenGL1/GLd3d.cpp 22.44% <63.33%> (+4.84%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ea28f8...c86f060. Read the comment docs.

@RobertBColton RobertBColton force-pushed the generic-glm-matrix branch 2 times, most recently from f15a274 to be905ee Sep 29, 2018

@enigma-dev enigma-dev deleted a comment from EnigmaBot Sep 29, 2018

@enigma-dev enigma-dev deleted a comment from EnigmaBot Sep 30, 2018

@enigma-dev enigma-dev deleted a comment from EnigmaBot Sep 30, 2018

@enigma-dev enigma-dev deleted a comment from EnigmaBot Sep 30, 2018

@EnigmaBot

This comment has been minimized.

EnigmaBot commented Oct 1, 2018

Regression tests have indicated that graphical changes have been introduced. Carefully review the following image comparison for anomalies and adjust the changeset accordingly.

328cbf6 Master Diff
Image Diff Image Diff Screen Save

@enigma-dev enigma-dev deleted a comment from EnigmaBot Oct 1, 2018

@RobertBColton

This comment has been minimized.

Member

RobertBColton commented Oct 1, 2018

@JoshDreamland EnigmaBot is correct that there are changes in the screen. It seems the projection of GLM must be using different half-pixel alignment. I wonder what we should do about that or do nothing?

@JoshDreamland

Great change. I came for the lighting but stayed for the cleanup.

Show resolved Hide resolved ENIGMAsystem/SHELL/Graphics_Systems/OpenGL1/GLd3d.cpp
@@ -154,7 +154,7 @@ static inline int draw_tiles()
void clear_view(float x, float y, float w, float h, float angle, bool showcolor)
{
if (enigma::d3dMode && enigma::d3dPerspective)
d3d_set_projection_perspective(x, y, w, h, angle);
d3d_set_projection_perspective(x, y, w, h, 40);

This comment has been minimized.

@JoshDreamland

JoshDreamland Oct 1, 2018

Member

40? Did you mean to leave this? What is 40?

This comment has been minimized.

@RobertBColton

RobertBColton Oct 1, 2018

Member

Nope, I've now fixed it: c86f060

This was a mistake I made in #1388 with respect to the default field of view. I don't know how I didn't see it in the GM8 manual, but the angle parameter of both d3d_set_projection_ortho and d3d_set_projection_perspective is an angle used to rotate the entire screen. So for example you could rotate the picture upside down this way. This is why this pull request is good, because we are eliminating these inconsistencies and ensuring we're doing the right thing.

http://gamemaker.info/en/manual/415_05_projection#d3d_set_projection_ortho

This is called the field of view. A reasonable value is somewhere between 40 and 45 degrees.

Or you want to return to the default perspective projection.

d3d_set_projection_ortho(x,y,w,h,angle) Sets a normal orthographic projection of the indicated area in the room, rotated over the indicated angle.
d3d_set_projection_perspective(x,y,w,h,angle) Sets a normal perspective projection of the indicated area in the room, rotated over the indicated angle.

But I also know from comparing fps6.gmk to GM8 that 40 should be the default. That game only ever uses d3d_set_perspective and d3d_set_projection so it doesn't call the version that sets the field of view. And this is the only field of view I can use which gives the same render pixel for pixel.

Pull request GM8
ENIGMA GL1 fps6.gmk GM8 fps6.gmk
Show resolved Hide resolved ENIGMAsystem/SHELL/Graphics_Systems/General/GSmatrix.h
Show resolved Hide resolved ENIGMAsystem/SHELL/Graphics_Systems/General/GSmatrix.cpp
Show resolved Hide resolved ENIGMAsystem/SHELL/Graphics_Systems/Direct3D9/DX9d3d.cpp
Show resolved Hide resolved ENIGMAsystem/SHELL/Graphics_Systems/Direct3D11/DX11vertex.cpp
@JoshDreamland

This comment has been minimized.

Member

JoshDreamland commented Oct 1, 2018

Oh, and regarding the half-pixel alignment: the new screenshot is superior to the old. Look at the corners of the rectangle. They're actually sharp, now (the cornermost pixel is no longer missing). I think it's fine to not worry about it. If you're feeling jumpy, check to see if the (x, y)th pixel is actually the one being filled (that is, manually verify that the corners of the rectangle are the ones you're passing to draw_rectangle).

@EnigmaBot

This comment has been minimized.

EnigmaBot commented Oct 1, 2018

Regression tests have indicated that graphical changes have been introduced. Carefully review the following image comparison for anomalies and adjust the changeset accordingly.

c86f060 Master Diff
Image Diff Image Diff Screen Save

@enigma-dev enigma-dev deleted a comment from EnigmaBot Oct 1, 2018

@RobertBColton

This comment has been minimized.

Member

RobertBColton commented Oct 1, 2018

@JoshDreamland Ok, I tested it. It seems like OpenGL is a full pixel off from where D3D is in these images. This inconsistency has been a long time issue for us, and we'll just have to come back to it again in the future. I think I am now going to proceed to merging this.

draw_rectangle(1, 1, 3, 3, true);
draw_rectangle(4, 4, 6, 6, false);
GL1 GL3 D3D9 D3D11
Half-Pixel Alignment of Rectangle GL1 Half-Pixel Alignment of Rectangle GL3 Half-Pixel Alignment of Rectangle D3D9 D3D11

@RobertBColton RobertBColton merged commit 4e8ad7c into master Oct 1, 2018

4 checks passed

codecov/patch 18.67% of diff hit (target 16.94%)
Details
codecov/project Absolute coverage decreased by -0.2% but relative coverage increased by +1.73% compared to 4ea28f8
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@RobertBColton RobertBColton deleted the generic-glm-matrix branch Oct 1, 2018

JoshDreamland added a commit that referenced this pull request Nov 10, 2018

Generic Matrices Using OpenGL Mathematics (GLM) (#1396)
* All old matrix and transform code has been generalized using the GLM library
* GL1 lighting has been fixed by using only the view matrix to define and update light positions
* The wheels of the car in Wild Racing are now properly rotated
* The world sphere in Geosphere example is now correctly scaled and not ellipsoidal
* `d3d_start` in d3d9 now correctly turns on hidden surface removal
* All of the GMS2 matrix functions have been added, but may not work exactly yet because they are untested
* The 3D Cubes demo now has a correct transformation in all 4 systems, including GL1 where it was broken
* The 3D Cubes demo now has correct lighting in GL1 thanks to discoveries of D3D<->OGL differences and new clarity to the problem
* The House Effects demo has more correct lighting now wrt the ceiling in GL1 thanks to the former
* The character in the Animation Platform Example sent to me by DarkAceZ is now correctly transformed in all 4 systems, including GL1 where it was broken
* The ceiling tiles in House Effects Demo are correctly transformed in GL1 now and not sideways
* `d3d_set_projection_perspective` now rotates the projection by the angle parameter instead of using it as the field of view, and the field of view is 40 by default, just like GM8
* Because of the former change, view_angle now works correctly for perspective views
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment