-
Notifications
You must be signed in to change notification settings - Fork 86
Change OpenGL 3 Renderer to Core Profile #234
base: master
Are you sure you want to change the base?
Conversation
This profile does not allow any pre-modern OpenGL practices, making it a good way to make sure the OpenGL 3 video driver is resilient to different OpenGL implementations (especially the ones that only support modern practices).
There are still major graphical glitches that make it unplayable, but the GUI is rendering as well as many game objects.
The game is now playable and the driver is working correctly!
I added a minor optimization that removed the allocation of new VBOs every beginDraw call. Instead there is a global VBO used by all beginDraw calls except ones called by drawHardwareBuffer. Still inefficient and can be vastly improved, but at least it's better than before.
I want to explain why I commented out GL_GENERATE_MIPMAP_HINT in case someone knows a good replacement for this hint. I believe there might be a performance regression by me commenting out these lines so a replacement might be needed.
Thanks for the PR. |
Instead of having a global variable UseGlobalVBO, have multiple versions of beginDraw and drawVertexPrimitiveList that do and do not use GlobalVBO. This is better coding practice and could prevent strange bugs from forming in the future due to UseGlobalVBO being set to an unexpected value.
Thank you, it required a little bit of reworking the |
source/Irrlicht/OpenGL/Driver.cpp
Outdated
} | ||
} | ||
|
||
void COpenGL3DriverBase::beginDraw(const VertexType &vertexType, int vertexCount, uintptr_t verticesBase) |
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.
Now that verticesBase
is always a pointer and not an offset, it should likely be const void *
I guess Some time ago, I’ve seen code that simply replaces Also, how do you handle index buffers? |
Replace unnecessary type uintptr_t with an actual pointer type.
When EHM_NEVER is used, then GL_ARRAY_BUFFER is not bound, so it does not make sense to use drawVertexPrimitiveListWithBoundVBO. On the other hand, since all data is on the GPU anyway now, EHM_NEVER does not make much sense as an option in the first place and doesn't really do what's expected.
Thank you for the code correction, and you were absolutely right about changing that type. So, right now my video driver handles index buffers in not such a good way; it assumes |
Ignoring irrlicht/source/Irrlicht/CWebGL1Driver.cpp Lines 77 to 95 in cfb73d0
So should be fine on GL3 as well, as long as is done properly and systematically and doesn’t cause any leaks (Irrlicht way of caching meshes is prone of that to my knowledge). |
Assuming this is superseded due to #250 |
@calebabutler Any news on this? And on EHM_NEVER on index buffers: I now think just ignoring one is wrong, it’s better to handle it the same way as vertices, moving to a buffer just before rendering. To reduce potential surprises from the “hw buffer link” and whatever else. I’d not expose any mixed RAM/VBO calls though, keeping all that stuff as an internal detail. |
This is a much smaller pull request compared to my previous one that only switches to the 3.3 core profile without any of the other major changes. I also cleaned up the code a little bit and made the reasons why I was commenting certain things out more obvious.