Each target with its own context

[Groogy]

Hey! Long time since I've been posting here but if I am gonna return might as well return with some nagging

Anyway so been playing around with SFML lately on my free time and noticed that each render target is its own OpenGL Context. I think it has always been like that but I noticed an issue with it because of it. Because of them being their own individual context it means that you can't share certain buffers between the different render targets, stuff like VBO's.

Wondering if that could be up to debate for changing in the near future? There isn't really any reason as far as I can see that the FBO's under the render texture can't rely on the Window's opengl context. If I remember correctly when render texture was first implemented this was done as a lazy way to ensure there will always be an active opengl context. Though Laurent will have to correct me on that one since that's years ago.

[binary1248]

Because of them being their own individual context it means that you can't share certain buffers between the different render targets, stuff like VBO's.

All OpenGL buffer objects are shareable between contexts. I've done this many times myself. The only commonly used objects that aren't shareable between contexts are FBOs.

Wondering if that could be up to debate for changing in the near future? There isn't really any reason as far as I can see that the FBO's under the render texture can't rely on the Window's opengl context. If I remember correctly when render texture was first implemented this was done as a lazy way to ensure there will always be an active opengl context.

It is true that because RenderTextures live in their own contexts they can remain bound forever, however that is not the main "advantage" of this implementation. OpenGL contexts can only be active in a single thread at a time. If you try to activate a context that is already active in another thread, activation will fail and you will not be able to perform OpenGL operations. Letting the RenderTexture live in its own context makes it easier to not have to care about which thread a user draws from. If it were to share the same context as some other window, SFML would have to perform inter-thread communication and synchronization in order to get the other thread to deactivate before the RenderTexture's context could activate on its own thread.

Yes... it is possible for SFML to run in a single context mode when the user only does rendering from a single thread, but given the flexibility SFML promises the user at this point and the implementation that is in place to make it happen, either you have to break the promise and make the library harder to use in general, or you would have to come up with a really complex implementation that would be harder to maintain and even have an impact on performance in edge cases.

I already made the first step on the way to reducing the number of contexts an SFML application ends up creating. You can find the associated thread here. As usual with these kinds of changes, it is stuck in testing hell and might take a while to get accepted.

[Groogy]

you can't share VBO's either, which is a problem because it means in my experiment I have one VBO of the same data for each RenderTexture it is going to be rendered to.

And yes you would need to have one context per thread but that is achievable by using a global thread-local(lol) variable than by having several contexts per thread. Everything you described is still achieved and nothing lost. You still won't be able to bind buffers(VBO, FBO, etc.) that are not associated with the current active context.

The idea of that the render texture would guarantee thread safety is not true, OpenGL does that anyway all we have to guarantee is that a context exists and the user won't notice the difference. The only "promise" that would be broken is that the interface is made simpler since the activate function becomes obsolete.


All the code to make it work is already in place in the library and just top of my head something in style with this should be simple enough and eliminate need of FBO specific contexts making the code simpler to maintain but still provide the same flexibility as before where the user can have his own contexts that are not created by render targets.

// C++11 specific example
#include <threads.h>
thread_local sf::Context globalThreadContext;

void ensureGLContext() // called by functions that need an active context
{
    sf::Context* activeContext = sf::Context::getActiveContext();
    if(activeContext == nullptr)
        globalThreadContext.setActive();  
}
 

edit: realized the active context function would have to be a thread-local one as well, which it probably is already I hope? Or else it's kinda bad since it doesn't mimic OpenGL behavior then.
edit2: Yes it is with a SFML's own thread local pointer object.


Also with this you get the added choice of the user deciding what context to use at all times. If he want he can keep one context for the entire application regardless of threads. (create everything in main thread when loading then switch context ownership to render thread, etc.) The current system kinda forces you to work around the system and is quite rigid.

[binary1248]

you can't share VBO's either, which is a problem because it means in my experiment I have one VBO of the same data for each RenderTexture it is going to be rendered to.

