Assert dimensions of quads are normal in iced_tiny_skia
#2082
Conversation
tiny_skia/src/backend.rs
Outdated
| @@ -154,7 +154,26 @@ impl Backend { | |||
| border_width, | |||
| border_color, | |||
| } => { | |||
| let physical_bounds = (*bounds + translation) * scale_factor; | |||
| // Tiny Skia's `PathBuilder::finish` returns `None` on infinites and NaNs which causes iced to panic. | |||
| // Certain widget combinations can lead to an infinite in either dimension hence the need for this check. | |||
There was a problem hiding this comment.
What kind of valid widget combinations could lead to an infinite quad? It's important we don't try to fix invalid use cases.
There was a problem hiding this comment.
That's a good point! I mainly tried to fix the panic as reported in #2066, but the code is likely wrong since the submitter is sticking a TextInput into a Scrollable which is what directly leads to the panic. It also doesn't do what they want it do.
With that said, the WGPU renderer doesn't panic whereas the Tiny Skia renderer panics when it unwraps None. The message confused the submitter as mentioned in their original issue since it doesn't say anything about what caused the panic.
Would it make sense to merge this so neither renderer panics? Or should the Tiny Skia renderer still panic for the incorrect code?
Edit:
I think guarding the check so that it only applies in debug builds may be a better idea than what I submitted so far because:
- It's likely that the programmer(s) are working with debug builds when they make the mistake that would lead to the panic
- Tiny Skia panicking while WGPU doesn't is possibly confusing
- Having that check in release mode for every quad drawn is needlessly expensive, especially considering the code at hand is likely incorrect
hecrj
force-pushed
the
issue2066
branch
2 times, most recently
from
26b200c
to
1004863
Compare
hecrj
changed the title
iced_tiny_skia
hecrj
changed the title
iced_tiny_skiaiced_tiny_skia
Fixes #2066.
Iced may produce primitives with an infinite length or width. The WGPU renderer handles those somewhere along the pipeline. Tiny Skia's PathBuilder::finish returns
Noneif non-normal floats are passed to the builder. This patch adds a check to prevent that.I only fixed the immediate issue since it didn't seem like the other primitives needed the check. Plus, I didn't want to preemptively add checks where they're not needed. Let me know if I should make any changes.
See: