bonsai-rx / bonsai

The compiler, IDE, and standard library for the Bonsai visual programming language for reactive systems
https://bonsai-rx.org
MIT License
128 stars 26 forks source link

Unrelated texture operations mess up texture bindings #1773

Open PathogenDavid opened 2 weeks ago

PathogenDavid commented 2 weeks ago

The API that Bonsai presents for binding textures implies that the texture will be durably bound to the specified shader, however OpenGL textures slots are global application state.

Binding a shader essentially just schedules a glBindTexture to be called when the shader is bound for rendering.

This means texture bindings for a given shader will change unexpectedly in situations where a texture slot is used for something else and nothing in the workflow is constantly scheduling a rebind for the shader.

Repro

To reproduce this issue:

  1. Download TextureOverrideIssue.zip and open the enclosed workflow
  2. Run the workflow, you should see a single freeze-frame from your webcam rendered to the screen
  3. Stop the workflow and enable the UpdateTexture node (Note that this node is updating the TestDynamic texture, which is not bound by anything within the workflow.)
  4. Run the workflow, you'll now see that the live view of your webcam is rendered to the screen

This happens because the UpdateTexture node has to bind the texture in order to update it. This texture hangs around in the texture slot for the remainder of the run since the delegate scheduled by the BindTexture node is executed only once ever.

Proposed fix

I haven't investigated thoroughly, but I think ideally rather than blindly scheduling Actions to be executed when the shader is bound, we should instead maintain a proper table of texture bindings and ensure they're rebound every time the shader is bound.

As an aside, I find the naming of shaders to be a bit unfortunate since I think the term "material" more accurately represents the abstraction they provide. (The ShaderResources node even calls them materials.)

glopesdev commented 2 weeks ago

@PathogenDavid this is an unfortunate consequence of OpenGL itself being a globally accessible mutable state machine. It is not just texture bindings interacting, but pretty much everything that you can possibly do with OpenGL, including changing render states, enabling / disabling blending, changing point sizes, viewports, etc.

I tried quite a few different designs when the Shaders package was first conceived, and in some of them the operators would try to restore global state to some reasonable neutral default to avoid leaving traces. It turns out this is actually frowned upon in several OpenGL circles concerned with high-performance rendering, where you want to minimize global state changes whenever possible, so we have since moved away from this towards a design where the user of the package is responsible for setting every state that needs changing, whenever necessary.

Shaders is trying to strike a probably impossible middle ground between a game engine and having to write all the OpenGL boilerplate for everything by hand. It is in theory a package for building game engines, rather than a game engine itself. There are only two rough design principles: 1) minimize boilerplate; 2) capture reusable patterns for "useful nuggets of work", such as running a shader, setting up a framebuffer for render to texture, etc. but without trying to place too many assumptions on how the global state machine is supposed to be used.

The idea was somewhat that this package would not be used directly most of the times other than for simple, direct access stuff, but rather would provide a foundation for other high-level engines such as BonVision or the Shaders.Rendering package.

The reason for specifying everything as an Action is a reflection of this in that it tries to capture the minimal pattern required to do anything in OpenGL itself with the least assumptions possible. Since OpenGL is just globally accessible everywhere, the only thing you really need is to make the right sequence of calls in the thread owning the current GL context, so essentially a scheduler, roughly equivalent to WinForms' BeginInvoke.

This simple approach did allow us to build some pretty interesting and powerful abstractions using a reasonably small architecture, even if they probably look strange to both OpenGL and render engine developers. We could have provided more principled, opinionated, and sanitized abstractions, but the goal was more to try and expose a minimal boilerplate version of OpenGL 4, warts and all. That said, the fact that the package is still in 0.27.0 version tells you a lot about how happy I am with the API 😅

The point about the name "shaders" is well taken, although to be fair the kind of programs you can register with the manager includes not only materials, but also compute shaders, which imply compiling a different pipeline altogether. If we went strictly with OpenGL parlance, the correct name might be Program, since the common thing that unites all of them is you need to call GL.CreateProgram, but I found that term too generic.

I'm not opposed to completely redesigning the shaders package, or possibly even creating an entirely new package altogether. Especially when we plan on migrating to the new OpenTK or any other .NET Core compatible backend we are sure to need breaking changes, but most likely for the .NET Framework world this slightly wonky Shaders package will remain as a lesson on how not to build a game engine.