Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move CkCanvas to new JS-interop #19748

Merged
merged 6 commits into from Jul 16, 2020
Merged

Move CkCanvas to new JS-interop #19748

merged 6 commits into from Jul 16, 2020

Conversation

yjbanov
Copy link
Contributor

@yjbanov yjbanov commented Jul 14, 2020

Description

Move CkCanvas to the new JS-interop.

Benchmark results:

Score	Average A (noise)	Average B (noise)	Speed-up
bench_card_infinite_scroll.canvaskit.drawFrameDuration.average	2982.46 (9.98%)	2409.46 (5.28%)	1.24x	
bench_card_infinite_scroll.canvaskit.totalUiFrame.average	3821.00 (4.62%)	3052.67 (11.97%)	1.25x	
bench_card_infinite_scroll_backward.canvaskit.drawFrameDuration.average	2742.83 (6.26%)	1837.10 (12.32%)	1.49x	
bench_card_infinite_scroll_backward.canvaskit.totalUiFrame.average	3911.00 (4.59%)	2687.67 (8.14%)	1.46x	
clipped_out_pictures.canvaskit.drawFrameDuration.average	3540.29 (5.01%)	2055.62 (2.03%)	1.72x	
clipped_out_pictures.canvaskit.totalUiFrame.average	4538.67 (4.90%)	2665.67 (13.63%)	1.70x	
draw_rect.canvaskit.drawFrameDuration.average	5632.81 (7.03%)	3929.68 (2.81%)	1.43x	
draw_rect.canvaskit.totalUiFrame.average	6629.33 (14.16%)	4689.00 (7.90%)	1.41x	
draw_rect_variable_paint.canvaskit.drawFrameDuration.average	13498.28 (3.56%)	9841.54 (7.90%)	1.37x	
draw_rect_variable_paint.canvaskit.totalUiFrame.average	14530.33 (1.30%)	11068.33 (13.78%)	1.31x	
bench_path_recording.canvaskit.recordPathConstruction.average	20799.10 (0.52%)	20904.79 (1.45%)	0.99x	
text_out_of_picture_bounds.canvaskit.drawFrameDuration.average	4566.99 (8.28%)	4624.09 (2.23%)	0.99x	
text_out_of_picture_bounds.canvaskit.totalUiFrame.average	5399.00 (9.07%)	5133.67 (1.06%)	1.05x	
bench_simple_lazy_text_scroll.canvaskit.drawFrameDuration.average	4944.68 (1.94%)	3680.19 (2.89%)	1.34x	
bench_simple_lazy_text_scroll.canvaskit.totalUiFrame.average	5704.00 (5.44%)	4146.00 (2.30%)	1.38x	
build_material_checkbox.canvaskit.drawFrameDuration.average	11026.33 (0.93%)	8901.66 (1.78%)	1.24x	
build_material_checkbox.canvaskit.totalUiFrame.average	11309.33 (1.34%)	9215.67 (2.11%)	1.23x	
dynamic_clip_on_static_picture.canvaskit.drawFrameDuration.average	1590.39 (5.86%)	1495.84 (0.57%)	1.06x	
dynamic_clip_on_static_picture.canvaskit.totalUiFrame.average	1863.33 (3.38%)	1837.67 (2.81%)	1.01x	
bench_picture_recording.canvaskit.recordPaintCommands.average	17117.50 (6.36%)	8514.61 (1.73%)	2.01x	
bench_picture_recording.canvaskit.estimatePaintBounds.average	65.21 (1.43%)	86.61 (1.44%)	0.75x	
bench_update_many_child_layers.canvaskit.drawFrameDuration.average	8070.84 (8.64%)	5682.77 (4.13%)	1.42x	
bench_update_many_child_layers.canvaskit.totalUiFrame.average	9179.33 (7.94%)	7313.67 (5.78%)	1.26x	
bench_mouse_region_grid_scroll.canvaskit.drawFrameDuration.average	6388.93 (9.30%)	6510.27 (1.23%)	0.98x	
bench_mouse_region_grid_scroll.canvaskit.totalUiFrame.average	8182.00 (4.29%)	7826.00 (4.79%)	1.05x	
bench_mouse_region_grid_hover.canvaskit.drawFrameDuration.average	5834.26 (5.87%)	5646.79 (1.71%)	1.03x	
bench_mouse_region_grid_hover.canvaskit.hitTestDuration.average	260.55 (4.96%)	279.12 (4.60%)	0.93x	
bench_mouse_region_grid_hover.canvaskit.totalUiFrame.average	7519.00 (4.06%)	7773.33 (12.05%)	0.97x	
text_canvas_kit_color_grid.canvaskit.drawFrameDuration.average	22538.38 (0.08%)	20249.01 (1.73%)	1.11x	

Results from the devicelab:

Screen Shot 2020-07-17 at 12 31 19 PM

Tests

I added the following tests:

  • More tests in canvaskit_api_test.dart.

Copy link
Contributor

@harryterkelsen harryterkelsen left a comment

Choose a reason for hiding this comment

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

LGTM

/// WebAssembly heap.
///
/// These objects are automatically deleted when no longer used.
abstract class SkiaObject {
abstract class SkiaObject<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I really like the solution of parameterizing SkiaObject

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants