Remove DamageOutOfRange error
#314
Conversation
madsmtm
force-pushed
the
madsmtm/remove-damage-out-of-range
branch
from
7540242 to
551b385
Compare
madsmtm
force-pushed
the
madsmtm/remove-damage-out-of-range
branch
from
551b385 to
ab88d81
Compare
| let x = rect.x.try_into().unwrap_or(i32::MAX); | ||
| let y = rect.y.try_into().unwrap_or(i32::MAX); |
| // TODO: Android requires the damage rect _at lock time_ | ||
| // Since we're faking the backing buffer _anyway_, we could even fake the surface lock | ||
| // and lock it here (if it doesn't influence timings). | ||
| let _ = damage; |
There was a problem hiding this comment.
It does this because the region can be expanded by the system, requesting the user to actually redraw a larger region. I'm not sure if/when this is used, or if corruption/artifacts occur when the enlarged damage region is not re-rendered.
EDIT: This comment was just moved, I don't remember why I didn't clarify that in the original place.
| rect.x.try_into().unwrap_or(u16::MAX), | ||
| rect.y.try_into().unwrap_or(u16::MAX), |
There was a problem hiding this comment.
Same here, it seems that not having x or y means the rect should be infinitely large, starting at 0 rather than MAX (and ending at MAX, making it 0×0)?
| width: NonZeroU32::new(u32::MAX).unwrap(), | ||
| height: NonZeroU32::new(u32::MAX).unwrap(), |
There was a problem hiding this comment.
NonZeroU32::MIN and ::MAX seem to be available since Rust 1.70, our MSRV is already 1.71.
| rect.width.get().min(buffer_width.get()).into(), | ||
| rect.height.get().min(buffer_height.get()).into(), |
There was a problem hiding this comment.
Shouldn't the width/height take into account the x/y offset before calling .min()?
Maybe explicitly linking this portion of the documentation:
|
There is a very natural thing to do if we receive a damage rectangle outside supported bounds: Clamp it to be inside. This is also what's done by Wayland IIUC, and probably on other platforms as well, so let's do it in Softbuffer as well, and avoid the needless error case.
This also allows us to make
presentforward directly topresent_with_damage, which is a nice internal simplification.Finally, this exposed latent bugs with out-of-bounds damage regions on Windows, Web and X11, I've fixed those as well.
Tested on: