libglui / glui

GLUI is a GLUT-based C++ user interface library which provides controls such as buttons, checkboxes, radio buttons, and spinners to OpenGL applications. It is window-system independent, using GLUT or FreeGLUT.
Other
194 stars 82 forks source link

Rotation texture ID is static/internal global variable. #98

Open m-7761 opened 5 years ago

m-7761 commented 5 years ago

I'm just filing this away here. I don't know how big of a problem this is. If GLUT spreads an OpenGL context over every window then it might be less of a problem. If not, it's a big problem, obviously.

A simple fix is stashing the texture ID in the top-level GLUI container, since that is per GLUT window, unless I'm wrong about that.

But that would not be a solution that extends to other controls. I guess there would need to be a texture slot registration process to arrive at something extensible.

m-7761 commented 5 years ago

https://www.opengl.org/resources/libraries/glut/spec3/node16.html says each GLUT window has its own context. https://www.opengl.org/resources/libraries/glut/spec3/node17.html says the same for subwindows.

Going by this, GLUI's rotation texture will only work on the first window or subwindow that includes one, unless I'm missing something?

nigels-com commented 5 years ago

It's typical to share contexts in multi-window scenarios, especially for the purpose of textures, buffers and display lists.

m-7761 commented 5 years ago

Does GLUT control that? Or is it an option to provide to GLUT? Or is it implementation dependent?

m-7761 commented 5 years ago

I looked and only found some claims that vanilla GLUT doesn't have any context sharing mechanisms. Below is the code I developed for the Rotation texture.

/************************************************************/
/*                                                          */
/*                  Class GLUI_Texture                      */
/*                                                          */
/************************************************************/

class Texture
{
public:

    const int id;
    Texture():id(UI::_textures_id(+1))
    {}

    inline unsigned char &get(UI *ui)
    {
        assert(ui);
        return (unsigned char&)ui->_textures[id];
    }
    inline bool get(UI *ui, unsigned int &texture)
    {
        texture = ui?get(ui):0; return texture!=0;
    }
    inline bool set(UI *ui, unsigned int &texture)
    {
        if(!ui||texture-1>=255) return false;
        unsigned char &slot = get(ui);
        if(!slot) slot = (unsigned char)texture;
        else return false; return true;
    }   
};

The constructor from glui.cpp follows. EDITED: I had to modify things to be able to get the size out of the exported function when initializing the GLUI object. EDITED: Had to edit again to make it possible to add a critical-section later.

int UI::_textures_id(int inc)
{
    //REMINDER: This has to be initialized inside this function to
    //be independent of translation-unit's initialization ordering.
    static int static_local_variable = 0;                                

    int id = static_local_variable; if(inc)
    {
        static_local_variable+=inc; if(glui_initialized)
        {
            size_t size = (size_t)static_local_variable;
            for(UI*ui=GLUI::first_ui();ui;ui=ui->next())
            ui->_textures.resize(size);
        }
    }
    return id;
}

You get the idea. I don't know if it's possible to make a GLUI window that uses the same texture names as the user's drawing area. I've added a canvas feature though for drawing over the UI. But that's a different matter. Anyway, I used String as the container, so the limit is 255.

    /** New way to draw over UI prior to flushing the buffer. */
    inline void set_main_gfx_canvas(void(*canvas_cb)(UI*)=NULL)
    {
        _main_gfx_canvas_cb = canvas_cb;
    }
m-7761 commented 5 years ago

EDITED: I edited this code twice. (I don't know how soon GitHub sends out email notifications.)

The constructor from glui.cpp follows. EDITED: I had to modify things to be able to get the size out of the exported function when initializing the GLUI object. EDITED: Had to edit again to make it possible to add a critical-section later.

nigels-com commented 5 years ago

glGenTextures might be preferable for this. Seems like a lot of plumbing for unique IDs. The advantage with glGenTextures is playing nice with other code that allocates IDs via glGenTextures, rather than hard-coded.

m-7761 commented 5 years ago

glGenTextures might be preferable for this. Seems like a lot of plumbing for unique IDs. The advantage with glGenTextures is playing nice with other code that allocates IDs via glGenTextures, rather than hard-coded.

I don't think you're conceptualizing the problem correctly. The unsigned int &texture argument IS the GLuint passed to glGenTextures. This is a coordination problem because the Rotation control doesn't know if it's the first Rotation in its node-tree or not. If it's first the value is 0. If not the texture is already generated.

The purpose of Texture isn't to interact with OpenGL, it's to solve the coordination problem. It shouldn't be more involved than is absolutely necessary. I've similarly cut back the Bitmap class to precisely its use-case, and the StdBitmaps class is only a typedef now: typedef const Bitmap *StdBitmaps[Bitmap::_NUM_ITEMS];

GLUI is in sore need of a disciplinarian. At best it's raw material in its present state. I'm whipping it into something that sane people might find useful. Take it or leave it.

nigels-com commented 5 years ago

So the problem you're aiming to solve is having more than one copy of the same texture for each OpenGL context?

I don't recall if there is a GLUT wrapper for glXGetCurrentContext but that's a common way to do book keeping of per-context state, and avoids complicating UI classes with more plumbing. Think of it as TLS (thread-local-storage) that follows GL context lifecycle, and possibly migrates from one thread to another.

m-7761 commented 5 years ago

It's like TLS but for the UI objects. It's a simple system. I wouldn't make it any more complicated. A library has to have a strong sense of identity. That includes what it is, and what it is not. If more controls used textures it would be worthwhile to centralize their management, but I wouldn't likely export that framework to users... I'd leave them to implement their own if they are doing custom stuff with textures.

For the record, I don't think GLUT is a multi-thread paradigm. It has one main-loop. So neither would GLUI ever be so. I did mention making the possibility of making it thread-safe, but that's just an afterthought I had to cleanup the function signature for posterity sake.

The set method doesn't need to be pass by address. I left it that way by accident. But it kind of demonstrates the symmetry... each "get" would have a corresponding "set". Obviously you want controls to share textures, but they cannot share textures across contexts, unless glGenTextures shares resources across contexts, but even if so it's not reliable to assume so.

m-7761 commented 5 years ago

DELETED: I wrote from experience (GLUI Example #5) the contexts were leaking, so probably shared, but I realized glutSetWindow is basically changing contexts, and so I needed to use it to initialize my main OpenGL window. (I'm injecting the examples into an existing GLUT app for simplicity.) Seems obvious in hindsight.

m-7761 commented 5 years ago

Scrollbar draw_scroll also has a texture issue that could interact with this. But I think it should be replaced with a polygon-stipple effect, since unless that's nonportable it doesn't require a texture in this case.

nigels-com commented 5 years ago

Yeah, storing the texture ID per GLUI_Rotation makes sense here.

Assuming that the lifetime of a GLUI_Rotation is limited to one OpenGL context.

void GLUI_Rotation::setup_texture()
{
  static GLuint tex=0u;
  GLenum t=GL_TEXTURE_2D;
  glEnable(t);
  glTexEnvf( GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_MODULATE );
  glColor3f( 1.0, 1.0, 1.0 );
  if (tex!=0u) {
  /* (OSL 2006/06) Just use glBindTexture to avoid having to re-upload the whole checkerboard every frame. */
    glBindTexture(t,tex);
    return;
  } /* Else need to make a new checkerboard texture */
  glGenTextures(1,&tex);
  glBindTexture(t,tex);