[rshapes] Review DrawRectangleLines() pixel offset

[RadsammyT]

Reviews DrawRectangleLine() so that the +1 offset is replaced with a +0.5 offset divided by the camera zoom (actually m0 from rlGetMatrixModelview(). m5 does have the zoom also but I am unsure if anything could throw either of those off in a 2D context). This also fixes an issue where the rectangle is drawn incorrectly if rendered at tiny sizes with a big enough zoom value.

Examples (from issue #3973's example snippet, modified for PR example):

modified 3973 example snippet

#include "raylib.h"
#include "rlgl.h"
#include <stdio.h>
#include <string.h>
#include <stdbool.h>

void matrixToString(Matrix *m, char *str) {
	sprintf(str,
			"%.02f, %.02f, %.02f, %.02f\n"
			"%.02f, %.02f, %.02f, %.02f\n"
			"%.02f, %.02f, %.02f, %.02f\n"
			"%.02f, %.02f, %.02f, %.02f\n"
			, m->m0, m->m4, m->m8, m->m12
			, m->m1, m->m5, m->m9, m->m13
			, m->m2, m->m6, m->m10, m->m14
			, m->m3, m->m7, m->m11, m->m15);
}

int main(void) {
	const int screenWidth = 800;
	const int screenHeight = 450;

	InitWindow(screenWidth, screenHeight, "Line Drawing Functions Test");

	Camera2D camera = {0};
	camera.target = (Vector2){0.0f, 0.0f};
	camera.offset = (Vector2){screenWidth / 2.0f, screenHeight / 2.0f};
	camera.rotation = 0.0f;
	camera.zoom = 64.0f;

	SetTargetFPS(60);
	bool toggle = false;
	Matrix mat;
	char buf[512];
	while (!WindowShouldClose()) {
		if (IsKeyDown(KEY_RIGHT))
			camera.target.x += 0.1;
		if (IsKeyDown(KEY_LEFT))
			camera.target.x -= 0.1;
		if (IsKeyDown(KEY_UP))
			camera.target.y -= 0.1;
		if (IsKeyDown(KEY_DOWN))
			camera.target.y += 0.1;
		if (IsKeyDown(KEY_S)) 
			camera.zoom += 0.5;
		if (IsKeyDown(KEY_D)) 
			camera.zoom -= 0.5;
		camera.zoom += GetMouseWheelMove();
		if (IsKeyPressed(KEY_T)) toggle = !toggle;
		BeginDrawing();
			ClearBackground(RAYWHITE);
			matrixToString(&mat, buf);
			BeginMode2D(camera);
				mat = rlGetMatrixModelview();
				DrawRectangleRec((Rectangle) { 0.0f, 0.0f, 1.0f, 1.0f}, RED);
				if (toggle) {
					DrawLineV((Vector2){0.0, 0.0}, (Vector2){1.0f, 0.0}, BLUE);
					DrawLineV((Vector2){1.0f, 0.0}, (Vector2){1.0f, 1.0f}, BLUE);
					DrawLineV((Vector2){1.0f, 1.0f}, (Vector2){0.0, 1.0f}, BLUE);
					DrawLineV((Vector2){0.0, 1.0f}, (Vector2){0.0, 0.0}, BLUE);
				} else {
					DrawRectangleLines(0, 0, 1, 1, GREEN);
				}
			EndMode2D();
			DrawRectangle(1, 1, 30, 30, RED);
			DrawRectangleLines(1, 1, 30, 30, BLUE);
			DrawText(buf, 0, 0, 12, BLACK);
		EndDrawing();
	}
	CloseWindow();
	return 0;
}

Old (taken @ commit 63ae57d):

New:

[raysan5]

@RadsammyT does it address #4075? Solution is ok but I try to minimize rlgl functions dependency for rshapes and it adds a very specific dependency (in the sense that is not a solution that makes it easy the porting of the module to other backends). Also, it solves the issue for DrawRectangleLines(), what about the other functions using RL_LINES? Do they all require the same "hack"?

[RadsammyT]

@RadsammyT does it address #4075? Solution is ok but I try to minimize rlgl functions dependency for rshapes and it adds a very specific dependency (in the sense that is not a solution that makes it easy the porting of the module to other backends). Also, it solves the issue for DrawRectangleLines(), what about the other functions using RL_LINES? Do they all require the same "hack"?

Oh, I haven't considered that issue in particular. When running the code example for that issue, it seems to be fine on my end:

So I modified DrawLine() (and its respective vector version) to offset by 0.5/zoom:

// Draw a line (using gl lines)
void DrawLine(int startPosX, int startPosY, int endPosX, int endPosY, Color color)
{
    Matrix mat = rlGetMatrixModelview();
    float zoomElement = 0.5f / mat.m0;
    rlBegin(RL_LINES);
        rlColor4ub(color.r, color.g, color.b, color.a);
        rlVertex2f((float)startPosX + zoomElement, (float)startPosY + zoomElement);
        rlVertex2f((float)endPosX + zoomElement, (float)endPosY + zoomElement);
    rlEnd();
}

// Draw a line (using gl lines)
void DrawLineV(Vector2 startPos, Vector2 endPos, Color color)
{
    Matrix mat = rlGetMatrixModelview();
    float zoomElement = 0.5f / mat.m0;
    rlBegin(RL_LINES);
        rlColor4ub(color.r, color.g, color.b, color.a);
        rlVertex2f(startPos.x + zoomElement, startPos.y + zoomElement);
        rlVertex2f(endPos.x + zoomElement, endPos.y + zoomElement);
    rlEnd();
}

And it seems to work just fine.

For #4130, different result but still a bad thing nonetheless: instead of the top line being culled, its the bottom line.

As for #3931 it doesn't break that one either.

I haven't tested this offset method with other RL_LINES drawing functions so I can't say for sure, but the answer for the DrawLine() case in particular is a definite "Maybe".

[casperbear]

Tried RayLib from branch https://github.com/RadsammyT/raylib/tree/rm-DrawRecLines-offset today, and in my app drawing of small rectangles is still broken. I mean it's drawn perfectly in RayLib 5 release, then was broken at some point in master branch, and is also broken in https://github.com/RadsammyT/raylib/tree/rm-DrawRecLines-offset. Attached two tiny images.

RayLib 5 release:

https://github.com/RadsammyT/raylib/tree/rm-DrawRecLines-offset

Images are cropped from exactly the same places on the screen. So the X of left vertical gray line, the X of left vertical black line, the Y of bottom horizontal gray line, and Y of bottom horizontal black line, those 4 values are correct, the rest is broken.

        DrawRectangleLines(screenWidth / 2 - 3, screenHeight / 2 - 3, 6, 6, LIGHTGRAY);
        DrawRectangleLines(screenWidth / 2 - 2, screenHeight / 2 - 2, 4, 4, BLACK);

[raysan5]

@casperbear Here the reason for the change: #3884

As expected, that fix, "broke" pixel-perfect line drawing... and introduced a GPU/drivers-dependant behaviour... but previous implemententation was drawing one rectangle for each line... and it also could generate some artifacts depending on the GPU/drivers... not easy to find a solution working for all use-cases...

@RadsammyT Please, could you take a look to this last issue introduced, just in case you can also reproduce it or if it could be addressed somewhat?

[RadsammyT]

Please, could you take a look to this last issue introduced, just in case you can also reproduce it or if it could be addressed somewhat?

I checked out the 5.0 release and can confirm #3884 happens on my machine (using DrawRectangleLines, QUADS version):

Screencast_20240825_074419.webm

This occurs even on DrawRectangleLinesEx when converting naively- but I figured out you can work around this issue by performing a similar hack I propose here in this PR: Divide 1 by the camera's zoom (be it Matrix.m0/m5 or just from the cam struct entirely) in the lineThick argument.

Screencast_20240825_080735.webm

C code snippet for above videos

#include "raylib.h"
#include "rlgl.h"
#include <stdio.h>

void matrixToString(Matrix *m, char *str) {
	sprintf(str,
			"%.02f, %.02f, %.02f, %.02f\n"
			"%.02f, %.02f, %.02f, %.02f\n"
			"%.02f, %.02f, %.02f, %.02f\n"
			"%.02f, %.02f, %.02f, %.02f\n"
			, m->m0, m->m4, m->m8, m->m12
			, m->m1, m->m5, m->m9, m->m13
			, m->m2, m->m6, m->m10, m->m14
			, m->m3, m->m7, m->m11, m->m15);
}
int main() {
	InitWindow(800, 800, "Bug");
	Camera2D cam = (Camera2D) {
		.zoom = 1
	};
	Matrix m;
	char buf[512];
	while(!WindowShouldClose()) {
		if(IsKeyPressed(KEY_R)) cam.zoom = 1;
		BeginDrawing();
			matrixToString(&m, buf);
			ClearBackground(BLACK);
			DrawText(buf, 400, 400, 20, WHITE);
			DrawText(RAYLIB_VERSION, 10, 800 - 20, 15, WHITE);
			BeginMode2D(cam);
				m = rlGetMatrixModelview();
				DrawLine(10, 10, 210, 10, WHITE);
				DrawLine(210, 10, 210, 210, WHITE);
				DrawLine(210, 210,10, 210, WHITE);
				DrawLine(10, 210, 10, 10, WHITE);
#if 1 // set to 0 to replicate bug
				DrawRectangleLinesEx((Rectangle){300, 10, 200, 200}, 1 / cam.zoom, WHITE);
#else
				DrawRectangleLines(300, 10, 200, 200, WHITE);
#endif
				cam.zoom -= GetFrameTime() * 0.1;
			EndMode2D();
		EndDrawing();
	}
}

Of course this implies that we might have to make DrawRectangleLines() be a wrapper to, or be similar in implementation of DrawRectangleLinesEx() to fix QUADS flickering and use rlgl, though...

[casperbear]

@raysan5 I've read through #3884 and understand the dilemma better now. Well, I personally can simply extract DrawRectangleLines code from RayLib 5, my app needs just several DrawRectangleLines each frame, quality and predictability is more important than performance.

[raysan5]

@RadsammyT After lot of thinking, I'm merging this improvement... Thank you very much for the review!