Hi rdb,

do you have a link to game engine addon (if it is possible, of course) to test it with the modification proposed?

I fix this problem in my SoC patch. I do it by making sure the Game engine follows a reasable GL state setting policy and (maybe) calling the gpu reset function when done (don't remember if its needed, cause ideally it shouldn't be).

I could review my code and see if there were any questions/problems I couldn't fix due to lack of knowledge. I think having the game engine play less fast and loose with state is a better solution than a bulk state reset.

@Jorge Bernal (lordloki) Hmm, that would be tricky. It relies on changes to and a custom build of the game engine in question (Panda3D), and a bridge API I created to allow for C/C++ RenderEngine implementations. The build set-up is not straightforward. I could create binaries for your platform if you ask me to, but it'd take me a few days to get everything sorted out.

@Jason Wilkins (jwilkins) To an extent, I appreciate that third-party game engines shouldn't play fast and loose with the OpenGL state. But the very nature of OpenGL makes it impossible to require render engines to reset the state perfectly as it was. It is not possible to query or save the current state without introducing sync points or relying on deprecated functions like glPushAttrib. Making render engine developers guess Blender's favourite state isn't an option either, since that would break whenever someone changes which OpenGL features Blender is using, on top of the fact that some of Blender's OpenGL state relies on internal variables that the game engines don't have convenient access to (eg. the glOrtho calls for the axis indicator)

The problem is two-fold: engines not resetting their own state (ie. not setting glActiveTexture back to 0 or leaving GL_LIGHT7 on), engines resetting *too much* state (ie. disabling the stencil test that Blender relies on). I think it's quite reasonable to require a render engine to, say, unbind its active sampler objects (Panda3D already goes out of its way to reset the state to a reasonable default before handing off control of the context), but it shouldn't have to know exactly which specific GL_AMBIENT setting Blender needs to be left in.

In conclusion, I do think going some way to reset the state is imperative to support third-party render engines reliably, while still expecting the game engines to be somewhat reasonable (unbinding shaders, disabling sRGB framebuffer, etc).

Out of curiosity, do you have a link to the patch you mentioned? Would it affect any RenderEngine or were you only talking about the BGE?

@rdb (rdb) I was actually talking about BGE, but I guess things are more compkex. I still need to look at it.

In D1017#18023, @Campbell Barton (campbellbarton) wrote:

Think that this shouldn't run every time for every engine.

To support some engines where we can't ensure the GL state I think it would be better to have an engine flag so you can optionally reset the state, but well behaved engines don't have to set this.

I was thinking along the same lines, but I wasn't sure how the flag would be implemented or if it was an acceptable solution.

In D1017#18023, @Campbell Barton (campbellbarton) wrote:

Think that this shouldn't run every time for every engine.

To support some engines where we can't ensure the GL state I think it would be better to have an engine flag so you can optionally reset the state, but well behaved engines don't have to set this.

Campbell, thanks for looking at my patch.

Could you define a bit better what you mean by "well-behaved"? Surely an engine isn't "misbehaving" by changing the modelview transform or scissor region, and not setting it back to a matrix that Blender calculated internally (and an external engine would have no knowledge of).
The less state that we reset here, the more clearly we have to specify to the engine developers what the state should be reset to by the engine. I could narrow down these state changes to a more minimal set, but then there also needs to be a clearer specification on which exact state the engine should return to.

So I'd appreciate it if you could explain what you would expect a "well-behaved" engine to do.

@rdb (rdb), perhaps bad choice of words.

Current engines - Cycles, Blender-internal (assume others too), dont need this because they restore any important OpenGL state.

Also they aren't using OpenGL heavily, (just drawing pixels).

Nevertheless, having this optional seems reasonable, so the OpenGL state is only reset when its needed.

The example given, of a matrix, is actually an example where there is a well know convention of pushing and popping a stack on order to preserve state.

I've been compiling a set of what I call OpenGL policies, as well as patches to Blender so it obeys the policies.

