Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(copy): fix typed subarrays handling #14845

Closed
wants to merge 2 commits into from
Closed

fix(copy): fix typed subarrays handling #14845

wants to merge 2 commits into from

Conversation

zhukov
Copy link
Contributor

@zhukov zhukov commented Jun 29, 2016

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Bug fix

What is the current behavior? (You can also link to an open issue here)

It copies the whole buffer, not a slice.
Here is the demo: http://plnkr.co/edit/nU76OITTchFc48jFab2a?p=preview
#14842

What is the new behavior (if this is a feature change)?

When copying typed subarray the new value has the same length as the initial typed array.
This is not a feature change.

Does this PR introduce a breaking change?

No

Please check if the PR fulfills these requirements

Other information:

Previously, it would return copy of the whole original typed array, not its slice.
Now buffer offset and length are also saved.

Closes #14842

Previously, it would return copy of the whole original typed array, not its slice.
Now buffer offset and length are also saved.

Closes #14842
@@ -901,7 +901,7 @@ function copy(source, destination) {
case '[object Uint8ClampedArray]':
case '[object Uint16Array]':
case '[object Uint32Array]':
return new source.constructor(copyElement(source.buffer));
return new source.constructor(copyElement(source.buffer), source.byteOffset, source.length);
Copy link
Member

Choose a reason for hiding this comment

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

Is length the same as byteLength (for non-uint8 arrays)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gkalpak nope, source.byteLength = source.length * source.BYTES_PER_ELEMENT.
I've made some sample code to demo why source.length and not source.byteLength:
https://plnkr.co/edit/afWbk8OboJopB6BgSri7

@gkalpak
Copy link
Member

gkalpak commented Jun 30, 2016

Other than one minor comment, LGTM 👍

@zhukov
Copy link
Contributor Author

zhukov commented Jul 4, 2016

Hm.. no idea, what should I fix to make it pass. Works fine on my branch.

@gkalpak
Copy link
Member

gkalpak commented Jul 4, 2016

@zhukov , it is probably a flake. I restarted the job. Thx for the changes 👍

@gkalpak gkalpak closed this in 6280ec8 Jul 4, 2016
gkalpak pushed a commit that referenced this pull request Jul 4, 2016
Previously, it would return a copy of the whole original typed array, not its slice.
Now, the `byteOffset` and `length` are also preserved.

Fixes #14842

Closes #14845
@zhukov zhukov deleted the fix-copy-typed-subarray branch July 5, 2016 10:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

angular.copy doesn't copy typed subarrays
3 participants