[Impeller] make sure binding nullptr texture does not crash. #56381
Conversation
| if (!texture) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
nit: I would prefer this is an assert instead of turning BindResource to a noop. It's harder to follow code if every procedure call is prefaced with an implicit "maybe", like "MaybeBindResource()". New people on the team complain about this pattern making the code hard to follow.
This at least communicates when nothing has happened with the returnval and matches existing patterns though so we shouldn't hold back the PR about it. How about adding [[nodiscard]] to this method to communicate at callsites that the method potentially is a noop?
There was a problem hiding this comment.
We don't want to crash the app if one of the draws is misconfigured. There is some ambiguity (due to runtime effect) as to whether this is covering up a bug in flutter or a bug in the user code.
DCHECK would be fine, except that it doesn't stop the production crash.
What I'd really like is to get those nullability annotations working correctly so we can correctly eliminate the nullptr closer to where it was generated.
jonahwilliams
added
the
autosubmit
…158303) flutter/engine@94dac95...076688d 2024-11-07 [email protected] [web:a11y] make header a <header> when non-empty and heading when empty (flutter/engine#55996) 2024-11-06 [email protected] [Impeller] make sure binding nullptr texture does not crash. (flutter/engine#56381) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…/engine#56381) Fixes flutter#158074 binding a nullptr texture should fail but not crash.
2 participants
Fixes flutter/flutter#158074
binding a nullptr texture should fail but not crash.