5
\$\begingroup\$

I have been developing a program with C lately, its purpose over the top is to reconstruct meshes from point clouds which are generated by finding the intersection points of numerous rays fired at the image which had previously been preprocessed by a HED neural-network.

I'm doing this for a school project and I haven't really been able to get used to C pretty much, which might have been the cause of me omitting some design and safety principles. According to Valgrind I have a few memory leaks, which I have trouble finding. But it's also flagging raylib's InitWindow() function to be leaking which leads me to believe that there might be some false positives.

#include <raylib.h>
#include <raymath.h>
#include <stdio.h>
#include <stdlib.h>
#include <memory.h>

typedef struct Vertice{
    Vector2 *verticeData;
    int verticeCount;
    struct Vertice* next;
    Vector2 *intersectionPoints;
    int intersectionPointCount;
} Vertice;

typedef struct IdenticalFunction{
    Vector2 start, end;
    Color color;
} IdenticalFunction;

typedef struct IFArray{
    IdenticalFunction *identicalFunctions;
    int identicalFunctionCount;
} IFArray;

typedef struct TextureBound{
    Texture2D texture;
    Rectangle rectangle;
} TextureBound;


IFArray* generateIdenticalFunctions(Vector2 worldTopLeft, Vector2 worldBottomRight){
    const int IDENTICAL_FUNCTION_GAP = 50;
    float identicalFunctionStartX = floorf(worldTopLeft.x / IDENTICAL_FUNCTION_GAP) * IDENTICAL_FUNCTION_GAP;
    float identicalFunctionStartY = floorf(worldTopLeft.y / IDENTICAL_FUNCTION_GAP) * IDENTICAL_FUNCTION_GAP;
    int identicalFunctionCountX = (worldBottomRight.x - worldTopLeft.x) / IDENTICAL_FUNCTION_GAP;
    int identicalFunctionCountY = (worldBottomRight.y - worldTopLeft.y) / IDENTICAL_FUNCTION_GAP;

    IdenticalFunction *identicalFunctions = (IdenticalFunction*)malloc((identicalFunctionCountX + identicalFunctionCountY) * sizeof(IdenticalFunction));
    for(int i = 1; i < identicalFunctionCountX; i++){
        float x  = identicalFunctionStartX + (i * IDENTICAL_FUNCTION_GAP);
        IdenticalFunction identicalFunction = {.start = (Vector2){x, worldTopLeft.y}, .end = (Vector2){x, worldBottomRight.y}, .color = RED};
        identicalFunctions[i -1] = identicalFunction;
    }
    for (int i = 1; i < identicalFunctionCountY; i++){
        float y = identicalFunctionStartY + (i * IDENTICAL_FUNCTION_GAP);
        IdenticalFunction identicalFunction = {.start = (Vector2){worldTopLeft.x, y}, .end = (Vector2){worldBottomRight.x, y}, .color = RED};
        identicalFunctions[i-1 + identicalFunctionCountX] = identicalFunction;
    }

    IFArray *ifArray = (IFArray*)malloc(sizeof(IFArray));
    ifArray->identicalFunctionCount = identicalFunctionCountX + identicalFunctionCountY;
    ifArray->identicalFunctions = identicalFunctions;

    return ifArray;
}

