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

[BUG] Depth issue with mlx_delete_image() #52

Closed
MyNameIsTrez opened this issue Aug 17, 2022 · 2 comments
Closed

[BUG] Depth issue with mlx_delete_image() #52

MyNameIsTrez opened this issue Aug 17, 2022 · 2 comments
Labels
Bug Something isn't working or should get fixed

Comments

@MyNameIsTrez
Copy link
Contributor

Describe the bug
There is an issue with depth, see the code below.

To Reproduce

void	loop(void *param)
{
	t_data				*data;
	static mlx_image_t	*image = NULL;
	int32_t				instance_1_id;
	int32_t				instance_2_id;

	data = param;

	// WHEN THIS IS COMMENTED IN, THE SECOND SQUARE DOESN'T GET DRAWN
	if (image != NULL)
	{
		mlx_delete_image(data->mlx, image);
		image = NULL;
	}

	if (image == NULL)
	{
		image = mlx_new_image(data->mlx, 100, 100);
		ft_memset(image->pixels, 255, image->width * image->height * sizeof(int));

		instance_1_id = mlx_image_to_window(data->mlx, image, 0, 0);
		instance_2_id = mlx_image_to_window(data->mlx, image, 100, 100);
	}
}

t_i32	main(void)
{
	t_data	data;

	ft_bzero(&data, sizeof(t_data));
	data.mlx = mlx_init(200, 200, "", false);
	mlx_loop_hook(data.mlx, loop, &data);
	mlx_loop(data.mlx);
	return (EXIT_SUCCESS);
}

Expected behavior
The second square to be drawn, see the comment in the code.

Additional context
If you replace the const float depth = mlxctx->zdepth; with const float depth = 1000000; inside of mlx_update_matrix() you'll see that everything works as expected, though hardcoding it to such a low value is obviously problematic when you create more than a million instances during the lifetime of the program. Ideally you also wouldn't set it to a huge ass constant since it'd still be technically reachable if you ran the program long enough, though how you resolve this issue is up to you.

@W2Wizard W2Wizard added the Bug Something isn't working or should get fixed label Aug 17, 2022
@MyNameIsTrez MyNameIsTrez changed the title [BUG] This code fails to draw the second square when using mlx_delete_image() [BUG] Depth issue with mlx_delete_image() Aug 17, 2022
@MyNameIsTrez
Copy link
Contributor Author

MyNameIsTrez commented Aug 17, 2022

@W2Wizard I've thought about the issue a bit more, and I am almost certain that the problem is floating point shenanigans.

If you look at mlx_update_matrix() in src/mlx_window.c you will find the line const float depth = mlxctx->zdepth;, so a float.

This local depth variable is then used with -2.f / (depth - -depth) (-> -2 / 2 * depth -> -depth) and with -((depth + -depth) / (depth - -depth)) (-> -(0/(2 * depth)) -> -0 -> 0).

I think there are floating point issues here because of how we saw on my computer that when I set depth to a million or something things looked fine, but that when I added an extra zero to the end all hell broke loose.

Marius came up with the suggestion to just throw out -2.f / (depth - -depth) and -((depth + -depth) / (depth - -depth)) entirely and to replace both of those with 0 (or something similar to it that achieves the camera's depth being 0). The idea behind that is that instead of moving the camera along or setting it really far away (but again, not too far away, cause after a certain point you have floating point bullshit), you'd instead hardcode the camera's depth to always be zero and just make the depth sorting of the instances be the other way around. We hadn't tried this, but it seems like the least buggy solution here and like it should work well, and would also make that code much more readable.

@W2Wizard
Copy link
Collaborator

Yes this matrix seems to be the bane of existence since I started this.

Your given solution with @Mariusmivw does seem to solve the issue in the code you have given as a reproduction.

However it breaks transparency:
image

@W2Wizard W2Wizard pinned this issue Aug 30, 2022
@W2Wizard W2Wizard closed this as not planned Won't fix, can't repro, duplicate, stale Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working or should get fixed
Projects
None yet
Development

No branches or pull requests

2 participants