Skip to content
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

Runtime openGL loading #40

Open
wants to merge 22 commits into
base: mitsuba
Choose a base branch
from

Conversation

Hazardu
Copy link

@Hazardu Hazardu commented May 26, 2020

No description provided.

@AnastaZIuk
Copy link

You should use the macros probably and as far as I remember split some functions into classes creating them by macro as well. Look at glue directive

@devshgraphicsprogramming

You should use the macros probably and as far as I remember split some functions into classes creating them by macro as well. Look at glue directive

As you may recall from our chat @Hazardu I gave examples of how its done in CCudaHandler and COpenCLHandler

Hazardu added 2 commits June 12, 2020 15:54
I Couldnt find defintions for the following fuctions online:
GlFramebufferDrawBuffersEXT
GlFramebufferDrawBufferEXT
GlFramebufferReadBufferEXT
GlGetNamedBufferSubDataEXT

The pace of writing those functions is quite slow, as im checking what each of them does

There also have been some on the wiki in related articles that i have added, for ex: glGet functions had a few more types on the wiki page
Hazardu added 2 commits June 24, 2020 21:33
todo: move the ptrs used in those classes to a private class
Hazardu added 2 commits July 5, 2020 21:54
…ler in COpenGLDRiver, partly in other files

Theres a few errors in CopenGLDriver.cpp
first being at line 233, Function Table is not a nonstatic data member or base class of class
next, 1072, initExtensions() no longer exist, so i assume i need to rewrite it, and put it inside COpenGLFunctionTable.h
3rd issue is RUNNING_IN_RENDERDOC_EXTENSION_NAME at line 1181 is undefined
lastly deltype at 2395 with `&COpenGLFunctionTable::glGeneral.pglDisablei`
@Hazardu
Copy link
Author

Hazardu commented Jul 7, 2020

in ExtensionHandler, there is a method initExtensions thats being invoked from COpenGLDriver.genericDriverInit. Should i rewrite it and put it inside FunctionTable or OpenGLDriver?

Comment on lines 61 to 66
#define glBindBufferRange_MACRO glBindBufferRange
#endif // glBindBufferRange_MACRO

#ifndef glGetNamedBufferParameteri64v_MACRO
#define glGetNamedBufferParameteri64v_MACRO glGetNamedBufferParameteri64v
#ifndef glGetNamedBufferParameteri64v_MACRO
#define glGetNamedBufferParameteri64v_MACRO glGetNamedBufferParameteri64v
#endif

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all these we don't need, actually I don't think we need the COpenGLStateManagerImpl and the COpenGLStateManager at all

@@ -180,7 +180,7 @@ bool COpenGLFrameBuffer::attach(E_FBO_ATTACHMENT_POINT attachmenPoint, core::sma
}
if (enabledBufferCnt)
enabledBufferCnt += 1-EFAP_COLOR_ATTACHMENT0;
COpenGLExtensionHandler::extGlNamedFramebufferDrawBuffers(frameBuffer, enabledBufferCnt, drawBuffers);
COpenGLFunctionTable::extGlNamedFramebufferDrawBuffers(frameBuffer, enabledBufferCnt, drawBuffers);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope the methods of the function table cannot be static, you need getThreadContext_helper then get the function table from the SAuxContext and then call the non-static method

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

//}
//}
//
//#endif

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just remove the file from the repo, we have a file history after all

@@ -38,7 +38,7 @@ class IOpenGLPipeline
//shader programs can be shared so all names can be freed by any thread
for (const auto& p : (*m_GLprograms))
if (p.GLname != 0u)
COpenGLExtensionHandler::extGlDeleteProgram(p.GLname);
COpenGLFunctionTable::extGlDeleteProgram(p.GLname);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no static methods

@@ -54,7 +54,7 @@ class IOpenGLPipeline

IRR_ASSUME_ALIGNED(_pcData, 128);

using gl = COpenGLExtensionHandler;
using gl = COpenGLFunctionTable;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no static methods

