Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misbehavior (with fix) on iOS when using FrameBuffer #7297

Open
1 of 6 tasks
b3nk4n opened this issue Dec 9, 2023 · 5 comments
Open
1 of 6 tasks

Misbehavior (with fix) on iOS when using FrameBuffer #7297

b3nk4n opened this issue Dec 9, 2023 · 5 comments
Milestone

Comments

@b3nk4n
Copy link

b3nk4n commented Dec 9, 2023

Issue details

I've been facing an issue on iOS only when using FrameBuffer. When I ported my game October Bro from Android/Desktop to iOS, I noticed that my game screen was just black (or just the screen-clear color I used).

After longer investigation and digging into the LibGdx code, I could figure out the problem, and come up with a workaround. And my hope is that this issue report turns into a simple fix PR (either by me or by you), so that my workaround will not be needed anymore.

Reproduction steps/code

Use the following code on iOS, and toggle the FIX flag.

public class MyGdxGame extends ApplicationAdapter {

	private static final boolean FIX = true; // <-- please toggle me! :)

	SpriteBatch batch;
	Texture img;

	FrameBuffer frameBuffer;

	OrthographicCamera camera;

	boolean created = false;

	@Override
	public void create () {
		batch = new SpriteBatch();
		img = new Texture("badlogic.jpg");

		camera = new OrthographicCamera();

		created = true;
		frameBuffer = new FrameBuffer(Pixmap.Format.RGBA8888, Gdx.graphics.getWidth(), Gdx.graphics.getHeight(), false);
	}

	@Override
	public void render () {
		Gdx.gl.glClearColor(1f, 0f, 0f, 1);
		Gdx.gl.glClear(GL20.GL_COLOR_BUFFER_BIT);

		IntBuffer oldFbo = BufferUtils.newIntBuffer(1);
		Gdx.gl.glGetIntegerv(GL_FRAMEBUFFER_BINDING, oldFbo);

		frameBuffer.begin();

		Gdx.gl.glClearColor(0f, 1f, 1f, 1);
		Gdx.gl.glClear(GL20.GL_COLOR_BUFFER_BIT);

		camera.setToOrtho(true, frameBuffer.getWidth(), frameBuffer.getHeight());
		camera.update();

		Gdx.gl.glClearColor(1.0f, 1.0f, 1.0f, 255 / 255.0f);

		batch.begin();
		batch.draw(img, 0, 0);
		batch.end();

		frameBuffer.end();

		if (FIX) {
			// use the previous back buffer index, instead of defaultFramebufferHandle from GLFrameBuffer#unbind()
			int fboIdx = oldFbo.get();
			Gdx.gl.glBindFramebuffer(GL_FRAMEBUFFER, fboIdx);
		}

		camera.setToOrtho(true, Gdx.graphics.getWidth(), Gdx.graphics.getHeight());
		camera.update();

		// render pixel-art buffer to screen
		batch.setProjectionMatrix(camera.combined);
		batch.begin();
		batch.draw(frameBuffer.getColorBufferTexture(),
				0, 0, 0, 0, Gdx.graphics.getWidth() / 2f, Gdx.graphics.getHeight() / 2f, 1f, 1f, 0f, 0, 0,
				frameBuffer.getWidth(), frameBuffer.getHeight(), false, true);
		batch.end();
	}

	@Override
	public void dispose () {
		img.dispose();
		batch.dispose();
		if (frameBuffer != null) {
			frameBuffer.dispose();
		}
	}
}

The problematic code seems to come from the following section, which seems to even try to exactly this iOS specific problem according to the comment:

// iOS uses a different framebuffer handle! (not necessarily 0)
if (!defaultFramebufferHandleInitialized) {
defaultFramebufferHandleInitialized = true;
if (Gdx.app.getType() == ApplicationType.iOS) {
IntBuffer intbuf = ByteBuffer.allocateDirect(16 * Integer.SIZE / 8).order(ByteOrder.nativeOrder()).asIntBuffer();
gl.glGetIntegerv(GL20.GL_FRAMEBUFFER_BINDING, intbuf);
defaultFramebufferHandle = intbuf.get(0);
} else {
defaultFramebufferHandle = 0;
}
}

However, while testing and debugging this, I could observe the following:

  • As written in these lines, the screen is always fboIdx = defaultFramebufferHandle = 0 on Desktop/Android, but usually fboIdx = 1 on iOS
  • However, that might depend on the game. Or on the point in time when the new FrameBuffer(...) constructor is called for the first time. I observed on my single iOS game that the very first time the constructor is called, that the stored fboIndex is first 0, and later it would be 1 on iOS.
  • The fact that via the static defaultFramebufferHandleInitialized the initialization of the defaultFramebufferHandle is only done once causes the problem that we might try to go back to the wrong index on FrameBuffer#end(). As shown in the above code example.