I did run into some problems with glDepthTest because of complex conditions on how its used, but at some point it should all be made consistent at some level (say a higher level off a complete viewport call) even if it isn't on a functiin by function level.

@rdb (rdb), still not a great example because uniforms are a part of program state and if you implement your own matrix it wouldn't affect Blender and I don't think that Blender should make any assumptions about glUseProgram after calling a foreign renderer.

If you were somehow using Blenders programs then it would be bad manners to not push and pop using the matrix functions implemented on top of core gl for Blender.

There is actually not much state which should not be reset to the default after use, because changing that state is rare.

Blender mostly uses OpenGL defaults as its own defaults.

I'd propose that you restore to the OpenGL default anything that changed. Blender can take care of anything that needs to be different.

I'm actually not sure why the flag should only be on our end. Couldn't you implement a "be nice" flag in your engine that set certain values back to thier defaults?

In anycase, it us a rather small inefficiency to argue too much about.

@Jason Wilkins (jwilkins) the engine in question does have a "be nice" flag, which I already activated, and I did already commit patches to the engine that would help further with restoring to a reasonable 'default' state. I would not be going through the hassle of submitting this patch if that had sufficed. There are in fact many state settings that Blender depends on that aren't the OpenGL initial state, and worse, there are states that depend on internal Blender configuration that external renderers don't have access to.

To give some examples of the non-default state settings Blender uses: Blender's GPU_state_init sets the GL_AMBIENT material setting's RGB components to 0.0. The OpenGL initial is 0.2. It sets the shininess to 35. OpenGL initial shininess is 0. It sets the specular to 0.5. OpenGL initial specular is 0. Diffuse to 0.5, OpenGL initial is 0.8. DepthFunc GL_LEQUAL, initial is GL_LESS. Blender enables GL_NORMALIZE, but OpenGL disables it initially. I could go on.

I do appreciate your desire to more clearly specify which state applications should return to, and it might be possible to reduce the amount of state Blender resets a little bit. It would be a fair bit more work though to hunt down every little state that might be set, though, and far more work on the side of the engine developers who have to make sure that every little fiddly little state change is set back to the exact same setting.

Made several inline comments.

I didn't have source available (using my phone), so I might have been able to answer some concerns myself.

I am thinking the proper solution is to give a foreign renderer it's own gl context.

Otherwise, this is a maintanence nightmare.

Thanks for looking at the patch. I responded to your in-line comments.

I wholeheartedly agree about the fact that using a separate context with context sharing would be a superior solution, although I haven't explored it in depth yet. Ideally I'd like Blender to have some sort of mechanism where Blender passes me a GLX/WGL context handle, so that I can create my own context with sharing enabled; then, for view_draw, I'd like Blender to just take a shared GL texture handle so that Blender could be doing all the drawing into its own context. I would imagine that it'd require a bunch of non-trivial changes to Blender's RenderEngine system, though.

following up,

I'm going to investigate providing a context to isolate state changes.

Do you need sharing? If so I'd only be worried about resource leaks. If not then you'd have a clean context to do whatever.

I figured the sharing would be necessary to get the rendered image back to Blender, but you are proposing instead that the second context would just render straight into the Blender window as well? If it works, sounds fine to me.

By sharing I mean objects created in one context are available in the other.

I thought about it again and realized resource leaks are a concern regardless of if a renderer uses the same context or if objects are shared.

I was looking at how FreeGLUT handles what it calls "subwindows", which have an option to be created with their own context, even though they are child windows of a top-level window (or of another subwindow).

Turns out that FreeGLUT creates an entirely new child window. I don't think this step is needed, unless one wants to be really strict about pixel ownership. Would probably also be fairly buggy because there'd have to be code added to GHOST and Blender to keep the child window in the right place. (Although it might all be restricted to region code).

However, it should be possible to create a new context for the game engine and just call MakeCurrent when it's time for the engine to draw.

Any engine will have to make sure to clean up all of it's resources when done if it is going to share textures/etc with Blender, otherwise it will leak into Blender every time it is stopped and started (I'm assuming the game engine does full init and exit each time it's invoked, but I might be wrong).

