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

Simplify the sampler object code path selection in GL backend #2955

Merged

Conversation

goodartistscopy
Copy link
Contributor

Sampler Objects are now used only based on the m_samplerObjectSupport boolean, in line with m_gles3 being discovered at runtime due to WebGL.
This should also fix the issue reported in an earlier PR #2947 (@PyryM)

Copy link
Owner

@bkaradzic bkaradzic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change doesn't work with wasm/WebGL.

@goodartistscopy goodartistscopy force-pushed the sampler_object_path_cleanup branch from bcf76b4 to 574b5d3 Compare October 13, 2022 16:55
@goodartistscopy
Copy link
Contributor Author

I specifically tested this to fix a WebGL2 problem in our app. I can confirm that sampler states are now correctly set.
After your prompt, I double checked that this did not break on WebGL1 and it doesn't. Am I missing something ?

Note that I build with BGFX_CONFIG_RENDERER_OPENGLES=30 to allow for both WebGL1 and 2 at runtime.
BGFX_CONFIG_RENDERER_OPENGLES=20 can't emit Sampler Object API, so I'm adding an additional sentinel:

m_samplerObjectSupport = false
	|| m_gles3 && BX_ENABLED(BGFX_CONFIG_RENDERER_OPENGLES >= 30)
	|| s_extension[Extension::ARB_sampler_objects].m_supported
	;

With that all the configurations I tested look fine.

@bkaradzic
Copy link
Owner

Yeah it works now.

@bkaradzic
Copy link
Owner

But this is incorrect:

			m_samplerObjectSupport = false
					|| m_gles3 && BX_ENABLED(BGFX_CONFIG_RENDERER_OPENGLES >= 30)

It should be just m_gles3, and something is missing about importing those sampler object functions so they turn to be null in WebGL.

Try removing && BX_ENABLED(BGFX_CONFIG_RENDERER_OPENGLES >= 30), and then run example-06-bump.

@goodartistscopy
Copy link
Contributor Author

With just m_gles3 and sampler object calls undefined it crashes. Or do you mean empty stubs ? but then they won't do what they should, instead of using the legacy API. I'm a bit puzzled.
When I missed && BX_ENABLED(BGFX_CONFIG_RENDERER_OPENGLES >= 30) on my first try, BGFX_CONFIG_RENDERER_OPENGLES=20 was indeed crashing on a webgl 2 enabled browser (peculiar configuration, but still allowed)

@bkaradzic
Copy link
Owner

Try this fix:
9e7aa2d

@goodartistscopy goodartistscopy force-pushed the sampler_object_path_cleanup branch from 574b5d3 to 4be4f1d Compare October 14, 2022 08:22
@goodartistscopy
Copy link
Contributor Author

Works like a charm, I missed those "GL_IMPORT____x()". Updating to just the rest of the simplifications. Thanks.

@bkaradzic bkaradzic merged commit f3ff07a into bkaradzic:master Oct 14, 2022
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

Successfully merging this pull request may close these issues.

2 participants