Remove DamageOutOfRange error
#314
Conversation
madsmtm
force-pushed
the
madsmtm/remove-damage-out-of-range
branch
2 times, most recently
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); |
There was a problem hiding this comment.
Hmm, no, rect.x is a u32, the problem we're protecting against is if it's a large value.
| 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)?
| 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()?
There was a problem hiding this comment.
I'm not entirely sure, but I think no? See the docs for dirtyWidth:
https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/putImageData#dirtywidth
I'd assume it's correct then to pass the size of the surface, though to be honest, I haven't really tested the Web part that much, only that it doesn't crash with width/height = u32::MAX.
Maybe explicitly linking this portion of the documentation:
|
madsmtm
force-pushed
the
madsmtm/remove-damage-out-of-range
branch
from
ab88d81 to
65b9295
Compare
Damage that fall outside the surface is ignored.
madsmtm
force-pushed
the
madsmtm/remove-damage-out-of-range
branch
from
65b9295 to
55ce8e9
Compare
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:
AndroidDoesn't use damageCoreGraphicsDoesn't use damage