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

[ui] fix missing color conversion in drawAtlas. #54750

Merged
merged 5 commits into from
Aug 26, 2024
Merged

[ui] fix missing color conversion in drawAtlas. #54750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9b76c5b
[ui] fix missing color conversion in drawAtlas.
Aug 23, 2024
421fcb7
add test.
Aug 24, 2024
557dcdd
come on.
Aug 24, 2024
c25c72e
Update canvas_test.dart
Aug 24, 2024
1f8406f
fix test.
Aug 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions lib/ui/painting/canvas.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,12 +596,17 @@ Dart_Handle Canvas::drawAtlas(Dart_Handle paint_objects,
tonic::Int32List colors(colors_handle);
tonic::Float32List cull_rect(cull_rect_handle);

std::vector<DlColor> dl_color(colors.num_elements());
Copy link
Contributor

@flar flar Aug 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

size_t count = colors.num_elements();
for (size_t i = 0; i < count; i++) {
dl_color[i] = DlColor(colors[i]);
}

DlPaint dl_paint;
const DlPaint* opt_paint = paint.paint(dl_paint, kDrawAtlasWithPaintFlags);
builder()->DrawAtlas(
dl_image, reinterpret_cast<const SkRSXform*>(transforms.data()),
reinterpret_cast<const SkRect*>(rects.data()),
reinterpret_cast<const DlColor*>(colors.data()),
reinterpret_cast<const SkRect*>(rects.data()), dl_color.data(),
rects.num_elements() / 4, // SkRect have four floats.
blend_mode, sampling, reinterpret_cast<const SkRect*>(cull_rect.data()),
opt_paint);
Expand Down
46 changes: 46 additions & 0 deletions testing/dart/canvas_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1267,6 +1267,52 @@ void main() async {
expect(paintCopy.colorFilter, equals(const ColorFilter.mode(Color(0xFF00FF00), BlendMode.color)));
expect(paintCopy.imageFilter, equals(ImageFilter.blur(sigmaX: 10.0, sigmaY: 10.0)));
});

test('DrawAtlas correctly copies color values into display list format', () async {
final Image testImage = await createTestImage();
final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
// Make a drawAtlas call that should be solid red.
canvas.drawAtlas(
testImage,
[
RSTransform.fromComponents(
rotation: 0,
scale: 10,
anchorX: 0,
anchorY: 0,
translateX: 0,
translateY: 0,
),
],
[
const Rect.fromLTWH(0, 0, 1, 1)
],
[
const Color.fromARGB(255, 255, 0, 0)
],
BlendMode.dst,
null,
Paint(),
);

final Image resultImage = await recorder.endRecording().toImage(1, 1);
final ByteData? data = await resultImage.toByteData();
if (data == null) {
fail('Expected non-null byte data');
return;
}
final int rgba = data.buffer.asUint32List()[0];
expect(rgba, 0xFF0000FF);
});
}

Future<Image> createTestImage() async {
final PictureRecorder recorder = PictureRecorder();
final Canvas recorderCanvas = Canvas(recorder);
recorderCanvas.scale(1.0, 1.0);
final Picture picture = recorder.endRecording();
return picture.toImage(1, 1);
}

Matcher listEquals(ByteData expected) => (dynamic v) {
Expand Down