I do not think it should be too difficult to add some functions to the GHOST and WM to create sub-windows (with or without a new OS child window behind it).

(There might also be use in having separate child windows because it would allow for OpenGL rendering in a separate thread that can bypass Blender's message loop to lower latency, but I digress)

The implementation of BGE would benefit from having its own context I think, because then it could diverge from Blender in how it handles OpenGL state, which might be more convenient or efficient.

If BGE was given its own context then there wouldn't need to be a new user flag either.

Panda3D certainly supports creating a subwindow that is parented to an existing window, and it sounds like it could be one solution. (The only thing to be worried about is when an incompatible window system is used. The only example that I know of of that being a problem is Carbon vs Cocoa on Mac.) This approach might even allow the render engine to use a different graphics back-end ie. Direct3D, although this is not important for us specifically.

If you went by that route, it would be nice if you let the application create their own sub-window, but exposed the OS-specific window handle (HWND, X11 Window handle, NSView pointer, whatever) via some API. We already support that case very well, since that's how eg. we render into a browser window in our web plug-in.

If as you say it is possible to simply render into the same window after switching the context with MakeCurrent, that would already solve the problem fairly neatly by itself, I think.

(One consideration that we may want to leave open is the ability for us to make the context current in our own thread so that we could draw into it in parallel with Blender. This might be easiest with context sharing so that we could render into a texture in a separate thread, and then indicate to Blender that the texture is ready for being drawn into the Blender viewport while the Panda thread starts rendering the next frame into a different texture. This might become more difficult or expensive if you want to switch the contexts at every view_draw(). I can't say I have ever tried something like this, though, so I might just be talking nonsense.)

First of all, why do we need to clean up the state *after* the render engine done it's business? It kinda makes sense for the render engine to re-set all changes in the state it did.

It's also kinda dangerous, because external render engine might depends on specific state and changing this state might ruin behavior of the render engine.

Also, not really sure it need to be a flag in RenderEngine. This it's pretty much easy to do without any modifications, like this:

class MyRenderEngine(bpy.types.RenderEngine):
    def view_draw(...):
        do_actual_render_engine_draw()
        bpy.bgl.setMatrixMode()
        # do other state reset

We could probably have some bpy.bgl.state_reset() which could also be handy for BGE guys?

@Jason Wilkins (jwilkins) I'm willing to abandon this revision in favour of an alternative approach using a separate context, which does seem more appealing and maintainable in the long run. I'm wondering if you're still willing to check in at least the ED_region_pixelspace() call, though, which probably gives me enough to work with in the interim (although it would probably also be a good idea to place that glDisable(GL_COLOR_MATERIAL) in the right position, or alternatively move the glMaterialfv block further down in that function, purely because it seems like something that might cause bugs elsewhere in Blender). I can work around anything else.

(What might be nicer than having a reset_gl_state flag would be to just expose the GPU_state_init() function itself via the bgl API, so that the render engine can call that before handing off control of the context back to Blender; this seems more elegant than having a magical flag on the renderer.)

@Sergey Sharybin (sergey) I'm not really sure what you mean. You realise we're talking about an external render engine, right, and not the BGE?

I've already addressed several of the points you mention in earlier comments. Not all state changes can be reset by the external render engine, since the external render engine doesn't and shouldn't know some of the more specific state settings needed to restore to the state used for rendering UI, and I've given some examples of this in earlier comments in this thread.

I'm create a patch that allows an external engine to use a separate context, but please feel free to send me a personal email if it looks like I've forgotten about it.

I think it will just involve some minor modifications to GHOST and the WM.

I'll need to look at the glColorMaterial changes more closely. I have noticed that NVidia and AMD seem to have different ideas of how to handle that, so changing it might break something (because it depends on a bug).

@rdb (rdb), sure i understand you're talking about external render engine. it does not cancel the fact you can do bgl bindings from the engine class.