search

bonsai-rx / bonsai

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

Consider binding textures to their uniform slots rather than texture units #1776

Open PathogenDavid opened 4 months ago

PathogenDavid commented 4 months ago

As mentioned in https://github.com/bonsai-rx/bonsai/issues/1773, textures in Bonsai conceptually bind to specific shaders. However the API still has you bind to OpenGL texture units (or TextureSlot in Bonsai parlance) rather than to uniforms within the shader.

I don't think it's helpful to expose this aspect of OpenGL (and it's very much an OpenGL-ism.) Realistically I think most people would want to bind the texture to a specific named sampler uniform in their shader, and Bonsai can allocate texture units on its own as it sees fit.

This would also allow automatic inference (or at least validation) that the texture target matches the sampler stated in the shader.

The current paradigm is somewhat error-prone since really people should be using BindTexture and UpdateUniform together to bind textures.

In fact it's being overlooked in the St. Andrews tutorial and I'd forgotten myself when I was creating the minimal repros for my recent texture-related issues.

You can technically get away with skipping the UpdateUniform if the uniform is still the default 0 and the texture is bound to slot 0 (this is why the St. Andrews worksheet and my repros still work), but in some ways this does more harm than good because it lets people get away with not realizing they need to do it whenever there's more than one texture.

Possible Solution

Naively implemented, this would be a breaking change. However I think a non-breaking changed (or at least one which can be migrated automatically) is possible. We could have it so that upon migration: The TextureSlot becomes the UniformName, and allow special case behavior for those magic strings to only set texture units and skip the uniforms.

However this creates a hairy situation when both modern UniformName and legacy TextureSlot-style UniformName. My gut says it's best to just disallow this scenario (and encourage people to migrate to the modern setup), but we could make sure that the allocation strategy of modern binds avoids using slots used by legacy binds. (The easy allocation strategy would just be to allocate them linearly as they appear in the shader though, so this adds a lot of complexity.)

The solution to this would likely influence how https://github.com/bonsai-rx/bonsai/issues/1773 is done. (However I'm fairly confident we could make a decision later if we decide it 1773 should happen before this.)

glopesdev commented 4 months ago

@PathogenDavid This is a good idea, and indeed something I was always keen to allow. I think there were a couple of reasons which made me hesitate to enforce this abstraction, having to do with cases where it was useful to really bind directly to texture slot, but I would need to dive in my earlier notes to confirm this and remember what they were.

In terms of making this a non-breaking change, one option would be to introduce a new UniformName property, make TextureSlot nullable (and possibly hide it from the property grid). We would then make UniformName simply take precedence over TextureSlot whenever it is specified. This would allow us to gradually phase-out TextureSlot while keeping all old workflows running (they would default to an empty UniformName in this case).

Happy to sit down and think again on the architecture for this, or if you have a proposal also happy to review and discuss it.