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

[rtext] Don't call TextLength in DrawTextEx and MeasureTextEx #4731

Closed
wants to merge 1 commit into from

Conversation

Peter0x44
Copy link
Contributor

Currently, DrawTextEx and MeasureTextEx call TextLength first to scan for the null terminator, before beginning a loop on all of the codepoints. Instead, check for the null terminator during the loop.

Currently, DrawTextEx and MeasureTextEx call TextLength first to scan
for the null terminator, before beginning a loop on all of the
codepoints. Instead, check for the null terminator during the loop.
@raysan5
Copy link
Owner

raysan5 commented Jan 27, 2025

This change increase code complexity on basic raylib functions, I need to think about it, did you measure the performance gain?

@asdqwe
Copy link
Contributor

asdqwe commented Jan 27, 2025

Out of curiosity, did this test:
#include "raylib.h"
int main(void) {
    InitWindow(800, 450, "test");
    SetTargetFPS(60);
    while (!WindowShouldClose()) {
        BeginDrawing();
        ClearBackground(RAYWHITE);

        double begin = GetTime();
        for (int i = 0; i < 40; i++) { DrawTextEx(GetFontDefault(), "The quick brown fox jumps over the lazy dog. The quick brown fox jumps over the lazy dog. The quick brown fox jumps over the lazy dog.", (Vector2) { 0, i*10 }, 10.0f, 1.0f, BLACK); }
        double end = GetTime();
        TraceLog(LOG_INFO, "time:%f", (end - begin));

        EndDrawing();
    }
    CloseWindow();
    return 0;
}

img

Got this results for `PLATFORM_DESKTOP_GLFW` on Linux Mint 22.0:

Results for current master branch:

INFO: time:0.005673
INFO: time:0.005729
INFO: time:0.005710
INFO: time:0.005678
INFO: time:0.005810
INFO: time:0.005697
INFO: time:0.005680
INFO: time:0.005732
INFO: time:0.005714
INFO: time:0.005826
INFO: TEXTURE: [ID 2] Unloaded texture data from VRAM (GPU)
INFO: SHADER: [ID 3] Default shader unloaded successfully
INFO: TEXTURE: [ID 1] Default texture unloaded successfully
INFO: Window closed successfully

Results for this PR:

INFO: time:0.005657
INFO: time:0.005714
INFO: time:0.005699
INFO: time:0.005657
INFO: time:0.005687
INFO: time:0.005702
INFO: time:0.005652
INFO: time:0.005712
INFO: time:0.005709
INFO: time:0.005660
INFO: TEXTURE: [ID 2] Unloaded texture data from VRAM (GPU)
INFO: SHADER: [ID 3] Default shader unloaded successfully
INFO: TEXTURE: [ID 1] Default texture unloaded successfully
INFO: Window closed successfully
Got this results for `PLATFORM_WEB` on Chromium 130.0.6723.116 on Linux Mint 22.0:

Results for current master branch:

test.html:235 INFO: time:0.004600
test.html:235 INFO: time:0.005200
test.html:235 INFO: time:0.004800
test.html:235 INFO: time:0.004900
test.html:235 INFO: time:0.004500
test.html:235 INFO: time:0.004800
test.html:235 INFO: time:0.004900
test.html:235 INFO: time:0.005000
test.html:235 INFO: time:0.004600
test.html:235 INFO: time:0.004700

Results for this PR:

test.html:235 INFO: time:0.004600
test.html:235 INFO: time:0.004700
test.html:235 INFO: time:0.004400
test.html:235 INFO: time:0.004300
test.html:235 INFO: time:0.004500
test.html:235 INFO: time:0.004800
test.html:235 INFO: time:0.004400
test.html:235 INFO: time:0.004300
test.html:235 INFO: time:0.004600
test.html:235 INFO: time:0.004800

Based on my napkin math, there was around 0.7% improvement on PLATFORM_DESKTOP_GLFW and around 5.5% improvement on PLATFORM_WEB.

@Peter0x44
Copy link
Contributor Author

Another thing worth mentioning that I didn't change, MeasureTextEx has the variable "byteCounter" which actually counts the number of codepoints in the longest line, not bytes. Probably that should be renamed, but I didn't do it in this PR.

@raysan5
Copy link
Owner

raysan5 commented Jan 28, 2025

@Peter0x44 @asdqwe Despite the small gain of this change, I decided not to merge it for now. Those two functions are very essential raylib functions and specially for new users, functions to check how code is implemented. The proposed change increases code complexity and legibility for a newcomer.

In any case, a comment could be left right before the for() loop explaining the improvement, as a learning reference.

@raysan5 raysan5 closed this Jan 28, 2025
@Peter0x44
Copy link
Contributor Author

I think it's a very minor change and beginners should not have a problem reading this either way.
And as I mentioned, there are other readability problems to review first.

@orcmid
Copy link
Contributor

orcmid commented Jan 28, 2025

@Peter0x44 @raysan5

I think it's a very minor change and beginners should not have a problem reading this either way. And as I mentioned, there are other readability problems to review first.

Although likely unrelated, a more interesting defensive-programming treatment is ensuring that there is always a null (including in UTF-8 or wide characters) at the end of any char[]. That can be handled by allocating one (or more) char than the set capacity so there is always one (or more) \0 off the end.

@Peter0x44
Copy link
Contributor Author

@orcmid I have no idea what that has to do with this PR because neither of these functions allocate memory

@orcmid
Copy link
Contributor

orcmid commented Jan 28, 2025

@orcmid I have no idea what that has to do with this PR because neither of these functions allocate memory

Correct. This is about having the operations that do allocate it provide a never over-written bumper off the end. Then these functions are safe :). As I said, "likely unrelated."

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.

4 participants