Initial Window Size
daxpedda opened this issue ยท 21 comments
Currently the initial window size is always 0 in width and height, this will lead to panics when calling buffer_mut()
if the size wasn't set with resize()
depending on the platform:
- Wayland: panics.
- MacOS: doesn't panic.
- Redox: doesn't panic.
- Web: doesn't panic, but will panic when calling
present()
orfetch()
(#104). - Windows: panics.
- X11: doesn't panic.
I would prefer if we make this consistent on all platforms. My current ideas:
- Require an initial size in
Surface::new()
. - Set the initial size to the window size (don't know if that's possible on all platforms).
- Introduce a new error variant instead of panicking.
Currently leaning towards idea 1.
Idea 1 sounds good to me, since an initial window size is also required by wgpu
.
This should definitely be consistent across backends, if nothing else.
Set the initial size to the window size (don't know if that's possible on all platforms).
I don't believe that's possible on Wayland, no. Only the code that created the surface has this information.
Introduce a new error variant instead of panicking.
This was discussed somewhat in #65, and I think it was generally agreed that panicking is a better way to handle programmer error than returning an error variant. So I think panicking consistently would be reasonable. Possibly if things were restructured a bit that could be handled in top level code instead of separately in each backend.
But requiring passing an initial size would statically prevent errors, so that's better if it doesn't make the library too much harder to use. Good point about Wgpu requiring that. That makes it seem more reasonable.
Doesn't the contract of buffer_mut
state that you always must call resize
first? Or are you looking into making this unnecessary?
I call resize
each frame even if the size hasn't changed. For consistency's sake it's just better to ensure the buffer size always matches the window size.
Or are you looking into making this unnecessary?
Yeah, that was the original thought.
Or are you looking into making this unnecessary?
Yeah, that was the original thought.
Hmm, what's the point of making it unnecessary? "every frame starts with a resize
call" is a good rule of thumb as-is, I can't really see a reason to try to eliminate it if you're going to call resize
every other time anyway.
Truth be told, I'd advocate for combining resize
and buffer_mut
into something like begin_frame
.
Hmm, what's the point of making it unnecessary? "every frame starts with a
resize
call" is a good rule of thumb as-is, I can't really see a reason to try to eliminate it if you're going to callresize
every other time anyway.
It's an improvement, I don't see why we should have an invalid state if it's avoidable (it's not clear to me yet if it can be avoided). I'm also really really not a big fan of panics in libraries, but I'm not sure who shares that opinion.
Truth be told, I'd advocate for combining
resize
andbuffer_mut
into something likebegin_frame
.
I'm not sure why we should require passing in a size when it's not required, it's up to you if you wanna do it that way but I wouldn't want to force it on everybody else. But it's a very good alternative solution.
I'm not sure why we should require passing in a size when it's not required, it's up to you if you wanna do it that way but I wouldn't want to force it on everybody else. But it's a very good alternative solution.
what do you mean "force"? is passing the size for each frame somehow a problem or inconvenience? don't you need those anyway in order to use the buffer properly? is there any use-case that would prevent someone from being able to pass the size? is it too expensive for softbuffer
to check if the passed-in size is already the same as the buffer size and simply reuse the buffer if so?
(I'm just confused)
is passing the size for each frame somehow a problem or inconvenience?
It isn't. It's why I said this is a very good alternative. But it doesn't have a benefit either unless it's an alternative to passing the size at the start.
I see the point of course that we could encourage users to always resize before drawing or fetching, but that's a completely different issue then whats being discussed here.
It isn't. It's why I said this is a very good alternative.
I'm not trying to be adversarial here. I just don't know why you would want to elide the resize
call in the first place. Does it have some obvious benefit that I'm not seeing?
I'm not trying to be adversarial here.
All good!
I just don't know why you would want to elide the
resize
call in the first place.
I'm not particularly against it, but I'm not particularly for it either.
Does it have some obvious benefit that I'm not seeing?
I don't see any particular benefit to it, which is why I'm not particularly in favor of it.
Did you maybe mean downside instead of benefit? No, but I don't see a reason to add it either, except again, as an alternative to what we were initially discussing.
Did you maybe mean downside instead of benefit?
I meant a benefit to eliding the call to resize
. In case you're curious, I just completed a round of benchmarks on some feature branches and resize
calls typically take under a microsecond, even going from a window size of 1600x1200 to 2880x1800. I don't see why anyone would need to eliminate this call by providing an initial size for the surface, nor why they would need to care enough to call resize
only when surface size is changed rather than just every frame.
I didn't really dispute anything you are saying here.
But as I pointed out in #106 (comment), eliding the resize()
call is only an indirect goal, my point is to prevent leaving Softbuffer in an invalid state that forces us to panic.
Maybe this is what was confusing you?
my point is to prevent leaving Softbuffer in an invalid state that forces us to panic.
Oh, because the buffer size must always be non-zero, but when you first create the Surface
, it is zero anyway? That does make more sense, yeah.
My 2ยข: Passing the size every frame as part of the drawing (.buffer_mut([width, height])
?) would likely lead to simpler (less repetitive) code for callers, compared to requiring it at creation โ the size only needs to be considered and translated in one place instead of two. Additionally, it makes sense from the perspective of getting the buffer length right โ the code that is about to write will specify the length and layout it needs to write into.
.buffer_mut([width, height])
?
They should probably be separate arguments rather than just an array :)
I disagree, actually โ it would be easier to write graphics API glue if everyone used some kind of 2d vector type instead of separate x and y args, and arrays are cheap whether or not you care โ but that's not relevant to this issue.
I disagree, actually โ it would be easier to write graphics API glue if everyone used some kind of 2d vector type instead of separate x and y args, and arrays are cheap whether or not you care โ but that's not relevant to this issue.
I use tuples for that case. You're right though, this API change belongs in a separate discussion
Man, this issue is getting really off-topic.
I would also prefer using a shared 2D vector type, but I don't like using an array instead. Indeed I would be more happy with a tuple.
I apologize for bringing up an unrelated issue in my example. I intended only to suggest that the simplest API that is convenient for both parties (application and library), and free of invalid states, is one where the size is always specified at the moment the buffer is being obtained.
(Though, on further thought, perhaps in some cases there could be latency improvements by resizing sooner than just before drawing? Depends on how the backends treat a resize operation.)
sooner than just before drawing
This isn't really a thing because you want to draw immediately after a window resize. So there's never going to be any significant gap between resizing the buffer and drawing the next frame.
IMHO, window resizing on every frame is superfluous if the window is not resizable, which is a common case with games. In these cases I'd prefer not having to call resize
separately. How about a new constructor method eg. Surface::with_size
?
- MacOS: doesn't panic.
(Does panic as of e10c3609)