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

Adding the GLContext class. #15

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

Conversation

linkpy
Copy link

@linkpy linkpy commented Aug 11, 2017

Nothing much to say.

I'm quiet new in Crystal, so fell free to give suggestions and critics about my code.

Copy link
Owner

@ysbaddaden ysbaddaden left a comment

Choose a reason for hiding this comment

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

Thank you for trying out OpenGL in SDL.

Can you run crystal tool format src/context.cr, so the code is formatted in the expected way (2 spaces indentation, ...)?

Overall, you can remove most type definitions, for example replace w : Window with a descriptive window variable name. You can also remove most explicit return, since all methods implicitely return the latest expression.

src/context.cr Outdated
def self.load_library( path : String | Nil ) : Nil
return if @@lib_loaded

ret = 0
Copy link
Owner

Choose a reason for hiding this comment

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

You may just do the following; Crystal automatically transforms nil to a NULL pointer, and specifying types is optional.

def self.load_library(path)
  return if @@initialized
  @@initialized = true

  if LibSDL.gl_load_library(path) == 0
    raise Error.new("SDL_GL_LoadLibrary")
  end
end

src/context.cr Outdated

# TODO: Maybe a generic function returning a `Proc` object ?
# not good enough for doing it right.
def self.get_proc_address( name : String ) : Void*
Copy link
Owner

Choose a reason for hiding this comment

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

Yes, we shouldn't have to deal with C pointers in Crystal. Can you point me to some usage examples?

Copy link
Author

Choose a reason for hiding this comment

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

SDL::GLContext.load_library( "path/to/my/gl/lib" )
# This is here I don't know how to do (i'm not even sure it's correct code) :
clear_color = SDL::GLContext.proc_address(Void, Float, Float, Float, Float)( "glClearColor" )
# clear_color would be of type `Proc(Void, Float, Float, Float, Float)`

Copy link
Owner

@ysbaddaden ysbaddaden Aug 11, 2017

Choose a reason for hiding this comment

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

You must cast the void pointer to a proc, something like:

clear_color = LibSDL.gl_get_proc_address("glClearColor")
  .as(Pointer(Float32, Float32, Float32, Float32 ->))
  .value
clear_color.call(0_f32, 0_f32, 0_f32, 0_f32)

I'm not sure this is required, thought. We can initialize a GL context in SDL then simply call GL functions, can't we? That way we wouldn't need to load libraries nor dynamically link symbols at runtime.

src/context.cr Outdated
def self.set_swap_interval( nv : Int ) : Nil
ret = LibSDL.gl_set_swap_interval( nv )
raise Error.new( "SDL_GL_SetSwapInterval" ) unless ret == 0
end
Copy link
Owner

Choose a reason for hiding this comment

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

We can use a getter/setter here:

def self.swap_interval
  LibSDL.gl_get_swap_interval
end

def self.swap_interval=(interval)
  unless LibSDL.gl_set_swap_interval(interval) == 0
    raise Error.new("SDL_GL_SetSwapInterval")
  end
  interval
end

@linkpy
Copy link
Author

linkpy commented Aug 12, 2017

After some tries I got something (but I can't really try it) :

def self.get_proc_address( name )
  # Current implementation, nothing changes here but we need to keep this
  # function.
end

macro []( name, *types )
  Proc( {{*types}} ).new( SDL::GLContext.get_proc_address( {{name}} ), Pointer(Void).null )
end

# And we should it this macro like this :
clear_color = SDL::GLContext[ "glClearColor", Void, Float, Float, Float, Float ]

I found it a bit weird, but we don't need to use the [] operator, we can use proc_address, or anything else that can fit our needs.

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

Successfully merging this pull request may close these issues.

None yet

2 participants