I don't get why you think VBOs aren't shareable... Like I said, I've done it many times and it is documented all over the internet, such as here. If binding a VBO in a different context is failing for you, then I can only assume there is something wrong with your code.

And yes you would need to have one context per thread but that is achievable by using a global thread-local(lol) variable than by having several contexts per thread. Everything you described is still achieved and nothing lost.

So... how are thread-local contexts supposed to support drawing to a RenderTexture in a different thread than the one it was created in? SFML in its current form already requires the user to manually deactivate the RenderTexture's context before moving it to another thread. Your implementation just prohibits moving it at all. I don't see how this is supposed to be better...

You still won't be able to bind buffers(VBO, FBO, etc.) that are not associated with the current active context.

Like I already said above, VBOs are shareable, FBOs aren't. And I don't understand what you mean with "associated with the current active context". If you use an OpenGL debugger, it might list an object as living in a specific context's space, but this doesn't mean that it is "associated" to that context in any way. When you make 2 empty contexts share their name space with each other, an object created in one context is automatically available in the other because they share a common space. There is no association of object to context unless you are talking about unshareable objects. You can bind an object to a binding point in a context, and these binding points are local to each context, but this doesn't make the object itself local to the context.

The idea of that the render texture would guarantee thread safety is not true, OpenGL does that anyway all we have to guarantee is that a context exists and the user won't notice the difference.

This isn't about thread safety. You can try to activate the same context in 2 threads, and those calls will be "thread-safe" if you can call it that. It's just that the activation call that takes place later will fail. How the driver synchronizes access to shareable objects that are accessed from different threads simultaneously is another story.

The only "promise" that would be broken is that the interface is made simpler since the activate function becomes obsolete.

How exactly is the user going to manually activate contexts for OpenGL operations in secondary threads like that? This completely breaks the feature. If you are suggesting that every thread automatically create a thread-local context whether or not rendering is actually done, I hope you are aware of the ludicrous overhead this would cause even in well-written multi-threaded applications.

All the code to make it work is already in place in the library and just top of my head something in style with this should be simple enough and eliminate need of FBO specific contexts making the code simpler to maintain but still provide the same flexibility as before where the user can have his own contexts that are not created by render targets.

// C++11 specific example
#include <threads.h>
thread_local sf::Context globalThreadContext;

void ensureGLContext() // called by functions that need an active context
{
    sf::Context* activeContext = sf::Context::getActiveContext();
    if(activeContext == nullptr)
        globalThreadContext.setActive();  
}
 

edit: realized the active context function would have to be a thread-local one as well, which it probably is already I hope? Or else it's kinda bad since it doesn't mimic OpenGL behavior then.
edit2: Yes it is with a SFML's own thread local pointer object.

This just makes the problem I tried to fix in my PR much worse...

I sincerely suggest you think this through a bit more...

[Groogy]

you can't share VBO's either, which is a problem because it means in my experiment I have one VBO of the same data for each RenderTexture it is going to be rendered to.

I don't get why you think VBOs aren't shareable... Like I said, I've done it many times and it is documented all over the internet, such as here. If binding a VBO in a different context is failing for you, then I can only assume there is something wrong with your code.

From that page: Any object sharing must be made explicitly, either as the context is created or before a newly created context creates any objects.

edit: Believe the explicit sharing between contexts are done with the wglShareLists in Windows and the other OSes has their own functions for it.

And with the current implementation of SFML if I attempt rendering a VBO created in the Windows context on a RenderTexture it fails on the binding because it has not been explicitly shared. My post renderer is an example of where I encountered the problem. If I do not create a VBO for the currently active context, it simply will refuse to bind the VBO. Also notice how I have to fumble around with activate A LOT to make this work in a frankly quite simple 2D application.

(click to show/hide)

