OpenGLComponent request

General JUCE discussion
1

I’m not really satisfied with the context handling in this component, I think it’s too restrictive.
Currently, I’d like to be able to “activate” the context, query some OpenGL value, and “release” the context.
Unfortunately, it doesn’t work out this way, since it’s not possible to make it active if it doesn’t exist already.
I’ve to hack the visibility of updateContext(), but clearly, I think a ScopedContext class would be better suited (with a “const ScopedContext & getScopedContext()”).

Edit: In fact I don’t understand the way it’s changed in the module branch. This comments:
/**
When this callback happens, the context will have been made current
using the makeCurrentContextActive() method, so there’s no need to call it
again in your code.
*/
virtual void releaseOpenGLContext();

In the previous version, I used to call deleteContext() myself, tracking if it was done in a thread or not.
What should I do now ?

2

The trouble with just letting you activate/deactivate it yourself is that platforms are restrictive about which thread is allowed to do that… So since I’ve been moving towards a system where all the rendering happens on a background thread, I’m not really sure when you’d expect to do this?

But having said that, I’m also not sure whether the current design is ideal, so I’m totally open to suggestions about how it could be better.

3

Basically, I’m doing a dual step initialization where I’m querying values in the first step (like the maximum supported texture size for each format, the maximum number of vertices, and so on).
Then based on the “current openGL” capabilities, I’m selecting the best option (in my case, it’s the color format that’s the best approximate of the one I need), and setting up shaders.
I’m not doing any rendering work by that time, so starting up a rendering thread is overkill, and might actually not happen at all if I find out no color format I like.

So I need a way to actually create a context so I can call the OpenGL query functions, and release it after usage.
In pseudo code I need this:

   context.makeActive();
   glGetInfo[...]();
   context.makeInactive();

What about this ?

/** This perform some OpenGL calls with a ephemeral context.
     Use this if you need to extract specific informations out-of OpenGL's driver, but any state you'd set will be lost on returning from the function. */
class QueryOpenGLParameters
{
    /** Called with an ephemeral context that might or might not persist, so don't expect any change in OpenGL machine state to persist when returning the function */
    virtual void queryParameter(OpenGLContext & context) = 0;    
};

// And in OpenGLComponent:
void queryParameter(QueryOpenGLParameters & param);
4

Can’t you do all that stuff in the newOpenGLContextCreated() callback?

5

No. The context isn’t created until the first rendering happen. And for this rendering to happen, I need to extract information out of OpenGL in order to set up the color conversion filters, the decoding engine and so on.
So for me it’s not a solution, as it would required handling multiple case in the rendering method which I would like to avoid.

[edit] The only requirement is that the component isShowing() is true (Windows limitation).

6

Another issue I’m seeing right now is that all the context function are no more accessible.
So since I’m using my own render thread, I can’t use the OpenGLComponent anymore.

Would it be possible to add a “protected: virtual Thread * createRenderThread()” method to the OpenGLComponent, and make the other context stuff protected ?
The OpenGLRenderThread is a friend of OpenGLCompenent, and since we can’t control its lifetime, it’s a kind of unfair.

I don’t want the render thread to derive from OpenGLRenderThread either, since the render thread doesn’t actually require OpenGL at all.
Said differently, I want to be able to trigger rendering on the OpenGL component from the outside, how can I do it ?

7

It seems like you’re using it in an awkward way, and I’m really not sure what would be the best way of handling this. Can’t think about it right now, let me have a dig around in there when I’m next working on it, and will see what I can think of…

8

Not being able to trigger a context creation is awkward.

In any platform, you have the usual:

  1. create context
  2. perform OpenGL work
  3. destroy context

Since Juce doesn’t allow the step 1, we have to rely on hoping the framework will sometime call us back, but it’s not clear when it’ll do so and in what thread/state.
I think being able to trigger step 1 from outside is a minimum. If you think it’s too “hacky”, please at least make it protected so only “power” user will be able to do so.

Also, not being able to hook into the rendering thread is too restrictive, but can be worked against (even if it’s ugly for the code required to do so).
Being imposed on the refreshing rate is not a good design. I don’t want to render my scene at 60fps, if I’m doing video rendering work at 25fps.
Worst, I usually render my scene only on a specific event (if something changed in the scene for example), and with the current design I can’t do so.

Concerning the OpenGL rendering thread, you should at least allow the user to start his own thread (again, with a protected overload), or split the OpenGLRenderingThread to call “threadJustStarted(Context &)” before entering the main thread loop, so we can put initialization in there, “threadWait(Context &)” for the wait time, and so on.
Ideally, please change the newContextCreated method signature with a bool saying if it’s inside the rendering thread or not.

Having initialization in the “renderOpenGL()” method is ugly IMHO.
Same should happen for “threadAboutToStop(Context &)”.

In fact, the more I think about it, the more I think the current design is not clean. You’re trying to mix two different behaviour in a single interface.
You might prefer to have a “OpenGLComponent” and a “OpenGLThreadedComponent”.
In that case, the corresponding documentation should say:

/** In an OpenGLComponent, the context is usually created on-the-fly when first needed. 
     It can be recreated, in case the underlying peer require so.
     You can force context creation by calling the "createContext()" method in your child class, in case you actually need it.
     A timer can be started to trigger calling renderOpenGL(). 
     Buffer swaps are done automatically. */

/** In a OpenGLThreadComponent, due to some limitation with some platform, a context is first created on the message thread.
     Then, it's recreated again in the rendering thread.
     You can force context creation by calling the "createContext()" method in your child class, but you need to respect the rule above (else it'll probably fail).
     You need to provide a rendering thread that'll call renderOpenGL() at regular interval.
     Buffer swaps are done automatically */
9

Yes, I agree it needs some redesigning. Maybe there should be a separate OpenGLRenderingThread class which handles the context creation, and the component could use a default one of these internally, or you could supply your own for it to use.

10

Also, I think using assert as an error detection is bad.
For example, OpenGLShaderProgram::link documentation states:

    /** Links all the compiled shaders into a usable program.
        If your app is built in debug mode, this method will assert if the program
        fails to link correctly.
    */
    void link() noexcept;

In my opinion, link and addShader should return a bool saying if they fails. Some shader code might fail on some driver (limitation in the number of texture unit for example), yet, we have no way to find out if it failed (and no jassert is not a good way, since it disappear in Release mode).
Ideally, a “String getOutput(bool compile)” method should exists too so error reporting from the driver is not silently dropped, and can be sent back to the developper when the error happen on user’s computer.

11

How do you enumerate the supported pixel format in the new version ?

12

That wasn’t something I could find a way to do on all platforms, so I disabled it. I think probably the best way to do it is to just try whatever formats you want, in order of preference, and use the first one that gets accepted.

13

Ok. I get it. Thanks.

[edit] Well, in fact, no, I don’t get it.
In OpenGLComponent::setPixelFormat, there is nothing saying if the format is accepted.
Where should I get an indication if a format is accepted ?