search

sensics / OSVR-RenderManager

Apache License 2.0
64 stars 45 forks source link

Missing Header Files in SDK #329

Closed MarkVabulas closed 7 years ago

MarkVabulas commented 7 years ago

I am currently writing a custom engine implementation for using OSVR. In it, it would be nice to have tighter coupling with the underlying data types for better inspection. Can I request that the RenderManagerOpenGL.h and RenderManagerD3D.h header files are included in the install target in CMakeLists.txt?

MarkVabulas commented 7 years ago

See this commit for example: https://github.com/MarkVabulas/OSVR-RenderManager/commit/789098bd7b95d59e18b25f282976c25b81ec80b2

russell-taylor commented 7 years ago

Go ahead and do a pull request into master on the Sensics site. We ended up making the D3D base class header public to enable tighter integration with another engine. We'd like to be able to do everything through the C API, but that is currently somewhat limited.

To enable static casting (or the safer dynamic casting and checking for nullptr) is it sufficient to declare the classes and not fully specify them in the code you are using?

MarkVabulas commented 7 years ago

Sadly, it is not. This is due to wanting to access some of the local variables. I'm actually inheriting from the class and then allowing const access to the Context information so I know what framebuffer it's expecting. This makes it simpler to deal with multiple framebuffers, and always be sure that if we change the OpenGL framebuffer state we can put it back where the Render/Display expects. Right now, if you change the Framebuffer/DrawBuffer in the callback, it's lost forever without a glGetInteger telling us the previous framebuffer (performance issues). This is all actually a workaround for not having the framebuffer specified for the callback, or associated with the colorbuffer/depthbuffer.

russell-taylor commented 7 years ago

Interesting. Glad for the pull request but wanted to let you know that the OpenGL render code should always be setting the things it needs to within its methods and it should also be saving and restoring the states that it modifies; if not, we missed one and will be glad to add it. We are calling a lot of glGet calls to retrieve them, can you elaborate on the performance issues?

MarkVabulas commented 7 years ago

No particular performance issues, just warnings from zealous compilers. It's obviously best practices to use known/expected values from libraries which are already in main memory instead of having to hit up the driver before a render pass.

russell-taylor commented 7 years ago

Okay, good. If you can let me know which state is not being set or preserved properly, I can add code to save and restore it. Sounds like the framebuffer/drawbuffer; anything else?

MarkVabulas commented 7 years ago

Normally in OpenGL a texture which is bound to a framebuffer is specified as based on glDrawBuffer() calls, after being attached using glFramebufferTexture2D() calls. The problem is that this is a pair of FBO ID and Texture ID, and if you change the bound framebuffer, then calls to glDrawBuffers will fail because the framebuffer it's associated to isn't available anymore. Can I request that the framebuffer ID which the CurrentColorBuffer is bound to is supplied to render callbacks, in case the bound framebuffer changes mid-call and needs to be reset.

rpavlik commented 7 years ago

pull request was merged #332 , and a fixed one was also merged #334 .

FWIW, the C++ API is not terribly stable/fixed, though if you're deriving your own classes from the provided ones you're probably pretty closely coupled. If something needs to be passed back and forth, feel free to open a new issue or new pull request. I'm going to close this issue because the actual main issue described has been fixed :)