class PostRenderer
  def self.initialize
    @@copy = Native::Effect.new()
    @@copy.load_from_file("resources/shaders/copy.shader")
  end

  def self.initialize_for(buffer)
    layout = Native::VertexLayout.new()
    layout << {:size => 2, :type => Float, :stride => 16, :offset => 0}
    layout << {:size => 2, :type => Float, :stride => 16, :offset => 8}

    display_quad = Native::VertexBufferObject.new(layout)
    display_quad.primitive = Native::TriangleStrip
    display_quad << [ -1.0, -1.0, 0.0, 0.0 ]
    display_quad << [ 1.0, -1.0, 1.0, 0.0 ]
    display_quad << [ -1.0, 1.0, 0.0, 1.0 ]
    display_quad << [ 1.0, 1.0, 1.0, 1.0 ]

    @@display_quads = {} if !defined?(@@display_quads)
    @@display_quads[buffer] = display_quad
    return display_quad
  end

  def self.initialize_shaders(window_size, supersample, downsample, tonemap, luminance, ambient_blur)
    @@supersample = supersample
    @@downsample = downsample
    @@tonemap = tonemap
    @@luminance = luminance
    @@ambient_blur = ambient_blur

    @@tonemap.set_parameter("exposure", 0.45)
    @@tonemap.set_parameter("exposureBias", 2.0)
    @@tonemap.set_parameter("luminanceBias", 1.0)

    @@ambient_blur.set_parameter("blurStrength", 5.0)

    @@tonemapping_buffer = SFML::RenderTexture.new(window_size.x, window_size.y, false)
    setup_texture(@@tonemapping_buffer)

    @@luminance_buffer = SFML::RenderTexture.new(window_size.x, window_size.y, false)
    setup_texture(@@luminance_buffer)

    @@blur_buffer = SFML::RenderTexture.new(window_size.x, window_size.y, false)
    setup_texture(@@blur_buffer)

    @@downsample_buffers = [
      SFML::RenderTexture.new(1, 1, false),
      SFML::RenderTexture.new(4, 4, false),
      SFML::RenderTexture.new(16, 16, false),
      SFML::RenderTexture.new(64, 64, false),
      SFML::RenderTexture.new(256, 256, false),
      SFML::RenderTexture.new(1024, 1024, false)
    ]

    setup_texture(@@downsample_buffers[0])
    setup_texture(@@downsample_buffers[1])
    setup_texture(@@downsample_buffers[2])
    setup_texture(@@downsample_buffers[3])
    setup_texture(@@downsample_buffers[4])
    setup_texture(@@downsample_buffers[5])
  end

  def self.setup_texture(texture)
    texture.active = true
    Native.make_hdr_texture(texture.texture)

    test = SFML::RectangleShape.new()
    texture.draw(test)
    initialize_for(texture)
  end

  def self.get_quad_for(buffer)
    if defined?(@@display_quads) && @@display_quads.has_key?(buffer)
      return @@display_quads[buffer]
    else
      return self.initialize_for(buffer)
    end
  end

  def self.exposure=(val)
    @@tonemap.set_parameter("exposure", val.to_f)
  end

  def self.exposure_bias=(val)
    @@tonemap.set_parameter("exposureBias", val.to_f)
  end

  def self.luminance=(val)
    @@tonemap.set_parameter("luminanceBias", val.to_f)
  end

  def self.copy(target, source)
    @@copy.set_parameter("sourceSampler", source)
    self.render(@@copy, target)
    target.display()
  end

  def self.supersample(target, source)
    @@supersample.set_parameter("sourceSampler", source)
    @@supersample.set_parameter("sourceTextureSize", source.size)
    self.render(@@supersample, target)
  end

  def self.tonemap(target)
    @@tonemapping_buffer.active = true
    self.copy(@@tonemapping_buffer, target.texture)

    @@luminance_buffer.active = true
    self.calc_luminance(@@luminance_buffer, target.texture)

    luminance_texture = self.downsample_to(1, @@luminance_buffer.texture)

    target.active = true
    @@tonemap.set_parameter("luminanceSampler", luminance_texture)
    @@tonemap.set_parameter("sourceSampler", @@tonemapping_buffer.texture)
    self.render(@@tonemap, target)
    target.display()
  end

  def self.calc_luminance(target, source)
    @@luminance.set_parameter("sourceSampler", source)
    self.render(@@luminance, target)
    target.display
  end

  def self.downsample_to(target_size, source)
    start_index = @@downsample_buffers.size - 1
    while start_index > 0 && @@downsample_buffers[start_index].size.x > source.size.x
      start_index -= 1
    end

    downsample(@@downsample_buffers[start_index], source)

    end_index = start_index
    (start_index-1).downto(0).each do |index|
      source = @@downsample_buffers[index+1]
      target = @@downsample_buffers[index]
      break if target.size.x < target_size

      downsample(target, source.texture)

      end_index = index
    end
    @@downsample_buffers[end_index].texture
  end

  def self.downsample(target, source)
    target.active = true
    @@downsample.set_parameter("sourceSampler", source)
    @@downsample.set_parameter("sourceTextureSize", source.size)
    self.render(@@downsample, target)
    target.display()
  end

  def self.blur_ambient(target)
    @@blur_buffer.active = true
    @@ambient_blur.set_parameter("sourceSampler", target.texture)
    @@ambient_blur.set_parameter("sourceTextureSize", target.size)
    @@ambient_blur.set_parameter("blurDirection", SFML::Vector2.new(1.0, 0.0))
    self.render(@@ambient_blur, @@blur_buffer)
    @@blur_buffer.display

    target.active = true
    @@ambient_blur.set_parameter("sourceSampler", @@blur_buffer.texture)
    @@ambient_blur.set_parameter("blurDirection", SFML::Vector2.new(0.0, 1.0))
    self.render(@@ambient_blur, target)
  end

  def self.render(effect, target)
    quad = get_quad_for(target)
    quad.effect = effect
    target.draw(quad)
  end
