[ui] fix missing color conversion in drawAtlas.

[jonahwilliams]

Ensure that int32 ui colors are correctly converted to Dl Colors (convert instead of reinterpret cast).

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

[jonahwilliams]

@flar / @gaaclarke for ideas?

[jonahwilliams]

Maybe dart:ui golden is the best route

[gaaclarke]

It will get an integration test when I migrate it to support wide gamut colors. I'm just adding some to the gradient path right now.

[gaaclarke]

However, wide gamut support for this could get deprioritized depending on how everything else goes so it might be a while.

[gaaclarke]

Here's a pr for such a test for gradients: flutter/flutter#153976

If we wanted we could add the test that asserts it gets an srgb color. Then when wide gamut is supported, switch it to wide gamut.

[jonahwilliams]

I'm not adding wide gamut support as a feature - this code does not correctly convert the int32 color into the 4 float color so drawAtlas is broken right now. Test added, hopefully this passes.

[jonahwilliams]

Test added

[gaaclarke]

lgtm

[flar]

I tend to use:

  std::vector<Foo> things;
  things.reserve(N);
  for (all the things) {
    things.emplace_back(next thing constructor arguments);
    // or things.push_back(next thing) if it's a simple thing
    // or we've already constructed the things
  }

It avoids double initialization of the memory. Reserve just allocates and doesn't initialize. Since the loop will be replacing all of the default-initialized values, it's a waste of time.

[jonahwilliams]

Yeah that is a better way to do it. I think I'll let this sit for now as my expectation is that we can rip it out once the wide gamut conversions are finished.

[gaaclarke]

I think there is even a lint for this that we don't have turned on.