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

Window::get_size() should return (u32, u32) #471

Open
atomicbitty opened this issue Jan 12, 2021 · 3 comments
Open

Window::get_size() should return (u32, u32) #471

atomicbitty opened this issue Jan 12, 2021 · 3 comments

Comments

@atomicbitty
Copy link

atomicbitty commented Jan 12, 2021

The glfw.create_window() function uses u32 for the width and height, so I think it would make more sense to return a pair of u32s in Window::get_size() instead of a pair of i32s

@aloucks
Copy link
Contributor

aloucks commented Dec 4, 2021

glfwGetWindowSize returns values as i32 (as well as all of the other *size functions and events). Someone would need to confirm that they can never hold negative values and then change all of them to u32 for consistency. Note that I'm fairly certain that WindowEvent::Pos(i32, i32) can have valid negative values and explicitly not on the list below.

@RpxdYTX
Copy link

RpxdYTX commented Jan 1, 2022

But Glfw::create_window accepts u32 for width and height instead of i32, which is what it is in the C version.

@aloucks
Copy link
Contributor

aloucks commented Jan 1, 2022

But Glfw::create_window accepts u32 for width and height instead of i32, which is what it is in the C version.

The window can be resized.

All of the glfw functions (other than create) return or use signed int. Either this was an oversight in GLFW or there is some edge cases on some platform that could return negative numbers.

I think the best option here is to open an issue with GLFW. If they decide to change these to be unsigned values than we could update to match accordingly. If they don't want to change these to be unsigned, then we'll have our reason why.

In either case, we should continue to use the same type (currently signed int: i32) that glfw is using.

vosen pushed a commit that referenced this issue Nov 5, 2024
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

No branches or pull requests

3 participants