int main()
{
    const int WIDTH = 800;
    const int HEIGHT = 600;

    InitWindow(WIDTH, HEIGHT, "Window");

    Camera2D camera = {.offset = (Vector2){0, 0}, .target = (Vector2){0 - (float)GetScreenWidth() / 2, 0 - (float)GetScreenHeight() / 2}, .rotation = 0.0f, .zoom = 1.0f};

    SetTargetFPS(60);  
    const int CELL_SIZE = 50;
    const float ZOOM_INCREMENT = 0.125f;

    int algorithmStep = 0;

    Vertice *head = (Vertice*)malloc(sizeof(Vertice));
    head->verticeData = NULL;
    head->verticeCount = 0;
    head->next = NULL;
    head->intersectionPoints = NULL;
    head->intersectionPointCount = 0;

    Vertice *current_D = head;
    Vertice *verticeIndex = head;

    Vector2 *verticeData_G = NULL; 
    int verticeCount_G = 0;

    IFArray *ifArray_D = NULL;

    Image HED_image = LoadImage("output_hed.png");
    Texture2D HED_texture = LoadTextureFromImage(HED_image);
    UnloadImage(HED_image);
    TextureBound HED_bound = {.texture = HED_texture, .rectangle = (Rectangle){.x = 0 - HED_texture.width / 2.0f, .y = 0 - HED_texture.height / 2.0f, .width = HED_texture.width, .height = HED_texture.height}};

    while (!WindowShouldClose()) {
        BeginDrawing();
        ClearBackground(BLACK);
        BeginMode2D(camera);
        
        Vector2 cameraPosition = {camera.offset.x, camera.offset.y};
        Vector2 worldTopLeft = GetScreenToWorld2D(cameraPosition, camera);
        Vector2 worldBottomRight = GetScreenToWorld2D((Vector2){ cameraPosition.x + GetScreenWidth() / camera.zoom, cameraPosition.y + GetScreenHeight() / camera.zoom}, camera);

        float gridStartX = floorf(worldTopLeft.x / CELL_SIZE) * CELL_SIZE;
        float gridStartY = floorf(worldTopLeft.y / CELL_SIZE) * CELL_SIZE;
        
        int cellCountVER = (worldBottomRight.x - worldTopLeft.x) / CELL_SIZE;
        int cellCountHOR = (worldBottomRight.y - worldTopLeft.y) / CELL_SIZE;

        /*DrawTextureV(HED_bound.texture, (Vector2){.x = HED_bound.rectangle.x, .y = HED_bound.rectangle.y}, WHITE);
        DrawRectangleLinesEx(HED_bound.rectangle, 3.0f, RED);
        if(CheckCollisionPointRec(GetMousePosition(), HED_bound.rectangle) {
        HED_bound.rectangle.width = GetMousePosition().x - HED_bound.rectangle.x;
        HED_bound.rectangle.height = GetMousePosition().y - HED_bound.rectangle.y;
        HED_bound.texture.width = HED_bound.rectangle.width;
        HED_bound.texture.height = HED_bound.rectangle.height;
        }*/

        for (int i = 1; i < cellCountVER; i++) {
        float x = gridStartX + (i * CELL_SIZE);
        DrawLine(x, worldTopLeft.y, x, worldBottomRight.y, WHITE);
        const char* numberText = TextFormat("%d", (int)x);
        DrawText(numberText, x + 3, 3, 15, YELLOW);
        }
        for (int i = 1; i < cellCountHOR; i++) {
        float y = gridStartY + (i * CELL_SIZE);
        DrawLine(worldTopLeft.x, y, worldBottomRight.x, y, WHITE);
        const char* numberText = TextFormat("%d", (int)-y);
        DrawText(numberText, 3, y + 3, 15, YELLOW);
        }

        DrawLine(worldTopLeft.x, 0, worldBottomRight.x, 0, ORANGE);
        DrawLine(0, worldTopLeft.y, 0, worldBottomRight.y, ORANGE);

        if (IsMouseButtonDown(MOUSE_BUTTON_LEFT) && algorithmStep == 0) {
        verticeData_G = (Vector2*)realloc(verticeData_G, (verticeCount_G + 1) * sizeof(Vector2));
        verticeData_G[verticeCount_G] = GetScreenToWorld2D(GetMousePosition(), camera);
        verticeCount_G++;
        }
        if(IsMouseButtonReleased(MOUSE_BUTTON_LEFT)) {
        Vertice *newVertice = (Vertice*)malloc(sizeof(Vertice));
        newVertice->verticeData = (Vector2*)malloc(verticeCount_G * sizeof(Vector2));
        memcpy(newVertice->verticeData, verticeData_G, verticeCount_G * sizeof(Vector2));
        newVertice->verticeCount = verticeCount_G;
        newVertice->next = verticeIndex->next;
        newVertice->intersectionPoints = NULL;
        newVertice->intersectionPointCount = 0;

        verticeIndex->next = newVertice;
        verticeIndex = newVertice;

        verticeCount_G = 0;

        IFArray *ifArray = generateIdenticalFunctions(worldTopLeft, worldBottomRight);
        int collidingPointCount = 0;
        Vector2* collidingPoints = (Vector2*)malloc(sizeof(Vector2));
        for(int i = 0; i < verticeIndex->verticeCount - 1; i++){
            for(int j = 0; j < ifArray->identicalFunctionCount; j++){
                if(CheckCollisionLines(verticeIndex->verticeData[i], verticeIndex->verticeData[i+1], ifArray->identicalFunctions[j].start, ifArray->identicalFunctions[j].end, &collidingPoints[collidingPointCount])){
                    collidingPointCount++;
                    collidingPoints = (Vector2*)realloc(collidingPoints, (collidingPointCount + 1) * sizeof(Vector2));
                }
            }
        }

        verticeIndex->intersectionPoints = (Vector2*)malloc(collidingPointCount * sizeof(Vector2));
        memcpy(verticeIndex->intersectionPoints, collidingPoints, collidingPointCount * sizeof(Vector2));
        verticeIndex->intersectionPointCount = collidingPointCount;

        free(collidingPoints);
        free(ifArray->identicalFunctions);
        free(ifArray);
        }
        if(IsKeyDown(KEY_LEFT_CONTROL) && IsKeyPressed(KEY_Z) && algorithmStep == 0){
        if(verticeIndex != head){
            Vertice *current = head;
            while(current->next != verticeIndex) current = current->next;
            verticeIndex = current;
        }
        }
        if(IsKeyDown(KEY_LEFT_CONTROL) && IsKeyPressed(KEY_R) && algorithmStep == 0){
        if(verticeIndex->next != NULL) verticeIndex = verticeIndex->next;
        }

        if(algorithmStep == 2){
        for(int i = 0; i < ifArray_D->identicalFunctionCount; i++){
            DrawLineV(ifArray_D->identicalFunctions[i].start, ifArray_D->identicalFunctions[i].end, ifArray_D->identicalFunctions[i].color);
        }
        }
        if(verticeData_G != NULL) DrawLineStrip(verticeData_G, verticeCount_G, GREEN); for(int i = 0; i < verticeCount_G; i++) DrawCircleV(verticeData_G[i], 5, YELLOW);
        if(head->next != NULL){
        current_D = head->next;
        while(current_D != verticeIndex->next){
            for(int i = 0; i < current_D->verticeCount; i++) DrawCircleV(current_D->verticeData[i], 5, YELLOW);
            DrawLineStrip(current_D->verticeData, current_D->verticeCount, GREEN);

            if(algorithmStep == 2) {
                for(int i = 0; i < current_D->intersectionPointCount; i++){
                    DrawCircleV(current_D->intersectionPoints[i], 5, BLUE);
                }
            }
            current_D = current_D->next;
        }
        }

        EndDrawing();
        if (IsMouseButtonDown(MOUSE_BUTTON_RIGHT)){
        Vector2 delta = GetMouseDelta();
        delta = Vector2Scale(delta, -1.0f/camera.zoom);
        camera.target = Vector2Add(camera.target, delta);
        if(ifArray_D != NULL) free(ifArray_D->identicalFunctions); free(ifArray_D);
        ifArray_D = generateIdenticalFunctions(worldTopLeft, worldBottomRight);
        }
        float wheel = GetMouseWheelMove();
        if (wheel != 0){            
        camera.zoom += (wheel*ZOOM_INCREMENT);
        if (camera.zoom < ZOOM_INCREMENT) camera.zoom = ZOOM_INCREMENT;
        if (camera.zoom > 1) camera.zoom = 1;
        if(ifArray_D != NULL) free(ifArray_D->identicalFunctions); free(ifArray_D);
        ifArray_D = generateIdenticalFunctions(worldTopLeft, worldBottomRight);
        }
        if(IsKeyPressed(KEY_SPACE)){
        if(ifArray_D != NULL) free(ifArray_D->identicalFunctions); free(ifArray_D);
        ifArray_D = generateIdenticalFunctions(worldTopLeft, worldBottomRight);
        algorithmStep = 2;
        }

    }

    UnloadTexture(HED_texture);
    free(ifArray_D->identicalFunctions);
    free(ifArray_D);
    Vertice *current_F = head;
    Vertice *next_F;
    while(current_F != NULL){
        next_F = current_F->next;
        free(current_F->verticeData);
        free(current_F->intersectionPoints);
        free(current_F);
        current_F = next_F;
    }
    free(verticeData_G);

    CloseWindow();       
    return 0;
}