Comment on lines +176 to +179
GLuint GLname = COpenGLFunctionTable::extGlCreateShaderProgramv(m_GLstage, 1u, &glslCode_cstr);

GLchar logbuf[1u<<12]; //4k
COpenGLExtensionHandler::extGlGetProgramInfoLog(GLname, sizeof(logbuf), nullptr, logbuf);
COpenGLFunctionTable::extGlGetProgramInfoLog(GLname, sizeof(logbuf), nullptr, logbuf);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no static methods

Comment on lines -188 to +190
COpenGLExtensionHandler::extGlGetProgramiv(GLname, GL_PROGRAM_BINARY_LENGTH, &binaryLength);
COpenGLFunctionTable::extGlGetProgramiv(GLname, GL_PROGRAM_BINARY_LENGTH, &binaryLength);
binary.binary = core::make_refctd_dynamic_array<core::smart_refctd_dynamic_array<uint8_t>>(binaryLength);
COpenGLExtensionHandler::extGlGetProgramBinary(GLname, binaryLength, nullptr, &binary.format, binary.binary->data());
COpenGLFunctionTable::extGlGetProgramBinary(GLname, binaryLength, nullptr, &binary.format, binary.binary->data());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no static methods

@@ -196,7 +196,7 @@ void COpenGLSpecializedShader::gatherUniformLocations(GLuint _GLname) const
{
m_locations.resize(m_uniformsList.size());
for (size_t i = 0ull; i < m_uniformsList.size(); ++i)
m_locations[i] = COpenGLExtensionHandler::extGlGetUniformLocation(_GLname, m_uniformsList[i].m.name.c_str());
m_locations[i] = COpenGLFunctionTable::extGlGetUniformLocation(_GLname, m_uniformsList[i].m.name.c_str());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no static methods

Comment on lines -52 to +60
COpenGLExtensionHandler::extGlTextureParameterIuiv(name,target,GL_TEXTURE_SWIZZLE_RGBA,swizzle);
COpenGLFunctionTable::extGlTextureParameterIuiv(name,target,GL_TEXTURE_SWIZZLE_RGBA,swizzle);
}

