Fix crash in gradient sampling #3828
Conversation
CC: @Dherse
| @@ -1249,7 +1249,11 @@ fn sample_stops(stops: &[(Color, Ratio)], mixing_space: ColorSpace, t: f64) -> C | |||
|
|
|||
| let (col_0, pos_0) = stops[low - 1]; | |||
| let (col_1, pos_1) = stops[low]; | |||
| let t = (t - pos_0.get()) / (pos_1.get() - pos_0.get()); | |||
| let mut delta = pos_1.get() - pos_0.get(); | |||
| if delta == 0.0 { | |||
There was a problem hiding this comment.
can we get into trouble if delta isn't equal to 0.0, but very close to it?
There was a problem hiding this comment.
The current way this is dealt with seems somewhat arbitrary to me. Wouldn't it make more sense to simply make t either 0.0, 0.5 or 1.0 in the event that two stops are the same?
Maybe I misunderstand what's going on though.
There was a problem hiding this comment.
I agree. As-is it behaves as if the two stops were actually very far apart (the full 100% = 1.0) and which of the stops "wins" depends on where the stops are along the gradient.
|
https://discord.com/channels/1054443721975922748/1088371867913572452/1224684053870153848 Some minor discussion on discord |
| if delta == 0.0 { | ||
| delta = 1.0; | ||
| } | ||
| let t = (t - pos_0.get()) / delta; |
There was a problem hiding this comment.
We can also deduplicate stops beforehand, but I think we should in any case clamp t to 0.0..=1.0 here. You never know what's gonna happen with floats.
|
@frozolotl still on your radar? |
|
Closing due to inactivity. Feel free to reopen if you still want to work on this! |
Closes #3826.
CC: @Dherse