[vector_math] Write out parameters once through setValues - #38
Open
spydon wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates Vector2, Vector3, and Vector4 (both 32-bit and 64-bit implementations) so that normalizeInto, copyInto, min, max, and mix write to their output parameters exactly once using setValues. It also adds tests to verify this behavior for subclass observation. The review feedback highlights multiple instances where newly introduced lines exceed the 80-character limit specified in the style guide, suggesting wrapping arguments to improve readability.
normalizeInto wrote its target with setFrom followed by a normalize pass, min, max and mix assigned each component through a separate setter, and copyInto wrote the target's storage field by field. The Vector2, Vector3 and Vector4 members that take an out or result parameter now compute their result first and write it with a single setValues call, which measures 6 to 17 percent faster in AOT for normalizeInto, min, max and mix while producing identical results. Claude-Session: https://claude.ai/code/session_01PixXaRTPs8v1P2mQF2QHGG
spydon
force-pushed
the
fix/vector-write-out-parameters-once
branch
from
August 30, 2026 19:46
69d5fdb to
f0f0e94
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes the
Vector2,Vector3andVector4members that take anout/resultparameter compute their result first and write it with a singlesetValuescall:normalizeInto(out)didout..setFrom(this)..normalize(): one pass to copy the components and a second pass to read them back, compute the length and scale them. It now computes the normalized components fromthisand writes them once; the zero vector is still copied unchanged.min,maxandmixassigned each component through a separate setter call (result..x = …..y = …). They now make onesetValuescall.copyInto(arg)wrote the target's storage field by field; it is aligned with the same singlesetValuespattern.Numeric results are identical (verified by the benchmark checksums and the test suite).
vector_math_64was regenerated withtool/generate_vector_math_64.dart; only the vector files are included, because the generator also produces unrelated diffs inintersection_result.dartandquaternion.dart(those 64-bit files were hand-edited after generation), so they are left untouched here.A new test,
test/out_parameter_test.dart, covers the rewritten members for all three classes: results fornormalizeInto(including the zero-length vector andv.normalizeInto(v)aliasing),copyInto,min,maxandmix.Benchmark
Standalone benchmark (not committed) compiled with
dart compile exeagainstmainand against this branch, on macOS arm64; 4096 vectors per case, each binary run five times alternately, values are min/median/max of the runs in nanoseconds per call.mainVector2normalizeIntoVector2copyIntoVector2minVector2maxVector2mixVector3normalizeIntoVector3copyIntoVector3minVector3maxVector3mixVector4normalizeIntoVector4copyIntoVector4minVector4maxVector4mixThe gains are AOT-specific. The same benchmark compiled with
dart compile js -O4and run in node measures neutral: every case is within about 2% with overlapping ranges. Both builds produce identical checksums on both platforms.Fixes flutter/flutter#192041
Pre-Review Checklist
[vector_math]///).https://claude.ai/code/session_01PixXaRTPs8v1P2mQF2QHGG
Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2