Consistently use system_fbo instead of binding 0 as it is needed for iOS devices

[clayjohn]

Fixes: #86830

system_fbo is always 0 except on iOS where it is given a value. On iOS, binding to 0 breaks subsequent rendering commands. So we should never be binding to 0.

#86830 (comment) suggests that the issue was introduced by a commit that added an extra binding to 0. The OP confirmed that just changing that one instance of binding to 0 wasn't enough (#86830 (comment)) however, new binds to 0 have been introduced since that comment was made, so its possible that the issue won't be fixed until we have replaced it everywhere, which this PR does.

edit, the OP confirmed that this fixes their issue

Even if this doesn't fix #86830, we still need to merge this PR as we should never be binding the Framebuffer to 0

Note for maintainers, this PR can't be cherrypicked, but if it is confirmed to work, we need to back port it to 4.2

[dsnopek]

I'm not sure this is exactly right. In some cases we bind to framebuffer 0 in order to draw to the screen (the system FBO) and sometimes we do it to clear out the framebuffer because the shader we're running shouldn't have one (like in the case of the particle shader, where having a proper framebuffer can break things on GLES, since it's a non-rendering shader).

I think this might be the cause of the issue from this comment:

I did a little bit more complex test and it crashes when drawing particles.

I think in the case of particles (which is what I was fixing with PR #83756 originally), we probably want to keep binding the framebuffer to 0, but then when we're done, bind back to GLES3::TextureStorage::system_fbo. (Or, posssibly even better, binding to GLES3::TextureStorage::system_fbo right before we need to do some actual rendering? I don't know that we want these cases where we're assuming the framebuffer is already the system FBO, as opposed to explicitly setting it?)

I don't know if there are more cases like this aside from particles.

[clayjohn]

I'm not sure this is exactly right. In some cases we bind to framebuffer 0 in order to draw to the screen (the system FBO) and sometimes we do it to clear out the framebuffer because the shader we're running shouldn't have one (like in the case of the particle shader, where having a proper framebuffer can break things on GLES, since it's a non-rendering shader).

I think this might be the cause of the issue from this comment:

I did a little bit more complex test and it crashes when drawing particles.

I think in the case of particles (which is what I was fixing with PR #83756 originally), we probably want to keep binding the framebuffer to 0, but then when we're done, bind back to GLES3::TextureStorage::system_fbo. (Or, posssibly even better, binding to GLES3::TextureStorage::system_fbo right before we need to do some actual rendering? I don't know that we want these cases where we're assuming the framebuffer is already the system FBO, as opposed to explicitly setting it?)

I don't know if there are more cases like this aside from particles.

Hmmm, that is an interesting theory and it makes total sense to me. I don't know enough about the GL drivers on iOS to be sure if it's true or not though. On non-iOS devices 0 is the system FBO. Accordingly there is no ability to bind to a state where no FBO is bound.

I guess we should try binding 0 before doing any transform feedback and then always bind to system_fbo after transform feedback. My understanding is the original issue comes from binding to 0 and leaving that bound in subsequent draw operations.

[clayjohn]

@dsnopek I've force pushed a change to ensure that we bind FBO 0 before transform feedback operations and we bind the system_fbo after

[clayjohn]

@dsnopek See the comment here #86830 (comment)

Looks like binding to 0 breaks things all over again.

Particles are broken on iOS in 4.2.1, so that wasn't a regression from this PR.

I've reverted the changes back to consistently using system_fbo. Also, remember that on non-iOS devices system_fbo is always 0. So there is no functional difference between binding to system_fbo and binding to 0 manually on those devices.

[dsnopek]

Sounds good! The idea was worth a try :-)

[BastiaanOlij]

lgtm

[akien-mga]

Thanks!