Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@jonahwilliams
Copy link
Contributor

Colors are optionally provided, so make sure to null check the array.

Fixes flutter/flutter#155783

sk_colors.push_back(colors[i].argb());
}
}
canvas_->drawAtlas(skia_atlas.get(), xform, ToSkRects(tex), sk_colors.data(),
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

damn you STL

Copy link
Member

@gaaclarke gaaclarke left a 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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
colors == nullptr ? nullptr : sk_colors.data(), count,
sk_colors.empty() ? nullptr : sk_colors.data(), count,

This is a more direct condition =)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


final Image resultImage = await recorder.endRecording().toImage(1, 1);
final ByteData? data = await resultImage.toByteData();
if (data == null) {
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 27, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 27, 2024
@auto-submit
Copy link
Contributor

auto-submit bot commented Sep 27, 2024

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.

@Solido
Copy link

Solido commented Sep 28, 2024

You were fast to correct the ticket, thanks!!
When can we expect the correction into dev or master please.
Not blocking it's just that it difficult to estimate real app perfs without drawAtlas.
Best regards.

@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 28, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 28, 2024
@auto-submit
Copy link
Contributor

auto-submit bot commented Sep 28, 2024

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.

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 28, 2024
@auto-submit auto-submit bot merged commit c4784aa into flutter:main Sep 28, 2024
@jonahwilliams
Copy link
Contributor Author

It's on master as soon as this PR rolls into the framework in a few hours. And there is no dev channel 😁

@Solido
Copy link

Solido commented Sep 28, 2024

Used to be long time ago ;)
Thanks for the correction.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 28, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Sep 28, 2024
thejitenpatel pushed a commit to thejitenpatel/flutter that referenced this pull request Oct 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[MacOS] App crashes when using canvas.drawAtlas() on beta & master

3 participants