-
Notifications
You must be signed in to change notification settings - Fork 6k
Fix npe during skia dispatch of drawAtlas #55497
Conversation
| sk_colors.push_back(colors[i].argb()); | ||
| } | ||
| } | ||
| canvas_->drawAtlas(skia_atlas.get(), xform, ToSkRects(tex), sk_colors.data(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately .data() is not guaranteed to return nullptr for empty std::vector. We need to explicitly send it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
damn you STL
gaaclarke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, just a suggestion and a question
| canvas_->drawAtlas(skia_atlas.get(), xform, ToSkRects(tex), sk_colors.data(), | ||
| count, ToSk(mode), ToSk(sampling), ToSkRect(cullRect), | ||
| canvas_->drawAtlas(skia_atlas.get(), xform, ToSkRects(tex), | ||
| colors == nullptr ? nullptr : sk_colors.data(), count, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| colors == nullptr ? nullptr : sk_colors.data(), count, | |
| sk_colors.empty() ? nullptr : sk_colors.data(), count, |
This is a more direct condition =)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
testing/dart/canvas_test.dart
Outdated
|
|
||
| final Image resultImage = await recorder.endRecording().toImage(1, 1); | ||
| final ByteData? data = await resultImage.toByteData(); | ||
| if (data == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not use an expect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I guess this is from old litetest days.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
auto label is removed for flutter/engine/55497, due to - The status or check suite Mac mac_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
You were fast to correct the ticket, thanks!! |
|
auto label is removed for flutter/engine/55497, due to - The status or check suite Mac mac_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
It's on master as soon as this PR rolls into the framework in a few hours. And there is no dev channel 😁 |
|
Used to be long time ago ;) |
…155889) flutter/engine@ff45417...c4784aa 2024-09-28 [email protected] Fix npe during skia dispatch of drawAtlas (flutter/engine#55497) 2024-09-28 [email protected] Roll Skia from 7efc11f2ea9e to f88a9ae4d9c1 (2 revisions) (flutter/engine#55511) 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
…lutter#155889) flutter/engine@ff45417...c4784aa 2024-09-28 [email protected] Fix npe during skia dispatch of drawAtlas (flutter/engine#55497) 2024-09-28 [email protected] Roll Skia from 7efc11f2ea9e to f88a9ae4d9c1 (2 revisions) (flutter/engine#55511) 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
Colors are optionally provided, so make sure to null check the array.
Fixes flutter/flutter#155783