TL;DR Workaround

Summarizing my current workaround in a brief from above, the problem can be prevented by wrapping the frameBuffer begin/end function with:

IntBuffer oldFbo = BufferUtils.newIntBuffer(1);
Gdx.gl.glGetIntegerv(GL_FRAMEBUFFER_BINDING, oldFbo);

frameBuffer.begin();
// ...
frameBuffer.end();

int fboIdx = oldFbo.get();
Gdx.gl.glBindFramebuffer(GL_FRAMEBUFFER, fboIdx);

To kind of override the internal behavior of how FrameBuffer#end() does the unbind to go back to the screen.

Version of libGDX and/or relevant dependencies

gdxVersion = '1.12.0'

Screenshots

With both FIX=false or FIX=true, there is no problem on Android/Desktop:

Screenshot 2023-12-09 at 1 33 23 PM

However, on iOS, using FIX=false causes a wrong result:

Simulator Screenshot - iPhone 15 - 2023-12-09 at 13 32 31

The reason is that FrameBuffer#end() does not work as expected.

With my fix to enforce going back to the previous frame buffer (here: the screen), it works correctly on any platform, even iOS:

Simulator Screenshot - iPhone 15 - 2023-12-09 at 13 33 00

Please select the affected platforms

  • Android
  • iOS
  • HTML/GWT
  • Windows
  • Linux
  • macOS

References:

@b3nk4n
Copy link
Author

b3nk4n commented Dec 9, 2023

Personally, I'm unblocked by my workaround. However, would be happy to provide a PR for this reported problem. Before doing so, I would however like to get some questions clarified before I would create a PR, to have less back and forth as part of the final PR:

Questions:

  1. Is there any strict need to only set the defaultFramebufferHandle only once via defaultFramebufferHandleInitialized, instead of whenever an FBO is created? Are e.g. these OpenGL functions to get the FRAMEBUFFER_BINDING expensive?
  2. Would you prefer fixing this in a way that yields iOS specific special handling, as in the current implementation here? Even when the fix (or iOS special handling) also would work fine with other platforms? Or instead better have a single implementation that works for all the same way. Even when there might be a slightly more optimal implementation for specific platforms (e.g. simply assume defaultFramebufferHandle = 0 if platform is not iOS)

Thank you in advance!

@Tom-Ski
Copy link
Member

Tom-Ski commented Dec 10, 2023

Tested this on device, and it does not reproduce. Are you testing on sim here?

@Tom-Ski
Copy link
Member

Tom-Ski commented Dec 10, 2023

Looked closer into this, and Ok this is nuts! 👁️

I think this was introduced by 0f24d72 as we have changed creation callback from waiting for the first frame, but I'm still super surprised that it hasn't been reported before you.

I guess most of us are not creating framebuffers on create, (usually after some splash screen which doesn't require FBO would be my guess)

Its not apparent in the test bench, since they construct their framebuffers after a first render due to structure of test suite. This has to happen in the create of the Adapater. We need to wait for the very first frame before finding the 'default' render buffer handle it seems as it returns 0 until we get that first frame.

I'd rather not query something on every single interaction with framebuffers, every little helps with not putting gl commands into the buffers. Perhaps we can bring back the first frame logic, even if its for setting the default handle.
Should take a look at #6971 again and see if we can find a better solution for all.

@b3nk4n
Copy link
Author

b3nk4n commented Dec 11, 2023

Tested this on device, and it does not reproduce. Are you testing on sim here?

I tested this on both a simulator with iOS 17.0 on an iPhone 15 with the following simulator version:

Version 15.0.1 (1015.2)
SimulatorKit 935.1
CoreSimulator 932.2

And using iOS 17.1.1 on my physical iPhone 13 device.

Thx @Tom-Ski for all the details above 👍
If I understand correctly, if I would skip creating the FBO in just the very first frame, then things would work correctly (Edit: and I can confirm that this is working that way when I create the FBO lazily in the second frame).

Sounds more complicated that I thought. And would therefore better leave that to you, as you seem to have a lot of context around this already. But let me know in case there is anything I can help with.
Thx!

@crykn crykn added this to the 1.12.2 milestone Dec 26, 2023
@LobbyDivinus
Copy link

So this will be fixed? Nice. I also stumbled across this issue when using shadow light which internally uses a framebuffer. Postponing the creation until the first update call works but doesn't feel right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants