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

Drop ENABLE_GLES #13257

Closed
numberZero opened this issue Feb 27, 2023 · 5 comments · Fixed by #13280
Closed

Drop ENABLE_GLES #13257

numberZero opened this issue Feb 27, 2023 · 5 comments · Fixed by #13280
Labels
Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements

Comments

@numberZero
Copy link
Contributor

ENABLE_GLES predates forking Irrlicht. Its primary use was to distinguish Irrlicht-ogles from mainstream version as Minetest could be compiled with either. That’s not necessary anymore, and gets in the way sometimes.

@sfan5 sfan5 added the Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements label Feb 27, 2023
@sfan5
Copy link
Member

sfan5 commented Feb 27, 2023

I agree, most of the #if guards can be simply removed.
However:

  • defaultsettings uses to it control some things
  • the near_plane setting is only allowed on gles
  • guiEngine.cpp does something suspicious that should probably be in an if guard
  • shader.cpp defines some attributes that should be behind a runtime check instead

@numberZero
Copy link
Contributor Author

numberZero commented Feb 27, 2023

defaultsettings uses to it control some things

...and that is suspicious.

the near_plane setting is only allowed on gles

In the current code that setting is allowed on regular GL too, as long as GLES support was compiled in.

guiEngine.cpp does something suspicious that should probably be in an if guard

If you mean the Align2Npot2 thing, it is a no-op on OGLES2. It is there for OGLES1.

@sfan5
Copy link
Member

sfan5 commented Feb 27, 2023

In the current code that setting is allowed on regular GL too, as long as GLES support was compiled in.

Well yes, but that was the idea.

If you mean the Align2Npot2 thing, it is a no-op on OGLES2. It is there for OGLES1.

Sure, but this looks suspicious:
grafik

@numberZero
Copy link
Contributor Author

In the current code that setting is allowed on regular GL too, as long as GLES support was compiled in.

Well yes, but that was the idea.

And what was the reason behind it?

this looks suspicious:

Yes. Looks like the texture leaks if GLES is not compiled in, and might be freed prematurely if it is. If only it would be possible to, maybe, reference-count textures...

@sfan5
Copy link
Member

sfan5 commented Mar 4, 2023

And what was the reason behind it?

cheating concerns #8749

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants