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

Fix 3D Perspective Mode #1388

Merged
merged 3 commits into from Sep 19, 2018

Conversation

Projects
None yet
2 participants
@RobertBColton
Copy link
Member

RobertBColton commented Sep 16, 2018

This one is going to be a little bit complicated to explain, but from my testing, d3d_set_perspective in GM8.1 and GMSv1.4 should never take effect until the next frame/screen_redraw. It should also only take effect if we are actually in d3d mode meaning that d3d_start() has been called.

The newer GMS manual describes this function as if it affects the "default" room projection because it mentions the view:
https://docs.yoyogames.com/source/dadiospice/002_reference/drawing/drawing%203d/3d%20setup/d3d_set_perspective.html

This turns on (or off) perspective projection. With a perspective projection instances that have a greater depth will appear smaller, so when the depth is 0 it is equal to the original size and the viewpoint for the camera is placed at a distance above the room. This default distance is equal to the width of the room as that gives a reasonable starting projection, and only instances in front of the camera are drawn. Don't use instances with a depth smaller than 0 (or at least not smaller than -w where w is the width of the room or the view).

I've tested this on several games which make use of the d3d_set_perspective function including the High Polygonal 3D Cubes Demo, House Effects Demo, Animation Platform Example (DarkAceZ game). I did not discover any regressions as a result of these changes here. However, I should also mention that I notice no improvements in any game that makes use of this function either (probably due to the generally poor understanding of how it was supposed to work even in GM8.1).

To clarify: d3d_set_perspective only affects the starting projection at the beginning of every frame when in 3D mode

As usual, I have created a test GMK: perspective-projection-test.zip

In GM8.1, GMSv1.4, and ENIGMA (with this pr applied) the game draws the text inverted. This means that the orthographic projection set immediately after d3d_start does not take effect because it is overridden by the perspective projection that GM sets before each draw event. If we move the setting of the orthographic projection into the draw event of each, then the text is drawn normally and not inverted. If we run this test project on ENIGMA master, the text is just always drawn normal without being inverted (incorrect because the perspective projection is never applied).

Another example that proves d3d_set_perspective does not have any effect until the next frame is the following:

d3d_set_projection_ortho(0, 0, room_width, room_height, 0);
d3d_set_perspective(true); // no effect in 8.1 or GMS
draw_text(0, 0, "ABCDEFGHIJKLMNOP!");

@RobertBColton RobertBColton changed the title Fix Perspective Mode Fix 3D Perspective Mode Sep 16, 2018

@RobertBColton

This comment has been minimized.

Copy link
Member Author

RobertBColton commented Sep 16, 2018

@JoshDreamland ok this one is basically good to go. I've gone to great depths to make sure this is correct. I don't think we should worry about this too much considering our existing implementations of the functions were totally broken anyway, they would only ever set a perspective one but never unset it. This is definitely a more correct change that behaves more like GM and doesn't appear to break anything. Please take a look at it, thanks!

@@ -153,7 +153,10 @@ static inline int draw_tiles()

