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.

Sign up for GitHub

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

Update gl.cpp #2

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Update gl.cpp #2

Changes from 12 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
0c26fb3
Update gl.cpp
ricosolana Feb 9, 2021
179bc07
Revert "Update gl.cpp"
ricosolana Feb 10, 2021
9f3b46c
signal
ricosolana Feb 10, 2021
a01369f
SDL2
ricosolana Feb 10, 2021
b53c81a
nglDrawLine3D div by 0
ricosolana Feb 10, 2021
9ac3f0c
Revert "nglDrawLine3D div by 0"
ricosolana Feb 10, 2021
8e2e440
nglDrawLine3D div by 0
ricosolana Feb 10, 2021
c0d23ed
forgot abs method
ricosolana Feb 10, 2021
8195e1d
msvc beautify
ricosolana Feb 10, 2021
1810fa1
Revert "msvc beautify"
ricosolana Feb 10, 2021
ed7ac6a
nglDrawLine3D z-clipping
ricosolana Feb 10, 2021
ca935a2
ptr
ricosolana Feb 10, 2021
a66153f
__builtin_expect for windows
ricosolana Mar 1, 2021
3b41ceb
static no global
ricosolana Mar 1, 2021
6d897bd
asserts for sdl
ricosolana Mar 1, 2021
15aced6
etiquette and sdl_quit
ricosolana Mar 1, 2021
f30880e
screen inverted nspire only
ricosolana Mar 1, 2021
93eb162
added abs to fix.h
ricosolana Mar 2, 2021
4d149bd
signal def guard
ricosolana Mar 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 43 additions & 17 deletions gl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,17 @@
#ifdef _TINSPIRE
#include <libndls.h>
#else
#include <SDL/SDL.h>
#include <assert.h>
#include <signal.h>
static SDL_Surface *scr;
#ifdef _WIN32
#include <SDL.h>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#include <SDL.h> should actually work for both

#include <signal.h>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be in the "not windows" block?

#else
#include <SDL/SDL.h>
#endif
SDL_Window* sdl_window;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should all be static, as not used/accessible outside of this source file

SDL_Renderer* sdl_renderer;

SDL_Texture* sdl_texture; // send to gpu
#endif

#include "gl.h"
Expand Down Expand Up @@ -62,9 +69,18 @@ void nglInit()
else
lcd_init(SCR_320x240_565);
#else
SDL_Init(SDL_INIT_VIDEO);
scr = SDL_SetVideoMode(SCREEN_WIDTH, SCREEN_HEIGHT, 16, SDL_SWSURFACE);
SDL_CreateWindowAndRenderer(SCREEN_WIDTH, SCREEN_HEIGHT, SDL_WINDOW_SHOWN, &sdl_window, &sdl_renderer);

sdl_texture = SDL_CreateTexture(sdl_renderer,
SDL_PIXELFORMAT_RGB565,
SDL_TEXTUREACCESS_STREAMING,
SCREEN_WIDTH, SCREEN_HEIGHT);

SDL_SetWindowTitle(sdl_window, "nGl");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
SDL_SetWindowTitle(sdl_window, "nGl");
SDL_SetWindowTitle(sdl_window, "nGL");


#ifndef _WIN32
signal(SIGINT, SIG_DFL);
#endif
assert(scr);
#endif

Expand All @@ -84,7 +100,7 @@ void nglUninit()
#ifdef _TINSPIRE
lcd_init(SCR_TYPE_INVALID);
#else
//TODO
SDL_DestroyRenderer(sdl_renderer);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there something like SDL_Quit?

#endif
}

Expand Down Expand Up @@ -238,10 +254,10 @@ void nglDisplay()
else
lcd_blit(screen, SCR_320x240_565);
#else
SDL_LockSurface(scr);
std::copy(screen, screen + SCREEN_HEIGHT*SCREEN_WIDTH, reinterpret_cast<COLOR*>(scr->pixels));
SDL_UnlockSurface(scr);
SDL_UpdateRect(scr, 0, 0, 0, 0);
SDL_UpdateTexture(sdl_texture, NULL, screen, SCREEN_WIDTH*sizeof(COLOR));
SDL_RenderClear(sdl_renderer);
SDL_RenderCopy(sdl_renderer, sdl_texture, NULL, NULL);
SDL_RenderPresent(sdl_renderer);
#endif

#ifdef FPS_COUNTER
Expand Down Expand Up @@ -354,29 +370,36 @@ GLFix nglZBufferAt(const unsigned int x, const unsigned int y)
return z_buffer[x + y * SCREEN_WIDTH];
}

constexpr GLFix ABS(GLFix a) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This belongs into fix.h, as method

return a >= GLFix(0) ? a : -a;
}

//Doesn't interpolate colors even if enabled
void nglDrawLine3D(const VERTEX *v1, const VERTEX *v2)
{
//TODO: Z-Clipping
// Z-Clipping!
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proper Z clipping would involve only drawing the part of the line which fits into the view (z between NEAR_PLANE and inf). This is currently just a band-aid to avoid a division by zero, so the //TODO comment should stay.

if(v1->z < near_plane || v2->z < near_plane)
return;

VERTEX v1_p = *v1, v2_p = *v2;
nglPerspective(&v1_p);
Vogtinator marked this conversation as resolved.
Show resolved Hide resolved
nglPerspective(&v2_p);

const GLFix diff_x = v2_p.x - v1_p.x;
const GLFix dy = (v2_p.y - v1_p.y) / diff_x;
const GLFix diff_y = v2_p.y - v1_p.y;

const COLOR c = v1_p.c;

//Height > width? -> Interpolate X
if(dy > GLFix(1) || dy < GLFix(-1))
//if(dy > GLFix(1) || dy < GLFix(-1))

// if check passes diff_y should always be non zero
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can be more confident here:

Suggested change
// if check passes diff_y should always be non zero
// if check passes, diff_y is non zero

if (ABS(diff_x) < ABS(diff_y))
{
if(v2_p.y < v1_p.y)
std::swap(v1_p, v2_p);

const GLFix diff_y = v2_p.y - v1_p.y;

const GLFix dx = (v2_p.x - v1_p.x) / diff_y;
const GLFix dx = diff_x / diff_y;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's correct to use those values after the std::swap above. The swap is there to guarantee that v1_p has a lower Y than v2_p, so you would have to change the sign of all the diff_ values as well. Otherwise drawn lines face the wrong direction.

It's easier to just always compute the diffs after the swap though - subtraction is cheap.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using diff_x / diff_y is fine here. In the case where the points have to be swapped, diff_y is negative and the division returns the correct result. It's important to not mix diffs before and after the swap though, which is the case in line 403 for instance.

const GLFix dz = (v2_p.z - v1_p.z) / diff_y;

int end_y = v2_p.y;
Expand All @@ -394,10 +417,13 @@ void nglDrawLine3D(const VERTEX *v1, const VERTEX *v2)
}
else
{
const GLFix dy = diff_x != GLFix(0) ? (diff_y / diff_x) : diff_y;

if(v2_p.x < v1_p.x)
std::swap(v1_p, v2_p);

const GLFix dz = (v2_p.z - v1_p.z) / diff_x;
// still need ternary check (for const) since diff_x could be 0
const GLFix dz = diff_x != GLFix(0) ? (v2_p.z - v1_p.z) / diff_x : (v2_p.z - v1_p.z);

int end_x = v2_p.x;

Expand Down