The output_hed.png file is a white/transparent image thus: output_hed.png

\$\endgroup\$
4
  • \$\begingroup\$ Is a suitable output_hed.png file available online so that reviewers can experiment with the code? \$\endgroup\$ Jun 7, 2023 at 10:40
  • 1
    \$\begingroup\$ I've just added it, it's completely white and a PNG, so it might be hard to notice, but it's under the code section. \$\endgroup\$
    – Di0n
    Jun 7, 2023 at 10:48
  • \$\begingroup\$ Thanks. BTW, the indentation of this code is a mess, which is probably due to the use of tabs for indentation. Should I fix that for you? \$\endgroup\$ Jun 7, 2023 at 11:04
  • \$\begingroup\$ Oh sorry, don't bother yourself with it, they mostly occurred when porting from nvim to the site, they don't look like that \$\endgroup\$
    – Di0n
    Jun 7, 2023 at 11:15

2 Answers 2

6
\$\begingroup\$

The OP code is illegible, with no tabs, so after a clang-format pass I reviewed the following code instead.

#include <memory.h>
#include <raylib.h>
#include <raymath.h>
#include <stdio.h>
#include <stdlib.h>

typedef struct Vertice {
    Vector2 *verticeData;
    int verticeCount;
    struct Vertice *next;
    Vector2 *intersectionPoints;
    int intersectionPointCount;
} Vertice;

