-
-
Notifications
You must be signed in to change notification settings - Fork 278
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
Add a local storage of uniform constants values to the OpenGL renderer #8167
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2122,15 +2122,22 @@ static void LogFrameBufferError(GLenum status) | |
return ShaderDesc::LANGUAGE_GLSL_SM140; | ||
} | ||
|
||
static void OpenGLEnableProgram(HContext context, HProgram program) | ||
static void OpenGLEnableProgram(HContext _context, HProgram _program) | ||
{ | ||
glUseProgram(((OpenGLProgram*) program)->m_Id); | ||
OpenGLProgram* program = (OpenGLProgram*) _program; | ||
glUseProgram(program->m_Id); | ||
CHECK_GL_ERROR; | ||
|
||
OpenGLContext* context = (OpenGLContext*) _context; | ||
context->m_ActiveProgram = program; | ||
} | ||
|
||
static void OpenGLDisableProgram(HContext context) | ||
static void OpenGLDisableProgram(HContext _context) | ||
{ | ||
glUseProgram(0); | ||
|
||
OpenGLContext* context = (OpenGLContext*) _context; | ||
context->m_ActiveProgram = 0; | ||
} | ||
|
||
static bool TryLinkProgram(HVertexProgram vert_program, HFragmentProgram frag_program) | ||
|
@@ -2267,14 +2274,51 @@ static void LogFrameBufferError(GLenum status) | |
CHECK_GL_ERROR; | ||
} | ||
|
||
static bool StoreConstantValue(HContext context, const void* data, const size_t size, HUniformLocation base_location) | ||
{ | ||
assert(size <= 64); | ||
|
||
OpenGLProgram* program = (OpenGLProgram*)((OpenGLContext*)context)->m_ActiveProgram; | ||
assert(program); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||
|
||
dmArray<OpenGLConstantValue>& values = program->m_ConstantValues; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be reset to 0, when it is disabled. Which could ofc be an option to always use memcmp() to find out if it changed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. E.g. something like this:
Also, in terms of struct size / complexity, I wonder if a simple hash32 would suffice? E.g.:
It takes a few extra cycles I would think, but also simplifies the logic, and saves some storage (admittedly not much) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I thought about that, but hash functions have a small chance of giving the same values, right? It seems to me that users can be paranoid about this. So I'll have to make an option in game.project like "optimize uniform constants". So you can toggle it on and off if you are in doubt. |
||
int32_t location = base_location; | ||
if (values.Size() < location + 1) | ||
{ | ||
values.SetCapacity(location + 1); | ||
values.SetSize(location + 1); | ||
} | ||
|
||
OpenGLConstantValue& value = values[location]; | ||
if (value.m_ValueSet && memcmp(value.m_Value, data, size) == 0) | ||
{ | ||
return true; | ||
} | ||
|
||
memcpy(value.m_Value, data, size); | ||
value.m_ValueSet = true; | ||
|
||
return false; | ||
} | ||
|
||
static void OpenGLSetConstantV4(HContext context, const Vector4* data, int count, HUniformLocation base_location) | ||
{ | ||
if (StoreConstantValue(context, data, count * 4 * sizeof(GLfloat), base_location)) | ||
{ | ||
return; | ||
} | ||
|
||
glUniform4fv(base_location, count, (const GLfloat*) data); | ||
CHECK_GL_ERROR; | ||
} | ||
|
||
static void OpenGLSetConstantM4(HContext context, const Vector4* data, int count, HUniformLocation base_location) | ||
{ | ||
if (StoreConstantValue(context, data, count * 16 * sizeof(GLfloat), base_location)) | ||
{ | ||
return; | ||
} | ||
|
||
glUniformMatrix4fv(base_location, count, 0, (const GLfloat*) data); | ||
CHECK_GL_ERROR; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, we don't do defensive coding when the engine runs. While this file contains them, I'd say if's more for things like init/exit of engine and creation of the global state.
Numbers like this should generally be checked before this function. Do we even need to?
It helps us keep the engine size smaller too.