10
\$\begingroup\$

FTGL wasn't making me happy, so I decided to try something else. This isn't completely finished; I plan to work more on layouts and alignment, and add a few other things. It should be far enough along to review, though.

I realize there is some inconsistent whitespace, especially around argument lists. I don't care enough to fix it right now. I'd love to get feedback on anything else.


./gltext.h (716 bytes in 42 lines)

#ifndef __GLTEXT_H__
#define __GLTEXT_H__

#ifdef __cplusplus
extern "C" {
#endif

#include <ft2build.h>
#include FT_CONFIG_OPTIONS_H

#include <stdio.h>
#include <wchar.h>
#include <string.h>
#include "vertex-buffer.h"
#include "text-buffer.h"
#include "mat4.h"

typedef struct {
    vec2 *pen;
    markup_t *markup;
    text_buffer_t *text;
    vertex_buffer_t *vert;
    mat4 *model;
    mat4 *view;
    mat4 *projection;
} gltext_t;

gltext_t *gltext_new(void);

void gltext_delete(gltext_t *self);

void gltext_write(gltext_t *self, wchar_t *text);

void gltext_render(gltext_t *self);

void gltext_reshape(gltext_t *self, int width, int height);

#ifdef __cplusplus
}
#endif

#endif /* #define __GLTEXT_H__ */

./gltext.c (4009 bytes in 131 lines)

#include "gltext.h"

#include "shaders/v3f-c4f.vert.h"
#include "shaders/v3f-c4f.frag.h"

static GLuint shader;

static int initted = 0;

static void checkinit(void)
{
    if (initted == 1) return;
    #ifndef FT_CONFIG_OPTION_SUBPIXEL_RENDERING
        fprintf(stderr, "freetype not compiled with subpixel rendering.\n");
        exit( EXIT_FAILURE) ;
    #endif
    glewExperimental = GL_TRUE;
    GLenum err = glewInit();
    if (GLEW_OK != err)
    {
        fprintf( stderr, "Error: %s\n", glewGetErrorString(err) );
        exit( EXIT_FAILURE );
    }
    fprintf( stderr, "Using GLEW %s\n", glewGetString(GLEW_VERSION) );
    shader = shader_load_resource(
        shader_resource_v3f_c4f_vert, 
        shader_resource_v3f_c4f_frag);
    initted = 1;
}

gltext_t *gltext_new(void)
{
    checkinit();
    gltext_t *self = (gltext_t *) malloc (sizeof(gltext_t));
    vec4 black  = {{0.0, 0.0, 0.0, 1.0}};
    vec4 none   = {{1.0, 1.0, 1.0, 0.0}};
    self->vert = vertex_buffer_new( "vertex:3f,color:4f" );
    self->text = text_buffer_new( LCD_FILTERING_ON );
    self->text->base_color = black;

    self->pen = (vec2 *) malloc (sizeof(vec2));
    self->markup = (markup_t *) malloc (sizeof(markup_t));

    self->model = (mat4 *) malloc (sizeof(mat4));
    self->view = (mat4 *) malloc (sizeof(mat4));
    self->projection = (mat4 *) malloc (sizeof(mat4));

    mat4_set_identity( self->projection );
    mat4_set_identity( self->model );
    mat4_set_identity( self->view );

    self->markup->family  = "Sans";
    self->markup->size    = 10.0;
    self->markup->bold    = 0;
    self->markup->italic  = 0;
    self->markup->rise    = 0.0;
    self->markup->spacing = 0.0;
    self->markup->gamma   = 1.0;
    self->markup->foreground_color    = black;
    self->markup->background_color    = none;
    self->markup->underline           = 0;   
    self->markup->underline_color     = black;
    self->markup->overline            = 0;  
    self->markup->overline_color      = black;
    self->markup->strikethrough       = 0;   
    self->markup->strikethrough_color = black;
    self->markup->font = 0;

    return self;
}

void gltext_delete(gltext_t *self)
{
    vertex_buffer_delete(self->vert);
    text_buffer_delete(self->text);
    free(self->model);
    free(self->view);
    free(self->projection);
    free(self->pen);
    free(self->markup);
    free(self);
}

void gltext_write( gltext_t *self, wchar_t *text)
{
    text_buffer_add_text(self->text, self->pen, self->markup,
        text, wcslen(text));
}