typedef struct IdenticalFunction {
    Vector2 start, end;
    Color color;
} IdenticalFunction;

typedef struct IFArray {
    IdenticalFunction *identicalFunctions;
    int identicalFunctionCount;
} IFArray;

typedef struct TextureBound {
    Texture2D texture;
    Rectangle rectangle;
} TextureBound;

IFArray *generateIdenticalFunctions(Vector2 worldTopLeft,
                                    Vector2 worldBottomRight) {
    const int IDENTICAL_FUNCTION_GAP = 50;
    float identicalFunctionStartX =
        floorf(worldTopLeft.x / IDENTICAL_FUNCTION_GAP) *
        IDENTICAL_FUNCTION_GAP;
    float identicalFunctionStartY =
        floorf(worldTopLeft.y / IDENTICAL_FUNCTION_GAP) *
        IDENTICAL_FUNCTION_GAP;
    int identicalFunctionCountX =
        (worldBottomRight.x - worldTopLeft.x) / IDENTICAL_FUNCTION_GAP;
    int identicalFunctionCountY =
        (worldBottomRight.y - worldTopLeft.y) / IDENTICAL_FUNCTION_GAP;

    IdenticalFunction *identicalFunctions = (IdenticalFunction *)malloc(
        (identicalFunctionCountX + identicalFunctionCountY) *
        sizeof(IdenticalFunction));
    for (int i = 1; i < identicalFunctionCountX; i++) {
        float x = identicalFunctionStartX + (i * IDENTICAL_FUNCTION_GAP);
        IdenticalFunction identicalFunction = {
            .start = (Vector2){x, worldTopLeft.y},
            .end = (Vector2){x, worldBottomRight.y},
            .color = RED};
        identicalFunctions[i - 1] = identicalFunction;
    }
    for (int i = 1; i < identicalFunctionCountY; i++) {
        float y = identicalFunctionStartY + (i * IDENTICAL_FUNCTION_GAP);
        IdenticalFunction identicalFunction = {
            .start = (Vector2){worldTopLeft.x, y},
            .end = (Vector2){worldBottomRight.x, y},
            .color = RED};
        identicalFunctions[i - 1 + identicalFunctionCountX] = identicalFunction;
    }

    IFArray *ifArray = (IFArray *)malloc(sizeof(IFArray));
    ifArray->identicalFunctionCount =
        identicalFunctionCountX + identicalFunctionCountY;
    ifArray->identicalFunctions = identicalFunctions;

    return ifArray;
}

