Query GL extensions by enum instead of strings #25024
Conversation
sarbes
added
v22 "P"
Type: Improvement
There was a problem hiding this comment.
I'm not really sold on this idea. The problem is that it brings GL specifics into the base class. This problem may already exist (I didn't check) but we should avoid bloating the base class.
It may be a better idea to adjust the existing methods. For example we could tokenize the existing extension list and use std::unordered_map to cache the access. The hash lookup is likely less expensive than the string compare (sic).
| @@ -140,4 +145,5 @@ class CRenderSystemGLES : public CRenderSystemBase | |||
| ShaderMethodGLES m_method = ShaderMethodGLES::SM_DEFAULT; | |||
|
|
|||
| GLint m_viewPort[4]; | |||
| std::map<GLEXTENSIONS::EXTENSION, bool> m_extensionMap{}; | |||
|
|
||
| struct GLEXTENSIONS | ||
| { | ||
| enum EXTENSION |
There was a problem hiding this comment.
This email and values don't follow the normal conventions https://github.com/xbmc/xbmc/blob/master/docs/CODE_GUIDELINES.md#83-enums
| @@ -39,6 +40,7 @@ class CRenderSystemBase | |||
| virtual void PresentRender(bool rendered, bool videoLayer) = 0; | |||
| virtual bool ClearBuffers(UTILS::COLOR::Color color) = 0; | |||
| virtual bool IsExtSupported(const char* extension) const = 0; | |||
| virtual bool IsExtSupported(const GLEXTENSIONS::EXTENSION extension) { return false; } | |||
There was a problem hiding this comment.
This somewhat breaks abstraction by putting derived class specifics into the base class. I would think extensions would exist for the base class.
It is possible to get rid of this and cast winSystem where needed.
CRenderSystemBaseand CWindowSystemBase seem to be a dumping ground for non abstract methods.
jenkins4kodi
added
the
Rebase needed
|
@sarbes this needs a rebase |
Description
The PR changes the way to query for GL/GLES extensions. We can now query for an enum, instead of doing string compares.
Maybe there is a better way to implement it, but this approach looks reasonable to me. I'd like to hear if someone has some input.
Motivation and context
String compares are somewhat expensive. Some might be queried pretty often, so keeping track of what is supported might be better in terms of performance.
How has this been tested?
Compiles fine for GL/GLES. Runs fine.
What is the effect on users?
Almost none; except a tiny, tiny performance increase.
Screenshots (if appropriate):
Types of change
Checklist: