Initialialize AABB Rectangle #467
Merged
Conversation
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
mzgoddard
force-pushed
the
rect-init-matrix
branch
from
6fddfc0 to
447923a
Compare
- pass bounds as a destination parameter - add initFromMatrixRadius - use initFromMatrixRadius in getAABB
mzgoddard
force-pushed
the
rect-init-matrix
branch
from
447923a to
3b2cd5f
Compare
gnarf
reviewed
Jun 11, 2019
mzgoddard
force-pushed
the
rect-init-matrix
branch
from
3b2cd5f to
116e4a6
Compare
mzgoddard
force-pushed
the
rect-init-matrix
branch
from
116e4a6 to
5886622
Compare
mzgoddard
force-pushed
the
rect-init-matrix
branch
from
5886622 to
5d7957f
Compare
gnarf
reviewed
Jun 11, 2019
|
How does this compare to what I did in #445? |
|
@adroitwhiz I think #467 and #445 go together. We won't need to cache vectors for getAABB after #467, but we'll still need to cache vectors for getTransformedHullPoints. Probably still use #467 |
|
I've moved the math proof documentation into a jsdoc tutorial at |
mzgoddard
force-pushed
the
rect-init-matrix
branch
from
ba99c58 to
994e9be
Compare
cwillisf
approved these changes
Jun 21, 2019
This was referenced Aug 13, 2019
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.
Resolves
Render related VM performance, such as fencing a sprite inside the scratch canvas.
Proposed Changes
Reason for Changes
Current getAABB creates 4 3D vectors, transforms them by a 4x4 matrix and compares their axes to find the left, right, top, bottom extremes for the AABB. We can break that down into something like 2 function calls, 21 object member reads, 6 writes, 15 mulitplications, 13 additions, 2 created objects, and 5 comparisons per vectors. That is 64 operations per vector. Or 256 operations to construct the AABB.
This version by comparisons uses 6 object members reads, 6 multiplications, 6 additions, and 4 absolute value operations. In total that is 22 operations to construct the AABB.
The main cause in the difference in operations is that the current version transforms 4 vectors, and this version transform the 1 vector into the absolute value of its transformed components. Then the work is further reduced by contraining the transformation to 2 dimensions.
Passing a Rectangle to getBounds functions lets us reuse Rectangles to avoid creating lot of garbage that needs to be cleaned up. By chance we can't reuse a rectangle the functions can create their own to return.
Test Coverage
The existing Drawable units that test getAABB.
Benchmark Data
Chrome inspector
This table was collected from the Chrome Inspector's Javascript Profiler tab running the 14844969 benchmark. The values in this table are the percent of time spent under getFastBounds, which is used by SVGSkin to get the getFenceBounds to calculate the fence position when moving a sprite with motion_changex and other blocks. The values do not add up to 100 as many of the functions are nested in another.
Some columns do not report any time spent. getAABB and copy are still being called, but time spent there was not captured by the profiler.
scratch-vm benchmark
The following values were collected the scratchfoundation/scratch-vm#2196 change on scratch-vm.
Highlights
These highlighted benchmarks heavily use motion blocks that depend on the changed code.
edit: The before times in this table by accident do not reflect develop. This means the comparison has increased noise. I am leaving the table here for recording repurposes. This does not affect the chrome inspector table.
Full Benchmark Suite
This is a table of the benchmarks with one run each for project id, vm state, device, before and after. As such the noise to signal ratio is high here, so there is a high margin of error in the amount performance changed. Most of the identifiable correlation is in the above highlights table.
edit: The before times in this table by accident do not reflect develop. This means the comparison has increased noise. I am leaving the table here for recording repurposes. This does not affect the chrome inspector table.