void regenerateMipMapLevels() override
{
if (params.subresourceRange.levelCount <= 1u)
return;

COpenGLExtensionHandler::extGlGenerateTextureMipmap(name,target);
COpenGLFunctionTable::extGlGenerateTextureMipmap(name,target);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no static methods

@@ -37,27 +37,27 @@ class COpenGLImage final : public IGPUImage, public IDriverMemoryAllocation
internalFormat(GL_INVALID_ENUM), target(GL_INVALID_ENUM), name(0u)
{
#ifdef OPENGL_LEAK_DEBUG
COpenGLExtensionHandler::textureLeaker.registerObj(this);
COpenGLFunctionTable::textureLeaker.registerObj(this);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can get rid of the textureLeaker we will replace it with something better in the future

Comment on lines -22 to +28
COpenGLExtensionHandler::extGlCreateTextures(GL_TEXTURE_BUFFER, 1, &m_textureName);
COpenGLFunctionTable::extGlCreateTextures(GL_TEXTURE_BUFFER, 1, &m_textureName);
m_GLformat = getSizedOpenGLFormatFromOurFormat(m_format);

if (m_offset==0u && m_size==m_buffer->getSize())
COpenGLExtensionHandler::extGlTextureBuffer(m_textureName, m_GLformat, static_cast<COpenGLBuffer*>(m_buffer.get())->getOpenGLName());
COpenGLFunctionTable::extGlTextureBuffer(m_textureName, m_GLformat, static_cast<COpenGLBuffer*>(m_buffer.get())->getOpenGLName());
else
COpenGLExtensionHandler::extGlTextureBufferRange(m_textureName, m_GLformat, static_cast<COpenGLBuffer*>(m_buffer.get())->getOpenGLName(), m_offset, m_size);
COpenGLFunctionTable::extGlTextureBufferRange(m_textureName, m_GLformat, static_cast<COpenGLBuffer*>(m_buffer.get())->getOpenGLName(), m_offset, m_size);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no static methods

Comment on lines 1104 to 1115
glViewport(0, 0, Params.WindowSize.Width, Params.WindowSize.Height);

// adjust flat coloring scheme to DirectX version
///extGlProvokingVertex(GL_FIRST_VERTEX_CONVENTION_EXT);
///COpenGLFunctionTable::glShader.pglProvokingVertex(GL_FIRST_VERTEX_CONVENTION_EXT);

// We need to reset once more at the beginning of the first rendering.
// This fixes problems with intermediate changes to the material during texture load.
SAuxContext* found = getThreadContext_helper(false);
extGlClipControl(GL_UPPER_LEFT, GL_ZERO_TO_ONE); //once set and should never change (engine doesnt track it)
COpenGLFunctionTable::glGeneral.pglClipControl(GL_UPPER_LEFT, GL_ZERO_TO_ONE); //once set and should never change (engine doesnt track it)
glEnable(GL_FRAMEBUFFER_SRGB);//once set and should never change (engine doesnt track it)
glEnable(GL_TEXTURE_CUBE_MAP_SEAMLESS);//once set and should never change (engine doesnt track it)
glDepthRange(1.0, 0.0);//once set and should never change (engine doesnt track it)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all this setup should probably move to context intialization, don't worry about it now we'll tackle it in a separate PR

@@ -3282,9 +3302,118 @@ void COpenGLDriver::enableClipPlane(uint32_t index, bool enable)
glDisable(GL_CLIP_DISTANCE0 + index);
}

void COpenGLDriver::initExtensions(bool stencilBuffer)
{
core::stringc vendorString = (char*)glGetString(GL_VENDOR);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to initExtensions after you init the function table for the main context

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because glGetString needs to be called from the pointer in the function table

Comment on lines +3317 to +3321
glGetIntegerv(GL_UNIFORM_BUFFER_OFFSET_ALIGNMENT, &reqUBOAlignment);
assert(core::is_alignment(reqUBOAlignment));
glGetIntegerv(GL_SHADER_STORAGE_BUFFER_OFFSET_ALIGNMENT, &reqSSBOAlignment);
assert(core::is_alignment(reqSSBOAlignment));
glGetIntegerv(GL_TEXTURE_BUFFER_OFFSET_ALIGNMENT, &reqTBOAlignment);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glGetIntegerv needs to be called from the pointer in the function table

Comment on lines +3329 to +3335
glGetIntegerv(GL_MAX_COMBINED_UNIFORM_BLOCKS, reinterpret_cast<GLint*>(&maxUBOBindings));
glGetIntegerv(GL_MAX_COMBINED_SHADER_STORAGE_BLOCKS, reinterpret_cast<GLint*>(&maxSSBOBindings));
glGetIntegerv(GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, reinterpret_cast<GLint*>(&maxTextureBindings));
glGetIntegerv(GL_MAX_COMPUTE_TEXTURE_IMAGE_UNITS, reinterpret_cast<GLint*>(&maxTextureBindingsCompute));
glGetIntegerv(GL_MAX_COMBINED_IMAGE_UNIFORMS, reinterpret_cast<GLint*>(&maxImageBindings));

glGetIntegerv(GL_MIN_MAP_BUFFER_ALIGNMENT, &minMemoryMapAlignment);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glGetIntegerv needs to be called from the pointer in the function table

Comment on lines 3342 to 3385
glGetIntegerv(GL_MAX_ARRAY_TEXTURE_LAYERS, &num);
MaxArrayTextureLayers = num;

if (FeatureAvailable[IRR_EXT_texture_filter_anisotropic])
{
glGetIntegerv(GL_MAX_TEXTURE_MAX_ANISOTROPY_EXT, &num);
MaxAnisotropy = static_cast<uint8_t>(num);
}


if (FeatureAvailable[IRR_ARB_geometry_shader4])
{
glGetIntegerv(GL_MAX_GEOMETRY_OUTPUT_VERTICES, &num);
MaxGeometryVerticesOut = static_cast<uint32_t>(num);
}

if (FeatureAvailable[IRR_EXT_texture_lod_bias])
glGetFloatv(GL_MAX_TEXTURE_LOD_BIAS_EXT, &MaxTextureLODBias);


glGetIntegerv(GL_MAX_CLIP_DISTANCES, &num);
MaxUserClipPlanes=static_cast<uint8_t>(num);
glGetIntegerv(GL_MAX_DRAW_BUFFERS, &num);
MaxMultipleRenderTargets = static_cast<uint8_t>(num);

glGetFloatv(GL_ALIASED_LINE_WIDTH_RANGE, DimAliasedLine);
glGetFloatv(GL_ALIASED_POINT_SIZE_RANGE, DimAliasedPoint);
glGetFloatv(GL_SMOOTH_LINE_WIDTH_RANGE, DimSmoothedLine);
glGetFloatv(GL_SMOOTH_POINT_SIZE_RANGE, DimSmoothedPoint);

const GLubyte* shaderVersion = glGetString(GL_SHADING_LANGUAGE_VERSION);
float sl_ver;
sscanf(reinterpret_cast<const char*>(shaderVersion),"%f",&sl_ver);
ShaderLanguageVersion = static_cast<uint16_t>(core::round(sl_ver*100.0f));


//! For EXT-DSA testing
if (IsIntelGPU)
{
Version = 440;
FeatureAvailable[IRR_ARB_direct_state_access] = false;
}
num=0;
glGetIntegerv(GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, &num);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glGetIntegerv needs to be called from the pointer in the function table

Comment on lines +3394 to +3408
glGetIntegerv(0x9047, &val);
os::Printer::log("Dedicated video memory (kB)", std::to_string(val));
glGetIntegerv(0x9048, &val);
os::Printer::log("Total video memory (kB)", std::to_string(val));
glGetIntegerv(0x9049, &val);
os::Printer::log("Available video memory (kB)", std::to_string(val));
}
if (FeatureAvailable[IRR_ATI_meminfo])
{
GLint val[4];
glGetIntegerv(GL_TEXTURE_FREE_MEMORY_ATI, val);
os::Printer::log("Free texture memory (kB)", std::to_string(val[0]));
glGetIntegerv(GL_VBO_FREE_MEMORY_ATI, val);
os::Printer::log("Free VBO memory (kB)", std::to_string(val[0]));
glGetIntegerv(GL_RENDERBUFFER_FREE_MEMORY_ATI, val);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glGetIntegerv needs to be called from the pointer in the function table

Repository owner deleted a comment from Hazardu Jul 8, 2020
// }
//
// inline void COpenGLExtensionHandler::extGlGetTextureImage(GLuint texture, GLenum target, GLint level, GLenum format, GLenum type, GLsizei bufSizeHint, void* pixels)
//

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just remove the file

@@ -635,7 +635,7 @@ bool COpenGLDriver::initDriver(CIrrDeviceWin32* device)
#endif // _IRR_COMPILE_WITH_OPENCL_
genericDriverInit(device->getAssetManager());

COpenGLFunctionTable::extGlSwapInterval(Params.Vsync ? 1 : 0);
getThreadContext_helper(true)->functionTable.extGlSwapInterval(Params.Vsync ? 1 : 0);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you already lock the context mutex? because thats the only time you should call that function with true

Comment on lines +474 to +475
//this is troublesome, how to obtain context reference here?
//functionTable.glGeneral.pglGetInternalformativ(GL_TEXTURE_2D, getSizedOpenGLFormatFromOurFormat(_fmt), GL_COLOR_RENDERABLE, sizeof(res), &res);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its not a static function, so it should be fine, the only problem is the const.... @Crisspl any advice?

Maybe we could make the operator() on the function pointer wrapper class a const operator?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants