Introduce error handling in the FemtoVG and Skia renderers #2402
Conversation
tronical
force-pushed
the
simon/rendering-error-handling
branch
3 times, most recently
from
0075574
to
03382e7
Compare
Avoid unwrap() and expect() and instead propagate errors all the way down to run_event_loop(), show(), and hide() in the Slint AIP.
tronical
force-pushed
the
simon/rendering-error-handling
branch
from
03382e7
to
90cd83e
Compare
internal/renderers/femtovg/lib.rs
Outdated
| let width = size.width; | ||
| let height = size.height; | ||
|
|
||
| let canvas = if self.canvas.borrow().is_some() { | ||
| std::cell::Ref::map(self.canvas.borrow(), |canvas_opt| canvas_opt.as_ref().unwrap()) | ||
| } else { | ||
| return; | ||
| return Ok(()); |
There was a problem hiding this comment.
looks supicious. when would there be no canvas?
There was a problem hiding this comment.
For example if the user of the renderer API forgot to call show(). Do you think it should be an error?
There was a problem hiding this comment.
Usually, i'd say:
- misuse of API should be panic
- runtime error should be forwared as error.
in this case, we call render but don't render anything. The user might wonder what's the bug, don't you think?
There was a problem hiding this comment.
I don't feel too strong about it - I think it's like calling show() on a QWidget - it'll be empty (and so will be the underlying window). Do you have a preference between (1) panic (2) runtime error (3) silence?
There was a problem hiding this comment.
I don't know. it feel like it shouldn't be silence. Since we can report error, we might as well do.
Apply the same error message as for #2402 (comment)
Avoid unwrap() and expect() and instead propagate errors all the way down to run_event_loop(), show(), and hide() in the Slint AIP.