void gltext_render( gltext_t *self )
{

    // glClearColor( 1.0, 1.0, 1.0, 1.0 );
    // glClear( GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT );

    text_buffer_t *text = self->text;
    vertex_buffer_t *vert = self->vert;
    GLuint textShader = text->shader;

    mat4 *model = self->model;
    mat4 *view = self->view;
    mat4 *projection = self->projection;

    glUseProgram( textShader );
    {
        glUniformMatrix4fv( glGetUniformLocation( textShader, "model" ),
            1, 0, model->data);
        glUniformMatrix4fv( glGetUniformLocation( textShader, "view" ),
            1, 0, view->data);
        glUniformMatrix4fv( glGetUniformLocation( textShader, "projection" ),
            1, 0, projection->data);
        text_buffer_render( text );
    }
    // glBlendFunc( GL_ONE, GL_ONE_MINUS_SRC_ALPHA );
    // glBlendColor( 1.0, 1.0, 1.0, 1.0 );
    glUseProgram( shader );
    {
        glUniformMatrix4fv( glGetUniformLocation( shader, "model" ),
                            1, 0, model->data);
        glUniformMatrix4fv( glGetUniformLocation( shader, "view" ),
                            1, 0, view->data);
        glUniformMatrix4fv( glGetUniformLocation( shader, "projection" ),
                            1, 0, projection->data);
        vertex_buffer_render( vert, GL_LINES );
    }
}

void gltext_reshape( gltext_t *self, int width, int height )
{
    mat4_set_orthographic( self->projection, 0, width, 0, height, -1, 1 );
}

./luagltext.c (4796 bytes in 177 lines)

#include <stdlib.h>
#include <lua.h>
#include <lualib.h>
#include <lauxlib.h>

#include <string.h>

#include "gltext.h"

#define LUAGLTEXT_META "luagltext.meta"

static float get_color_channel(lua_State *L, int index) {
    lua_pushnumber(L, index);
    lua_gettable(L, -2);
    luaL_checktype(L, -1, LUA_TNUMBER);
    float result = lua_tonumber(L, -1);
    lua_pop(L, 1);
    return result;
}

#define LUA_Tboolean LUA_TBOOLEAN
#define LUA_Tnumber LUA_TNUMBER
#define LUA_Tstring LUA_TSTRING

