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

[web] move some vector functions into vector_math.dart #32782

Merged
merged 2 commits into from
Apr 19, 2022
Merged

[web] move some vector functions into vector_math.dart #32782

merged 2 commits into from
Apr 19, 2022

Conversation

@yjbanov
Copy link
Contributor

@yjbanov yjbanov commented Apr 19, 2022

Move toMatrix32 and the (not so) FastMatrix32 vector math utilities into the vector_math.dart library, where they belong.

@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Apr 19, 2022
@flutter-dashboard
Copy link

flutter-dashboard bot commented Apr 19, 2022

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

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.

ditman
ditman approved these changes Apr 19, 2022
View reviewed changes
Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

I would exempt this of tests, it is just moving already existing code around. These are utility functions that are tested indirectly through other code that handles vector transformations elsewhere.

lib/web_ui/lib/src/engine/html/scene_builder.dart

import 'package:ui/ui.dart' as ui;

import '../../engine.dart' show kProfileApplyFrame, kProfilePrerollFrame, toMatrix32;
Copy link
Member

@ditman ditman Apr 19, 2022

Choose a reason for hiding this comment

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

Didn't we have a lint for unused imports?

Copy link
Contributor Author

@yjbanov yjbanov Apr 19, 2022

Choose a reason for hiding this comment

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

We do, but I think the lint doesn't trigger when two libraries vend the same member.

@yjbanov
Copy link
Contributor Author

yjbanov commented Apr 19, 2022

Actually, it may be worth testing. These utilities are only covered indirectly. I'll add tests.

@yjbanov yjbanov force-pushed the move-vector-math-funcs branch from e8d2732 to 22f3c63 Compare April 19, 2022 22:43
@ditman ditman added waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. and removed needs tests labels Apr 19, 2022
@ditman
Copy link
Member

ditman commented Apr 19, 2022

Tests were added.

@fluttergithubbot fluttergithubbot merged commit 299b25c into flutter:main Apr 19, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 20, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 20, 2022
@engine-flutter-autoroll
299b25c [web] move some vector functions into vector_math.dart (flutt…
6ece255 
…er/engine#32782)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 20, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 20, 2022
@engine-flutter-autoroll
299b25c [web] move some vector functions into vector_math.dart (flutt…
5565999 
…er/engine#32782)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 20, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 20, 2022
@engine-flutter-autoroll
299b25c [web] move some vector functions into vector_math.dart (flutt…
696e239 
…er/engine#32782)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 20, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 20, 2022
@engine-flutter-autoroll
299b25c [web] move some vector functions into vector_math.dart (flutt…
095e293 
…er/engine#32782)
cbracken pushed a commit to flutter/flutter that referenced this pull request Apr 20, 2022
@engine-flutter-autoroll
Roll Engine from 2ce50ac to 47af169 (5 revisions) (#102192)
1d84805 
* 299b25c [web] move some vector functions into vector_math.dart (flutter/engine#32782)

* 81a9213 Roll Dart SDK from 889e4039c8d6 to 1db3abc0ac54 (1 revision) (flutter/engine#32792)

* 99222ba Roll Fuchsia Linux SDK from sncTyrbDY... to wkRK4A6QT... (flutter/engine#32793)

* 584393a Roll Skia from 932801c29efb to 2cf8e9b602ec (2 revisions) (flutter/engine#32794)

* 47af169 Roll ANGLE to acdec48addfdff315efff293be0d5e2e37e8e560 (flutter/engine#32785)
justinmc pushed a commit to justinmc/engine that referenced this pull request Apr 20, 2022
@yjbanov @justinmc
[web] move some vector functions into vector_math.dart (flutter#32782)
25b5205
egramond pushed a commit to egramond/flutter that referenced this pull request May 5, 2022
@engine-flutter-autoroll @egramond
Roll Engine from 2ce50ac to 47af169 (5 revisions) (flutter#102192)
298ead5 
* 299b25c [web] move some vector functions into vector_math.dart (flutter/engine#32782)

* 81a9213 Roll Dart SDK from 889e4039c8d6 to 1db3abc0ac54 (1 revision) (flutter/engine#32792)

* 99222ba Roll Fuchsia Linux SDK from sncTyrbDY... to wkRK4A6QT... (flutter/engine#32793)

* 584393a Roll Skia from 932801c29efb to 2cf8e9b602ec (2 revisions) (flutter/engine#32794)

* 47af169 Roll ANGLE to acdec48addfdff315efff293be0d5e2e37e8e560 (flutter/engine#32785)
@mdebbar mdebbar mentioned this pull request Jul 18, 2023
Closed
11 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@ditman ditman ditman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

platform-web Code specifically for the web engine waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yjbanov @ditman @fluttergithubbot