int main() {
    const int WIDTH = 800;
    const int HEIGHT = 600;

    InitWindow(WIDTH, HEIGHT, "Window");

    Camera2D camera = {.offset = (Vector2){0, 0},
                       .target = (Vector2){0 - (float)GetScreenWidth() / 2,
                                           0 - (float)GetScreenHeight() / 2},
                       .rotation = 0.0f,
                       .zoom = 1.0f};

    SetTargetFPS(60);
    const int CELL_SIZE = 50;
    const float ZOOM_INCREMENT = 0.125f;

    int algorithmStep = 0;

    Vertice *head = (Vertice *)malloc(sizeof(Vertice));
    head->verticeData = NULL;
    head->verticeCount = 0;
    head->next = NULL;
    head->intersectionPoints = NULL;
    head->intersectionPointCount = 0;

    Vertice *current_D = head;
    Vertice *verticeIndex = head;

    Vector2 *verticeData_G = NULL;
    int verticeCount_G = 0;

    IFArray *ifArray_D = NULL;

    Image HED_image = LoadImage("output_hed.png");
    Texture2D HED_texture = LoadTextureFromImage(HED_image);
    UnloadImage(HED_image);
    TextureBound HED_bound = {
        .texture = HED_texture,
        .rectangle = (Rectangle){.x = 0 - HED_texture.width / 2.0f,
                                 .y = 0 - HED_texture.height / 2.0f,
                                 .width = HED_texture.width,
                                 .height = HED_texture.height}};

    while (!WindowShouldClose()) {
        BeginDrawing();
        ClearBackground(BLACK);
        BeginMode2D(camera);

        Vector2 cameraPosition = {camera.offset.x, camera.offset.y};
        Vector2 worldTopLeft = GetScreenToWorld2D(cameraPosition, camera);
        Vector2 worldBottomRight = GetScreenToWorld2D(
            (Vector2){cameraPosition.x + GetScreenWidth() / camera.zoom,
                      cameraPosition.y + GetScreenHeight() / camera.zoom},
            camera);

        float gridStartX = floorf(worldTopLeft.x / CELL_SIZE) * CELL_SIZE;
        float gridStartY = floorf(worldTopLeft.y / CELL_SIZE) * CELL_SIZE;

        int cellCountVER = (worldBottomRight.x - worldTopLeft.x) / CELL_SIZE;
        int cellCountHOR = (worldBottomRight.y - worldTopLeft.y) / CELL_SIZE;

        /*DrawTextureV(HED_bound.texture, (Vector2){.x = HED_bound.rectangle.x,
        .y = HED_bound.rectangle.y}, WHITE);
        DrawRectangleLinesEx(HED_bound.rectangle, 3.0f, RED);
        if(CheckCollisionPointRec(GetMousePosition(), HED_bound.rectangle) {
        HED_bound.rectangle.width = GetMousePosition().x -
        HED_bound.rectangle.x; HED_bound.rectangle.height = GetMousePosition().y
        - HED_bound.rectangle.y; HED_bound.texture.width =
        HED_bound.rectangle.width; HED_bound.texture.height =
        HED_bound.rectangle.height;
        }*/

        for (int i = 1; i < cellCountVER; i++) {
            float x = gridStartX + (i * CELL_SIZE);
            DrawLine(x, worldTopLeft.y, x, worldBottomRight.y, WHITE);
            const char *numberText = TextFormat("%d", (int)x);
            DrawText(numberText, x + 3, 3, 15, YELLOW);
        }
        for (int i = 1; i < cellCountHOR; i++) {
            float y = gridStartY + (i * CELL_SIZE);
            DrawLine(worldTopLeft.x, y, worldBottomRight.x, y, WHITE);
            const char *numberText = TextFormat("%d", (int)-y);
            DrawText(numberText, 3, y + 3, 15, YELLOW);
        }

        DrawLine(worldTopLeft.x, 0, worldBottomRight.x, 0, ORANGE);
        DrawLine(0, worldTopLeft.y, 0, worldBottomRight.y, ORANGE);

        if (IsMouseButtonDown(MOUSE_BUTTON_LEFT) && algorithmStep == 0) {
            verticeData_G = (Vector2 *)realloc(
                verticeData_G, (verticeCount_G + 1) * sizeof(Vector2));
            verticeData_G[verticeCount_G] =
                GetScreenToWorld2D(GetMousePosition(), camera);
            verticeCount_G++;
        }
        if (IsMouseButtonReleased(MOUSE_BUTTON_LEFT)) {
            Vertice *newVertice = (Vertice *)malloc(sizeof(Vertice));
            newVertice->verticeData =
                (Vector2 *)malloc(verticeCount_G * sizeof(Vector2));
            memcpy(newVertice->verticeData, verticeData_G,
                   verticeCount_G * sizeof(Vector2));
            newVertice->verticeCount = verticeCount_G;
            newVertice->next = verticeIndex->next;
            newVertice->intersectionPoints = NULL;
            newVertice->intersectionPointCount = 0;

            verticeIndex->next = newVertice;
            verticeIndex = newVertice;

            verticeCount_G = 0;

            IFArray *ifArray =
                generateIdenticalFunctions(worldTopLeft, worldBottomRight);
            int collidingPointCount = 0;
            Vector2 *collidingPoints = (Vector2 *)malloc(sizeof(Vector2));
            for (int i = 0; i < verticeIndex->verticeCount - 1; i++) {
                for (int j = 0; j < ifArray->identicalFunctionCount; j++) {
                    if (CheckCollisionLines(
                            verticeIndex->verticeData[i],
                            verticeIndex->verticeData[i + 1],
                            ifArray->identicalFunctions[j].start,
                            ifArray->identicalFunctions[j].end,
                            &collidingPoints[collidingPointCount])) {
                        collidingPointCount++;
                        collidingPoints = (Vector2 *)realloc(
                            collidingPoints,
                            (collidingPointCount + 1) * sizeof(Vector2));
                    }
                }
            }

            verticeIndex->intersectionPoints =
                (Vector2 *)malloc(collidingPointCount * sizeof(Vector2));
            memcpy(verticeIndex->intersectionPoints, collidingPoints,
                   collidingPointCount * sizeof(Vector2));
            verticeIndex->intersectionPointCount = collidingPointCount;

            free(collidingPoints);
            free(ifArray->identicalFunctions);
            free(ifArray);
        }
        if (IsKeyDown(KEY_LEFT_CONTROL) && IsKeyPressed(KEY_Z) &&
            algorithmStep == 0) {
            if (verticeIndex != head) {
                Vertice *current = head;
                while (current->next != verticeIndex) {
                    current = current->next;
                }
                verticeIndex = current;
            }
        }
        if (IsKeyDown(KEY_LEFT_CONTROL) && IsKeyPressed(KEY_R) &&
            algorithmStep == 0) {
            if (verticeIndex->next != NULL) {
                verticeIndex = verticeIndex->next;
            }
        }

        if (algorithmStep == 2) {
            for (int i = 0; i < ifArray_D->identicalFunctionCount; i++) {
                DrawLineV(ifArray_D->identicalFunctions[i].start,
                          ifArray_D->identicalFunctions[i].end,
                          ifArray_D->identicalFunctions[i].color);
            }
        }
        if (verticeData_G != NULL) {
            DrawLineStrip(verticeData_G, verticeCount_G, GREEN);
        }
        for (int i = 0; i < verticeCount_G; i++) {
            DrawCircleV(verticeData_G[i], 5, YELLOW);
        }
        if (head->next != NULL) {
            current_D = head->next;
            while (current_D != verticeIndex->next) {
                for (int i = 0; i < current_D->verticeCount; i++) {
                    DrawCircleV(current_D->verticeData[i], 5, YELLOW);
                }
                DrawLineStrip(current_D->verticeData, current_D->verticeCount,
                              GREEN);

                if (algorithmStep == 2) {
                    for (int i = 0; i < current_D->intersectionPointCount;
                         i++) {
                        DrawCircleV(current_D->intersectionPoints[i], 5, BLUE);
                    }
                }
                current_D = current_D->next;
            }
        }

        EndDrawing();
        if (IsMouseButtonDown(MOUSE_BUTTON_RIGHT)) {
            Vector2 delta = GetMouseDelta();
            delta = Vector2Scale(delta, -1.0f / camera.zoom);
            camera.target = Vector2Add(camera.target, delta);
            if (ifArray_D != NULL) {
                free(ifArray_D->identicalFunctions);
            }
            free(ifArray_D);
            ifArray_D =
                generateIdenticalFunctions(worldTopLeft, worldBottomRight);
        }
        float wheel = GetMouseWheelMove();
        if (wheel != 0) {
            camera.zoom += (wheel * ZOOM_INCREMENT);
            if (camera.zoom < ZOOM_INCREMENT) {
                camera.zoom = ZOOM_INCREMENT;
            }
            if (camera.zoom > 1) {
                camera.zoom = 1;
            }
            if (ifArray_D != NULL) {
                free(ifArray_D->identicalFunctions);
            }
            free(ifArray_D);
            ifArray_D =
                generateIdenticalFunctions(worldTopLeft, worldBottomRight);
        }
        if (IsKeyPressed(KEY_SPACE)) {
            if (ifArray_D != NULL) {
                free(ifArray_D->identicalFunctions);
            }
            free(ifArray_D);
            ifArray_D =
                generateIdenticalFunctions(worldTopLeft, worldBottomRight);
            algorithmStep = 2;
        }
    }

    UnloadTexture(HED_texture);
    free(ifArray_D->identicalFunctions);
    free(ifArray_D);
    Vertice *current_F = head;
    Vertice *next_F;
    while (current_F != NULL) {
        next_F = current_F->next;
        free(current_F->verticeData);
        free(current_F->intersectionPoints);
        free(current_F);
        current_F = next_F;
    }
    free(verticeData_G);

    CloseWindow();
    return 0;
}