#define SET_MARKUP(name, type) \
    lua_pushstring(L, # name); \
    lua_gettable(L, -2); \
    if (!lua_isnil(L, -1)) { \
        luaL_checktype(L, -1, LUA_T ## type); \
        self->markup->name = lua_to ## type(L, -1); \
    } \
    lua_pop(L, 1); \

#define SET_MARKUP_COLOR(name) \
    lua_pushstring(L, # name "Color"); \
    lua_gettable(L, -2); \
    if (!lua_isnil(L, -1)) { \
        luaL_checktype(L, -1, LUA_TTABLE); \
        float r = get_color_channel(L, 1); \
        float g = get_color_channel(L, 2); \
        float b = get_color_channel(L, 3); \
        float a = get_color_channel(L, 4); \
        vec4 color = {{r, g, b, a}}; \
        self->markup->name ## _color = color; \
    } \
    lua_pop(L, 1); \

static int instance_markup(lua_State *L) {
    luaL_checktype(L, 1, LUA_TLIGHTUSERDATA);
    luaL_checktype(L, 2, LUA_TTABLE);
    gltext_t *self = (void *)lua_touserdata(L, 1);

    self->markup->font = 0;

    lua_pushstring(L, "family");
    lua_gettable(L, -2);
    if (!lua_isnil(L, -1)) {
        luaL_checktype(L, -1, LUA_TSTRING);
        self->markup->family = (char *)lua_tostring(L, -1);
    }
    lua_pop(L, 1);

    // SET_MARKUP(family, string)

    SET_MARKUP(size, number)
    SET_MARKUP(bold, boolean)
    SET_MARKUP(italic, boolean)
    SET_MARKUP(rise, number)
    SET_MARKUP(spacing, number)
    SET_MARKUP(gamma, number)
    SET_MARKUP(underline, boolean)
    SET_MARKUP(overline, boolean)
    SET_MARKUP(strikethrough, boolean)

    SET_MARKUP_COLOR(foreground)
    SET_MARKUP_COLOR(background)
    SET_MARKUP_COLOR(underline)
    SET_MARKUP_COLOR(overline)
    SET_MARKUP_COLOR(strikethrough)

    return 0;
}

static int instance_position(lua_State *L) {
    luaL_checktype(L, 1, LUA_TLIGHTUSERDATA);
    gltext_t *self = (void *)lua_touserdata(L, 1);
    int argc = lua_gettop(L);
    lua_pushnumber(L, self->pen->x);
    lua_pushnumber(L, self->pen->y);
    if (argc > 1 && !lua_isnil(L, 2)) {
      luaL_checktype(L, 2, LUA_TNUMBER);
      self->pen->x = lua_tonumber(L, 2);
    }
    if (argc > 2 && !lua_isnil(L, 3)) {
      luaL_checktype(L, 3, LUA_TNUMBER);
      self->pen->y = lua_tonumber(L, 3);
    }
    return 2;
}

static int instance_render(lua_State *L) {
    luaL_checktype(L, 1, LUA_TLIGHTUSERDATA);
    gltext_t *self = (void *)lua_touserdata(L, 1);
    gltext_render(self);
    return 0;
}

static int instance_reshape(lua_State *L) {
    luaL_checktype(L, 1, LUA_TLIGHTUSERDATA);
    luaL_checktype(L, 2, LUA_TNUMBER);
    luaL_checktype(L, 3, LUA_TNUMBER);
    gltext_t *self = (void *)lua_touserdata(L, 1);
    int width = lua_tonumber(L, 2);
    int height = lua_tonumber(L, 3);
    gltext_reshape(self, width, height);
    return 0;
}

static int instance_write(lua_State *L) {
    luaL_checktype(L, 1, LUA_TLIGHTUSERDATA);
    luaL_checktype(L, 2, LUA_TSTRING);
    gltext_t *self = (void *)lua_touserdata(L, 1);
    const char *mbstring = lua_tostring(L, 2);
    size_t newsize = strlen(mbstring) + 1;
    wchar_t wc[newsize];
    wchar_t *wcstring = wc;
    mbstowcs(wcstring, mbstring, newsize);
    gltext_write(self, wcstring);
    return 0;
}

static int lib_create(lua_State *L) {
    lua_pushlightuserdata(L, gltext_new());
    luaL_getmetatable(L, LUAGLTEXT_META);
    lua_setmetatable(L, -2);
    return 1;
}

static int meta_gc (lua_State *L) {
    luaL_checktype(L, 1, LUA_TLIGHTUSERDATA);
    gltext_t *self = (void *)lua_touserdata(L, 1);
    gltext_delete(self);
    return 0;
}

#define SET_INSTANCE_METHOD(name) \
    lua_pushstring(L, # name); \
    lua_pushcfunction(L, instance_ ## name); \
    lua_settable(L, -3);

int luaopen_luagltext(lua_State *L) {
    luaL_newmetatable(L, LUAGLTEXT_META);
    {
      lua_pushstring(L, "__index");
      lua_newtable(L);
      {
          SET_INSTANCE_METHOD(markup)
          SET_INSTANCE_METHOD(position)
          SET_INSTANCE_METHOD(render)
          SET_INSTANCE_METHOD(reshape)
          SET_INSTANCE_METHOD(write)
      }
      lua_settable(L, -3);

      lua_pushstring(L, "__gc");
      lua_pushcfunction(L, meta_gc);
      lua_settable(L, -3);
    }
    lua_pop(L, 1);

    lua_newtable(L);
    lua_pushstring(L, "create");
    lua_pushcfunction(L, lib_create);
    lua_settable(L, -3);

    return 1;
}

./demo.lua (747 bytes in 44 lines)

require "luagl"
require "luaglut"
local gltext = require "luagltext"

glutInit(arg)
glutInitDisplayMode(GLUT_RGB + GLUT_DOUBLE + GLUT_DEPTH)
glutCreateWindow(arg[0] or "demo")
glClearColor(1, 1, 1, 1)

local page = gltext.create()

glutDisplayFunc(function()
  glClear(GL_COLOR_BUFFER_BIT + GL_DEPTH_BUFFER_BIT)
  page:render()
  glutSwapBuffers()
end)

glutReshapeFunc(function(width, height)
  glViewport(0, 0, width, height)
  page:reshape(width, height)
end)

page:markup({
  family = "Bitstream Vera Sans",
  size = 12,
  foregroundColor = {0, 0, 0, 1},
})

page:position(10, 200)

page:write([[Hello, world!
How are you?]])

page:markup({
  italic = true,
  size = 24,
})

page:position(10, 70)

page:write("Hello again!")

glutMainLoop();
\$\endgroup\$

2 Answers 2

6
\$\begingroup\$

A few general comments, as I know nothing of opengl or lua.

  • Why make gltext_t contain pointers to structure that must be allocated individually, rather than just containing the structs (i.e. omit the '*')?

  • Gltext is an alternative type name that avoids using the reserved _t suffix.

  • When you do allocate structures using malloc you should omit the casts.

  • Your void* casts on assignments to gltext_t* pointers is odd, why not cast using gltext_t* if it is needed?

  • your multi-line macros should be wrapped in a do {....} while(0) (no semicolon) or replaced by functions if at all possible.

\$\endgroup\$
1
  • \$\begingroup\$ text_buffer_t and vertex_buffer_t are opaque handles returned by the FTGL library. The vec2 and mat4s should definitely be stored by value though. \$\endgroup\$
    – glampert
    Sep 19, 2014 at 22:26
6
\$\begingroup\$

Declared types by value when possible:

Dynamic memory allocation places an overhead in both the machine and the programmer, so it should be avoided when possible.

Most fields in the gltext_t struct can be declared by value:

typedef struct {
    // Pointers first
    text_buffer_t   * text;
    vertex_buffer_t * vert;

    // These are all floats internally
    mat4 model;
    mat4 view;
    mat4 projection;
    vec2 pen;

    // And this is an aggregate of several types
    markup_t markup;
} gltext_t;

This will improve data locality and make the creation of gltext_t objects a bit faster, since it involves less dynamic allocations.

Notice that I've also reordered the fields to try to minimize padding added by the compiler. Grouping identical types together will help.

Mind the consts:

In this function:

void gltext_write(gltext_t *self, wchar_t *text);

The parameter text is a read-only reference. So it should be const. This will make your code more self-documented and less error prone:

void gltext_write(gltext_t *self, const wchar_t *text);

Use booleans:

static int initted = 0;

This is a boolean variable, so use bool for it (include <stdbool>).

Calling exit():

You have a few fatal errors currently being handled with a call to exit(). Is this ideal? Shouldn't you give the user a chance to recover in case of error? Maybe and error-return/return-code would be better in this case.

Use assertions:

There are several places where the caller can pass a null pointer to your library and crash the program, example:

void gltext_write( gltext_t *self, wchar_t *text)
void gltext_reshape( gltext_t *self, int width, int height )
// etcetera

If self or text happens to be NULL, you have undefined behavior. It would be much better to trap these errors early with an assert() on those parameters, right at the top of the function's body.

Align similar statements:

This block of assignments will look better if all = signs are aligned:

self->markup->family              = "Sans";
self->markup->size                = 10.0;
self->markup->bold                = 0;
self->markup->italic              = 0;
self->markup->rise                = 0.0;
self->markup->spacing             = 0.0;
self->markup->gamma               = 1.0;
self->markup->foreground_color    = black;
self->markup->background_color    = none;
self->markup->underline           = 0;
self->markup->underline_color     = black;
self->markup->overline            = 0;
self->markup->overline_color      = black;
self->markup->strikethrough       = 0;
self->markup->strikethrough_color = black;
self->markup->font                = 0;

Cache the GL uniform locations:

You should avoid calling glGetUniformLocation() every time you render. Since these values never change after the shader compilation, it would be best to query them once you initialize (on checkinit()) and save the locations inside local variables. E.g.:

static GLuint model_location;
static GLuint view_location;

static void checkinit(void)
{
   ...

   model_location = glGetUniformLocation( textShader, "model" );
   view_location   = glGetUniformLocation( textShader, "view" );
}

Clean up commented-out code:

Remove commented out blocks, like this one, once you are done:

// glBlendFunc( GL_ONE, GL_ONE_MINUS_SRC_ALPHA );
// glBlendColor( 1.0, 1.0, 1.0, 1.0 );

This is only liable to cause distraction and it conveys no information, since it is impossible to know if that code is even correct or not.

Reserved suffix:

The _t suffix is reserved by the POSIX standard for future expansion. So consider another name for gltext_t. Maybe GLText would be a better option.

\$\endgroup\$
3
  • \$\begingroup\$ assert is used to verify logical assumptions, not for run-time error checking. To catch NULL pointers, use a conditional and an error return. \$\endgroup\$ Sep 19, 2014 at 23:29
  • \$\begingroup\$ @WilliamMorris, agreed. But if the function has a precondition/assumption that the pointer is not null, then assert is adequate. \$\endgroup\$
    – glampert
    Sep 20, 2014 at 2:03
  • \$\begingroup\$ @WilliamMorris, glampert actually since the pointer is coming from a Lua lightuserdata, it's more likely to be a pointer to something else entirely than a null pointer. I'm thinking of solving it by including a field containing a type id as the first member of the struct and checking for it in all of those functions. I'm not sure how to avoid the segfault if the pointer points to unallocated memory, though. Thank you both for your reviews. I'll post a followup soon. \$\endgroup\$
    – Dagg
    Sep 20, 2014 at 5:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.