From c4baa5b81d19051bfa3408cdd2cc1e59c863ad65 Mon Sep 17 00:00:00 2001 From: Ray Date: Mon, 9 Feb 2026 22:23:23 +0100 Subject: [PATCH] REVIEWED: Comments --- src/rlgl.h | 87 +++++++++++++++++++++++++++--------------------------- 1 file changed, 43 insertions(+), 44 deletions(-) diff --git a/src/rlgl.h b/src/rlgl.h index d3e86cf1f..d05bc42cb 100644 --- a/src/rlgl.h +++ b/src/rlgl.h @@ -204,9 +204,9 @@ #define RL_DEFAULT_BATCH_BUFFER_ELEMENTS 8192 #endif #if defined(GRAPHICS_API_OPENGL_ES2) - // We reduce memory sizes for embedded systems (RPI and HTML5) + // Reducing memory sizes for embedded systems (RPI and HTML5) // NOTE: On HTML5 (emscripten) this is allocated on heap, - // by default it's only 16MB!...just take care... + // by default heap is only 16MB!...just take care... #define RL_DEFAULT_BATCH_BUFFER_ELEMENTS 2048 #endif #endif @@ -1277,7 +1277,7 @@ void rlTranslatef(float x, float y, float z) matTranslation.m13 = y; matTranslation.m14 = z; - // NOTE: We transpose matrix with multiplication order + // NOTE: Transposing matrix by multiplication order *RLGL.State.currentMatrix = rlMatrixMultiply(matTranslation, *RLGL.State.currentMatrix); } @@ -1322,7 +1322,7 @@ void rlRotatef(float angle, float x, float y, float z) matRotation.m14 = 0.0f; matRotation.m15 = 1.0f; - // NOTE: We transpose matrix with multiplication order + // NOTE: Transposing matrix by multiplication order *RLGL.State.currentMatrix = rlMatrixMultiply(matRotation, *RLGL.State.currentMatrix); } @@ -1336,7 +1336,7 @@ void rlScalef(float x, float y, float z) matScale.m5 = y; matScale.m10 = z; - // NOTE: We transpose matrix with multiplication order + // NOTE: Transposing matrix by multiplication order *RLGL.State.currentMatrix = rlMatrixMultiply(matScale, *RLGL.State.currentMatrix); } @@ -1418,7 +1418,6 @@ void rlOrtho(double left, double right, double bottom, double top, double znear, #endif // Set the viewport area (transformation from normalized device coordinates to window coordinates) -// NOTE: We store current viewport dimensions void rlViewport(int x, int y, int width, int height) { glViewport(x, y, width, height); @@ -1529,9 +1528,9 @@ void rlVertex3f(float x, float y, float z) tz = RLGL.State.transform.m2*x + RLGL.State.transform.m6*y + RLGL.State.transform.m10*z + RLGL.State.transform.m14; } - // WARNING: We can't break primitives when launching a new batch + // WARNING: Be careful with primitives breaking when launching a new batch! // RL_LINES comes in pairs, RL_TRIANGLES come in groups of 3 vertices and RL_QUADS come in groups of 4 vertices - // We must check current draw.mode when a new vertex is required and finish the batch only if the draw.mode draw.vertexCount is %2, %3 or %4 + // Checking current draw.mode when a new vertex is required and finish the batch only if the draw.mode draw.vertexCount is %2, %3 or %4 if (RLGL.State.vertexCounter > (RLGL.currentBatch->vertexBuffer[RLGL.currentBatch->currentBuffer].elementCount*4 - 4)) { if ((RLGL.currentBatch->draws[RLGL.currentBatch->drawCounter - 1].mode == RL_LINES) && @@ -1539,7 +1538,7 @@ void rlVertex3f(float x, float y, float z) { // Reached the maximum number of vertices for RL_LINES drawing // Launch a draw call but keep current state for next vertices comming - // NOTE: We add +1 vertex to the check for security + // NOTE: Adding +1 vertex to the check for some safety rlCheckRenderBatchLimit(2 + 1); } else if ((RLGL.currentBatch->draws[RLGL.currentBatch->drawCounter - 1].mode == RL_TRIANGLES) && @@ -1659,7 +1658,7 @@ void rlSetTexture(unsigned int id) #if defined(GRAPHICS_API_OPENGL_11) rlDisableTexture(); #else - // NOTE: If quads batch limit is reached, we force a draw call and next batch starts + // NOTE: If quads batch limit is reached, force a draw call and next batch starts if (RLGL.State.vertexCounter >= RLGL.currentBatch->vertexBuffer[RLGL.currentBatch->currentBuffer].elementCount*4) { @@ -2485,7 +2484,7 @@ void rlLoadExtensions(void *loader) const char **extList = (const char **)RL_CALLOC(512, sizeof(const char *)); // Allocate 512 strings pointers (2 KB) const char *extensions = (const char *)glGetString(GL_EXTENSIONS); // One big const string - // NOTE: We have to duplicate string because glGetString() returns a const string + // NOTE: String duplication rquired because glGetString() returns a const string int extensionsLength = (int)strlen(extensions); // Get extensions string size in bytes char *extensionsDup = (char *)RL_CALLOC(extensionsLength + 1, sizeof(char)); // Allocate space for copy with additional EOL byte strncpy(extensionsDup, extensions, extensionsLength); @@ -2970,19 +2969,20 @@ void rlUnloadRenderBatch(rlRenderBatch batch) } // Draw render batch -// NOTE: We require a pointer to reset batch and increase current buffer (multi-buffer) +// NOTE: Batch is reseted and current buffer is updated (for multi-buffer config) void rlDrawRenderBatch(rlRenderBatch *batch) { #if defined(GRAPHICS_API_OPENGL_33) || defined(GRAPHICS_API_OPENGL_ES2) // Update batch vertex buffers //------------------------------------------------------------------------------------------------------------ // NOTE: If there is not vertex data, buffers doesn't need to be updated (vertexCount > 0) - // TODO: If no data changed on the CPU arrays there is no need to re-upload data to GPU, - // a flag can be used to detect changes but it would imply keeping a copy buffer and memcmp() both, does it worth it? if (RLGL.State.vertexCounter > 0) { // Activate elements VAO if (RLGL.ExtSupported.vao) glBindVertexArray(batch->vertexBuffer[batch->currentBuffer].vaoId); + + // TODO: If no data changed on the CPU arrays there is no need to re-upload data to GPU, + // a flag can be used to detect changes but it would imply keeping a copy buffer and memcmp() both, does it worth it? // Vertex positions buffer glBindBuffer(GL_ARRAY_BUFFER, batch->vertexBuffer[batch->currentBuffer].vboId[0]); @@ -3006,18 +3006,17 @@ void rlDrawRenderBatch(rlRenderBatch *batch) // NOTE: glMapBuffer() causes sync issue // If GPU is working with this buffer, glMapBuffer() will wait(stall) until GPU to finish its job - // To avoid waiting (idle), you can call first glBufferData() with NULL pointer before glMapBuffer() - // If you do that, the previous data in PBO will be discarded and glMapBuffer() returns a new + // To avoid waiting (idle), glBufferData() can bee called first with NULL pointer before glMapBuffer() + // Doing that, the previous data in PBO will be discarded and glMapBuffer() returns a new // allocated pointer immediately even if GPU is still working with the previous data // Another option: map the buffer object into client's memory - // Probably this code could be moved somewhere else... - // batch->vertexBuffer[batch->currentBuffer].vertices = (float *)glMapBuffer(GL_ARRAY_BUFFER, GL_READ_WRITE); - // if (batch->vertexBuffer[batch->currentBuffer].vertices) - // { - // Update vertex data - // } - // glUnmapBuffer(GL_ARRAY_BUFFER); + //batch->vertexBuffer[batch->currentBuffer].vertices = (float *)glMapBuffer(GL_ARRAY_BUFFER, GL_READ_WRITE); + //if (batch->vertexBuffer[batch->currentBuffer].vertices) + //{ + // Update vertex data + //} + //glUnmapBuffer(GL_ARRAY_BUFFER); // Unbind the current VAO if (RLGL.ExtSupported.vao) glBindVertexArray(0); @@ -3132,7 +3131,7 @@ void rlDrawRenderBatch(rlRenderBatch *batch) else { #if defined(GRAPHICS_API_OPENGL_33) - // We need to define the number of indices to be processed: elementCount*6 + // The number of indices to be processed needs to be defined: elementCount*6 // NOTE: The final parameter tells the GPU the offset in bytes from the // start of the index buffer to the location of the first index to process glDrawElements(GL_TRIANGLES, batch->draws[i].vertexCount/4*6, GL_UNSIGNED_INT, (GLvoid *)(vertexOffset/4*6*sizeof(GLuint))); @@ -3233,7 +3232,7 @@ bool rlCheckRenderBatchLimit(int vCount) rlDrawRenderBatch(RLGL.currentBatch); // NOTE: Stereo rendering is checked inside - // Restore state of last batch so we can continue adding vertices + // Restore state of last batch so new vertices can be added RLGL.currentBatch->draws[RLGL.currentBatch->drawCounter - 1].mode = currentMode; RLGL.currentBatch->draws[RLGL.currentBatch->drawCounter - 1].textureId = currentTexture; } @@ -3395,7 +3394,7 @@ unsigned int rlLoadTexture(const void *data, int width, int height, int format, } #endif - // At this point we have the texture loaded in GPU and texture parameters configured + // At this point texture is loaded in GPU and texture parameters configured // NOTE: If mipmaps were not in data, they are not generated automatically @@ -3416,10 +3415,10 @@ unsigned int rlLoadTextureDepth(int width, int height, bool useRenderBuffer) if (!isGpuReady) { TRACELOG(RL_LOG_WARNING, "GL: GPU is not ready to load data, trying to load before InitWindow()?"); return id; } #if defined(GRAPHICS_API_OPENGL_33) || defined(GRAPHICS_API_OPENGL_ES2) - // In case depth textures not supported, we force renderbuffer usage + // In case depth textures were not supported, force renderbuffer usage if (!RLGL.ExtSupported.texDepth) useRenderBuffer = true; - // NOTE: We let the implementation to choose the best bit-depth + // NOTE: Letting the implementation to choose the best bit-depth // Possible formats: GL_DEPTH_COMPONENT16, GL_DEPTH_COMPONENT24, GL_DEPTH_COMPONENT32 and GL_DEPTH_COMPONENT32F unsigned int glInternalFormat = GL_DEPTH_COMPONENT; @@ -3565,7 +3564,7 @@ unsigned int rlLoadTextureCubemap(const void *data, int size, int format, int mi } // Update already loaded texture in GPU with new data -// NOTE: We don't know safely if internal texture format is the expected one... +// WARNING: Not possible to know safely if internal texture format is the expected one... void rlUpdateTexture(unsigned int id, int offsetX, int offsetY, int width, int height, int format, const void *data) { glBindTexture(GL_TEXTURE_2D, id); @@ -3699,7 +3698,7 @@ void *rlReadTexturePixels(unsigned int id, int width, int height, int format) #if defined(GRAPHICS_API_OPENGL_11) || defined(GRAPHICS_API_OPENGL_33) glBindTexture(GL_TEXTURE_2D, id); - // NOTE: Using texture id, we can retrieve some texture info (but not on OpenGL ES 2.0) + // NOTE: Using texture id, some texture info can be retrieved (but not on OpenGL ES 2.0) // Possible texture info: GL_TEXTURE_RED_SIZE, GL_TEXTURE_GREEN_SIZE, GL_TEXTURE_BLUE_SIZE, GL_TEXTURE_ALPHA_SIZE //int width, height, format; //glGetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_WIDTH, &width); @@ -3742,7 +3741,7 @@ void *rlReadTexturePixels(unsigned int id, int width, int height, int format) // Attach our texture to FBO glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, id, 0); - // We read data as RGBA because FBO texture is configured as RGBA, despite binding another texture format + // Reading data as RGBA because FBO texture is configured as RGBA, despite binding another texture format pixels = RL_CALLOC(rlGetPixelDataSize(width, height, RL_PIXELFORMAT_UNCOMPRESSED_R8G8B8A8), 1); glReadPixels(0, 0, width, height, GL_RGBA, GL_UNSIGNED_BYTE, pixels); @@ -3778,12 +3777,12 @@ unsigned char *rlReadScreenPixels(int width, int height) { unsigned char *imgData = (unsigned char *)RL_CALLOC(width*height*4, sizeof(unsigned char)); - // NOTE 1: glReadPixels returns image flipped vertically -> (0,0) is the bottom left corner of the framebuffer - // NOTE 2: We are getting alpha channel! Be careful, it can be transparent if not cleared properly! + // NOTE: glReadPixels() returns image flipped vertically -> (0,0) is the bottom left corner of the framebuffer + // WARNING: Getting alpha channel! Be careful, it can be transparent if not cleared properly! glReadPixels(0, 0, width, height, GL_RGBA, GL_UNSIGNED_BYTE, imgData); - // Flip image vertically! - // NOTE: Alpha value has already been applied to RGB in framebuffer, we don't need it! + // Flip image vertically + // NOTE: Alpha value has already been applied to RGB in framebuffer, not needed anymore for (int y = height - 1; y >= height/2; y--) { for (int x = 0; x < (width*4); x += 4) @@ -3904,13 +3903,13 @@ void rlUnloadFramebuffer(unsigned int id) { #if (defined(GRAPHICS_API_OPENGL_33) || defined(GRAPHICS_API_OPENGL_ES2)) // Query depth attachment to automatically delete texture/renderbuffer - int depthType = 0, depthId = 0; + int depthType = 0; glBindFramebuffer(GL_FRAMEBUFFER, id); // Bind framebuffer to query depth texture type glGetFramebufferAttachmentParameteriv(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE, &depthType); - // TODO: Review warning retrieving object name in WebGL // WARNING: WebGL: INVALID_ENUM: getFramebufferAttachmentParameter: invalid parameter name // REF: https://registry.khronos.org/webgl/specs/latest/1.0/ + int depthId = 0; glGetFramebufferAttachmentParameteriv(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME, &depthId); unsigned int depthIdU = (unsigned int)depthId; @@ -4190,15 +4189,15 @@ unsigned int rlLoadShaderCode(const char *vsCode, const char *fsCode) if (fsCode != NULL) fragmentShaderId = rlCompileShader(fsCode, GL_FRAGMENT_SHADER); else fragmentShaderId = RLGL.State.defaultFShaderId; - // In case vertex and fragment shader are the default ones, no need to recompile, we can just assign the default shader program id + // In case vertex and fragment shader are the default ones, no need to recompile, just assign the default shader program id if ((vertexShaderId == RLGL.State.defaultVShaderId) && (fragmentShaderId == RLGL.State.defaultFShaderId)) id = RLGL.State.defaultShaderId; else if ((vertexShaderId > 0) && (fragmentShaderId > 0)) { - // One of or both shader are new, we need to compile a new shader program + // One of or both shader are new, a new shader program needs to be compiled id = rlLoadShaderProgram(vertexShaderId, fragmentShaderId); - // We can detach and delete vertex/fragment shaders (if not default ones) - // NOTE: We detach shader before deletion to make sure memory is freed + // Detaching and deleting vertex/fragment shaders (if not default ones) + // WARNING: Detach shader before deletion to make sure memory is freed if (vertexShaderId != RLGL.State.defaultVShaderId) { // WARNING: Shader program linkage could fail and returned id is 0 @@ -4212,10 +4211,10 @@ unsigned int rlLoadShaderCode(const char *vsCode, const char *fsCode) glDeleteShader(fragmentShaderId); } - // In case shader program loading failed, we assign default shader + // In case shader program loading failed, assign default shader if (id == 0) { - // In case shader loading fails, we return the default shader + // In case shader loading fails, reassigning default shader TRACELOG(RL_LOG_WARNING, "SHADER: Failed to load custom shader code, using default shader"); id = RLGL.State.defaultShaderId; } @@ -4737,9 +4736,9 @@ Matrix rlGetMatrixTransform(void) Matrix mat = rlMatrixIdentity(); #if defined(GRAPHICS_API_OPENGL_33) || defined(GRAPHICS_API_OPENGL_ES2) // TODO: Consider possible transform matrices in the RLGL.State.stack - // Is this the right order? or should we start with the first stored matrix instead of the last one? //Matrix matStackTransform = rlMatrixIdentity(); //for (int i = RLGL.State.stackCounter; i > 0; i--) matStackTransform = rlMatrixMultiply(RLGL.State.stack[i], matStackTransform); + mat = RLGL.State.transform; #endif return mat;