void clear_view(float x, float y, float w, float h, float angle, bool showcolor)
{
d3d_set_projection_ortho(x, y, w, h, angle);
if (enigma::d3dMode && enigma::d3dPerspective)
d3d_set_projection_perspective(x, y, w, h, angle);

This comment has been minimized.

@RobertBColton

RobertBColton Sep 16, 2018

Author Member

@JoshDreamland this is where I (and GM) make d3d_set_perspective actually take effect.

This comment has been minimized.

@JoshDreamland

JoshDreamland Sep 18, 2018

Member

I might instead suggest storing the default protection in a matrix, although I suppose the parameters might not multiply in cleanly. This approach is therefore fine, but please consider a draw_clear_projection method, or maybe d3d_reset_projection, or your choice of permutation.

This comment has been minimized.

@RobertBColton

RobertBColton Sep 19, 2018

Author Member

That would be a good idea, but we're not exactly set up to do that cleanly yet until I generalize the transforms. I am not entirely sure I understand the suggested functions though.

@RobertBColton

This comment has been minimized.

Copy link
Member Author

RobertBColton commented Sep 16, 2018

I did this test too which proves that d3d_start and d3d_end do change the same variable as d3d_set_perspective:

d3d_set_perspective(true);
d3d_start();
d3d_set_perspective(true);
d3d_end();
d3d_set_perspective(true);

Only if you comment d3d_end is the text drawn inverted because of a perspective projection being set.

@JoshDreamland
Copy link
Member

JoshDreamland left a comment

Just the one comment (I expect clear_view to take an index, not a text). Otherwise, LGTM.

@RobertBColton

This comment has been minimized.

Copy link
Member Author

RobertBColton commented Sep 19, 2018

Alright, yeah I finally figured out how to get apitrace working with regular old GM. You have to use the x86 build of apitrace on GM's built executables. Doing this I was able to prove definitively that GM resets the viewport and projection matrix every frame.

// Create Event
d3d_start();
frames = 0;
// Draw Event
draw_line(0, 0, 10, 10);
frames += 1;
if (frames > 5) {
    d3d_set_perspective(false);
}
if (frames > 10) {
    game_end();
}

As you can see in the following image, the API trace created from the above GML code changes the projection matrix between frames 7 and 8. I can verify this by looking at the render target texture within apitrace because frame 8 shows the diagonal line in the upper-left of the window instead of the bottom-left like frame 7. This must mean that draw events start at frame 2, and since we call d3d_set_perspective(false) after 5 frames, we would expect that the transform would change in the frame after frame 7, which is frame 8. That's exactly what it does and it basically proves everything I've stated here. GM does reset the viewport and projection every frame, and d3d_set_perspective only affects how that projection is set on the next frame.

GM8 API Trace

@RobertBColton

This comment has been minimized.

Copy link
Member Author

RobertBColton commented Sep 19, 2018

In regards to making clear_view take an index instead of all the parameters, we can't do that because we use this helper function even when views are disabled. When that is the case we use the window region functions to determine the scaled region to clear as the viewport.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Sep 19, 2018

Codecov Report

Merging #1388 into master will increase coverage by 0.15%.
The diff coverage is 44.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1388      +/-   ##
==========================================
+ Coverage   16.82%   16.97%   +0.15%     
==========================================
  Files         165      166       +1     
  Lines       17307    17309       +2     
==========================================
+ Hits         2912     2939      +27     
+ Misses      14395    14370      -25
Impacted Files Coverage Δ
...system/SHELL/Graphics_Systems/OpenGL1/GLmatrix.cpp 18.25% <ø> (+4.3%) ⬆️
...system/SHELL/Graphics_Systems/General/GSmatrix.cpp 0% <0%> (ø)
...system/SHELL/Graphics_Systems/General/GSscreen.cpp 37.5% <100%> (+1.05%) ⬆️
...GMAsystem/SHELL/Graphics_Systems/OpenGL1/GLd3d.cpp 17.6% <50%> (+0.24%) ⬆️
...IGMAsystem/SHELL/Graphics_Systems/General/GSmath.h 34.27% <0%> (+6.57%) ⬆️

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 3a8646d...fb93c6b. Read the comment docs.

@RobertBColton RobertBColton force-pushed the perspective-projection-fix branch from fb93c6b to c11d214 Sep 19, 2018

@codecov

This comment has been minimized.

Copy link

codecov bot commented Sep 19, 2018

Codecov Report

Merging #1388 into master will increase coverage by 0.14%.
The diff coverage is 44.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1388      +/-   ##
==========================================
+ Coverage   16.83%   16.97%   +0.14%     
==========================================
  Files         165      166       +1     
  Lines       17307    17309       +2     
==========================================
+ Hits         2913     2939      +26     
+ Misses      14394    14370      -24
Impacted Files Coverage Δ
...system/SHELL/Graphics_Systems/OpenGL1/GLmatrix.cpp 18.25% <ø> (+4.3%) ⬆️
...system/SHELL/Graphics_Systems/General/GSmatrix.cpp 0% <0%> (ø)
...system/SHELL/Graphics_Systems/General/GSscreen.cpp 37.5% <100%> (+1.05%) ⬆️
...GMAsystem/SHELL/Graphics_Systems/OpenGL1/GLd3d.cpp 17.6% <50%> (+0.24%) ⬆️
ENIGMAsystem/SHELL/Platforms/xlib/XLIBmain.cpp 30% <0%> (-1%) ⬇️
...IGMAsystem/SHELL/Graphics_Systems/General/GSmath.h 34.27% <0%> (+6.57%) ⬆️

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 4801c41...c11d214. Read the comment docs.

@RobertBColton RobertBColton merged commit 007a772 into master Sep 19, 2018

@RobertBColton RobertBColton deleted the perspective-projection-fix branch Sep 19, 2018

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

Fix 3D Perspective Mode (#1388)
Set a perspective projection in screen_redraw when 3D mode is used and the perspective mode is enabled. This is exactly consistent with how the function is supposed to behave in GM.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.