When reviewing I always assume the original author got it to run, pass unit tests, and so on. For C code we typically assume at least "-Wall -Wextra -Wpedantic". But I don't see the floorf() call accompanied by #include <math.h> :-( Perhaps including raymath suffices.


typedef struct Vertice {

Singular of vertices? Other than that one time Cicero used it this way, conventionally the singular is Vertex. Oh, well, we'll roll with it.

typedef struct IdenticalFunction {

What a curious name! Definitely this is not a "function". Perhaps we have an EqualityResult?

I see five integers, there. I wonder if we could boil them down to a single hash, if equality ("identicalness"?) is all that matters.

    IdenticalFunction *identicalFunctions;

zomg, "you are lost in a twisty maze of functions, all identical".


IFArray *generateIdenticalFunctions(Vector2 worldTopLeft,
                                    Vector2 worldBottomRight) {

Maybe this could be generateHashes? Ok, I'll drop it.

Kudos on lovely parameter names. Often this would be worldTL, worldBR, but these names are nicer.

    float identicalFunctionStartX =
        floorf(worldTopLeft.x / IDENTICAL_FUNCTION_GAP) *
        IDENTICAL_FUNCTION_GAP;

I guess I'd be a little happier with gridStartX, since we're discretizing to a grid of 50. Consider introducing a macro / function named discretize(), which we use the same for X and Y.

    int identicalFunctionCountX =
        (worldBottomRight.x - worldTopLeft.x) / IDENTICAL_FUNCTION_GAP;

I don't understand what this means. In particular, it is not yet clear to me if there is some special relationship between numerator and denominator. Was there maybe a fractional part that was truncated to int? I would love to see a // comment about whether we expect that last grid to contain exactly zero pixels.

The bigger critique of this chunk of code is I would love to see it introduce a vocabulary word, a coarse grained grid, for the business concept we're describing here.

A similar approach would describe this as a mapping between a pair of related coordinate systems.

    for (int i = 1; i < identicalFunctionCountX; i++) {
        ...
        identicalFunctions[i - 1] = identicalFunction;

Uggh! You lost me there. Why didn't we start at 0? Why didn't we fill in the final grid?

DRY, same as above, couldn't an X macro or function be recycled for the Y axis?

We seem to be returning a bunch of vertical bands followed by many horizontal bands. The identifiers and the comments didn't really spell that out for us.


int main() {

zomg, it just never ends, does it? There's more than 200 LOC here. I haven't read one of them and already I'm crying out for "extract helper!"

Kudos on appropriately naming each magic number.

Although CELL_SIZE seems suspciously similar to IDENTICAL_FUNCTION_GAP.

    Vertice *head = ...

This is a terrific opportunity for Extract Helper.

I'm sure "D" and "G" are meaningful, but I confess that ATM I have no idea what current_D and verticeData_G denote.

    Texture2D HED_texture = ...

Consider extracting a helper here.

What is HED_bound for? It doesn't appear to be used. I don't think we evaluated for side effects??

Actually, some of the code so far could perhaps be safely deleted. Similarly the /*DrawTextureV ... */ remark. It may have been useful once, but now you're getting ready to merge down to main so it's time to prune away old cruft.


... GetScreenToWorld2D(
            (Vector2){cameraPosition.x + GetScreenWidth() / camera.zoom,
                      cameraPosition.y + GetScreenHeight() / camera.zoom}, ...

Oooh, lookit that! Another transform from one coordinate system to another. It's an important concept, so consider using a helper to clarify it.

        float gridStartX = floorf(worldTopLeft.x / CELL_SIZE) * CELL_SIZE;

Back to discretizing. "grid"? "cell"? Both are good names. I encourage you to pick one and standardize on just that.


            DrawText(numberText, x + 3, 3, 15, YELLOW);

Names for those magic numbers, please.

We're processing both vertical and horizontal bands. A single helper should be able to accomplish both tasks.


    int algorithmStep = 0;

Possibly we want a descriptive enum for that?

            verticeCount_G++;

Consider replacing that with a pre-increment:

            ... realloc(
                verticeData_G, (--verticeCount_G) * sizeof(Vector2));


            Vertice *newVertice = ...

Another good place to introduce a helper. Similarly for your computation of collidingPointCount.

                        collidingPointCount++;
                        collidingPoints = (Vector2 *)realloc(
                            collidingPoints,
                            (collidingPointCount + 1) * sizeof(Vector2));

Similar idiom as above, but phrased slightly differently. I don't understand the + 1 here. Surely the increment above took care of that? Oh, no it didn't. There's an invariant here, and it is correctly preserved, I just didn't read it properly the first time through, because the names are a little tricky and imply a situation that is off-by-one from what we've actually stored. Sigh.

The final malloc / memcpy is nice enough. But maybe it could be replaced by a simple pointer assignment?


We need a helper that will free ifArray_D->identicalFunctions and ifArray_D.

            algorithmStep = 2;

Give that obscure step a name, please.

Kudos on the orderly shutdown. It's nice to see everything carefully free'd. I wonder if raylib perhaps has some shutdown() function you'd like to call?


This code achieves most of its design goals.

I would be willing to delegate or accept maintenance tasks on this codebase.

\$\endgroup\$
8
  • 1
    \$\begingroup\$ I tab-expanded the code in the question to fix the formatting whilst you were writing this answer. That may invalidate some of your first point - please check. \$\endgroup\$ Jun 7, 2023 at 14:51
  • \$\begingroup\$ Hi, @J_H To clarify a few misunderstandings, the struct "IdenticalFunction" is literary my implementation of "f(x) = c "though in some cases it's rather a relation between sets than a function because in some cases it does not have a single "y" value for a single "x" value but the opposite. Oversimplified, it's a line with zero or undefined curvature. Next, "identicalFunctionStartX" and "gridStartX" must be two separate terms because I want the user to be able to change "IDENTICAL_FUNCTION_GAP" to allow faster but less precise collision checks or vice versa. PART 1 \$\endgroup\$
    – Di0n
    Jun 7, 2023 at 16:38
  • \$\begingroup\$ It also baffled you why I started the "for loop" from "1" instead of "0", that's because as you might have guessed we only need "n-1" cuts to divide an entirety into "n" parts. Also, to clear the naming conventions "_D" and "_G" they stand for "Drawing" and "Global" they are simply my solution to overcome some problems with linked lists, though I can imagine that they are bad practice. Lastly, "HED_bound" is there because I needed to obtain the world position of the "HED_texture" which was not a variable of it, so I created a struct which had a bounding rectangle of the texture. PART 2 \$\endgroup\$
    – Di0n
    Jun 7, 2023 at 16:39
  • \$\begingroup\$ Now what I didn't understand was the term "extract helper", seriously, what is it? I also didn't understand why you pre-dencremented the "verticeCount_G" variable? Could you elaborate a little further on them? The codebase had some changes as well, here's the repo: github.com/Di0n-0/Lineal-V2 P.S. I could very much use some well-needed help with the project, I feel like I bit a lot more than I chew with C. PART 3 \$\endgroup\$
    – Di0n
    Jun 7, 2023 at 16:40
  • \$\begingroup\$ Thank you, @TobySpeight. The OP posted v1 and you posted v6 with some extra whitespace. I'm unwilling to believe that either one of them are what OP would see in his IDE; there is loss in translation. For example at one point we have 3 + 1 conjuncts, for mouse-left & KEY_Z & step0 + "are we at head?". They're all at same level, with a pair of } braces directly above one another. It seems like we were about to combine it into a single if with 4 conjuncts, then bailed at the last moment. Similarly for step2 + for loop, and for "next non-null" + while. Code was almost ready for review. \$\endgroup\$
    – J_H
    Jun 7, 2023 at 17:31
4
\$\begingroup\$

I'm going to pick on a single line here; many of the points apply throughout:

collidingPoints = (Vector2*)realloc(collidingPoints, (collidingPointCount + 1) * sizeof(Vector2));

There's no need to cast the return value from the malloc() family of functions - they return void*, which is directly convertible to any object pointer in C.

As a style matter, it's good practice to use the sizeof the actual pointed-to element so that the reader can instantly see it's a match without having to search for the declaration. And it's good to have the sizeof expression first (which can avoid arithmetic overflow problems in some circumstances).

We really need to check that we didn't get a null pointer back before we use it. And if it's null, we shouldn't overwrite collidingPoints, as that's the one remaining pointer that can be used to free the memory we have - without that, we're condemned to have a leak.

The usual control flow for such reallocation is:

void *new_p = realloc(p, sizeof *p * new_element_count);
if (!new_p) {
    free(p);
    /* Add any other clean-up here. */
    return EXIT_FAILURE;
   /* EXIT_FAILURE is appropriate for main().
      For other functions, follow local conventions. */
}
p = new_p;
\$\endgroup\$
4
  • \$\begingroup\$ Thanks for your criticism and corrections, as for your first point I didn't really understand where in my code base a different sizeof() operation is used than the pointed to element/s could you clarify that for me? As for your second point, my goal was to well kind of implement the std::vector logic from cpp there like a dynamically allocated Vector2 array, was my mistake only that I did not check if it's null or not? \$\endgroup\$
    – Di0n
    Jun 7, 2023 at 15:57
  • \$\begingroup\$ You used sizeof (Vector2), which requires the reader to go back and confirm that that's the right size for *collidingPoints. Using sizeof with a type argument is usually inferior to using it with an expression argument. See how p = malloc(sizeof *p) is very obviously consistent, even if the declaration of p isn't visible? \$\endgroup\$ Jun 7, 2023 at 18:41
  • \$\begingroup\$ Two mistakes - not checking whether the returned pointer is null, and assigning it before the check. If realloc(p, new_size) returns a null pointer, the memory pointed to by p is unchanged, so p = realloc() leaves us with no way to free() it. \$\endgroup\$ Jun 7, 2023 at 18:44
  • \$\begingroup\$ Thanks, I understood it now \$\endgroup\$
    – Di0n
    Jun 7, 2023 at 19:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.