godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.11k stars 20.2k forks source link

GLES2 MSAA implementation relies on platform-defines instead of feature-based ones #32455

Open punto- opened 4 years ago

punto- commented 4 years ago

Godot version: master

Issue description: in https://github.com/godotengine/godot/blob/bd74084e2fb431e8ef0ea3150ca82a4499f54bfb/drivers/gles2/rasterizer_storage_gles2.cpp#L4687 , there's a bunch of platform specific blocks to implement MSAA differently depending on the platform. We need to change it to be platform independent, by naming the features and not the platforms. For example:

#ifdef MSAA_FEATURE_1
// do msaa using feature 1
#elif MSAA_FEATURE_2
// do msaa using feature 2
#endif

Then in platform_config.h for each platform, we enable the right define to get the desired code for the platform. This allows new platforms to be implemented without having to touch this code, which should be platform-independent.

I'm opening an issue instead of a pull request because I don't know the specifics of what's being used there, I'll talk to @clayjohn and @Faless about it

akien-mga commented 4 years ago

Same goes with: https://github.com/godotengine/godot/blob/bd74084e2fb431e8ef0ea3150ca82a4499f54bfb/drivers/gles2/rasterizer_storage_gles2.cpp#L84-L104

UWP doesn't build currently because it's using the same codebranch as Android (it's not GLES_OVER_GL, and it's not IPHONE_ENABLED, so it tries to load the EXT extensions instead of the ANGLE ones). I tried to hack around a bit to fix it but it's not trivial.

KoBeWi commented 3 years ago

What's the status of this issue? AFAIK GLES2 is removed in 4.0, so not sure why this milestone. Is it still valid in 3.2 branch?

clayjohn commented 3 years ago

It is still valid in the 3.2 branch. And it has implications beyond just MSAA. It may be relevant for the GLES3 renderer in 4.0. I would keep open for now.