end
 

So... how are thread-local contexts supposed to support drawing to a RenderTexture in a different thread than the one it was created in? SFML in its current form already requires the user to manually deactivate the RenderTexture's context before moving it to another thread. Your implementation just prohibits moving it at all. I don't see how this is supposed to be better...

This just makes the problem I tried to fix in my PR much worse...

I sincerely suggest you think this through a bit more...

It really doesn't, I remember back in the day I helped a ton of people because of the current "hidden behind" system of contexts messed stuff up for them. My suggestion would ensure an ad-hoc context(which currently is what we have but really bad) to make sure it always works but still allow the user to himself define the context he wants to work with himself because if he is doing that he is probably going to know better than the library does what he wants to do with his context. So So with my example it would guarantee and create a more flexible interface and management of contexts.

Example of creating a render texture and rendering to it in a different thread. The main point of the change is that it will make the user aware of contexts and their relationship with threads which right now is hidden. But like you say yourself, they themselves must still be aware that they need to disable a context in old thread before they are allowed to use it in the new one.

int threadfunc()
{
  threadData->aRandomContext.setActive();
  // do graphical stuff
}

int main()
{
    // Setup application and stuff
   aRandomContext.setActive(true);
   // Do graphical setup stuff
   aRandomContext.setActive(false);
   // start thread
}
 

If the user messes up and forgets to disable the context, he would still end up in the exact same state as with how it works today. The difference here is that he is explicitly made aware of contexts relationship with the thread and if he wants to do a multi-pass post process of the scene then he can do that without constantly switching contexts as well. Or the user just don't care and rely on the underlying system like always.


edit: I am kinda detecting a snarky tone with the whole "create a context for every thread even if no rendering is done" is kinda unnecessary hyperbole. Is my proposition threatening you somehow? For one I gave a quick example to motivate the behavior I found desirable by the library and not the actual implementation of that behavior. Obviously you would lazy allocate the context and your remark can only be seen as an active attempt of degrading my argument. If it is going to be like that we might as well end the conversation. I'll implement my own branch then and keep it to myself.