Summary: | [v8] Int16Array.set(array, offset) fails on first execution | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ulan Degenbaev <ulan> | ||||
Component: | WebGL | Assignee: | Ulan Degenbaev <ulan> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, japhet, kbr, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | 68890 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Ulan Degenbaev
2012-01-11 02:19:31 PST
Created attachment 121997 [details]
Pass the offset argument to copyElements
Ken, could you please take a look?
The problem was that I missed the offset argument when calling copyElements.
It passed the unit-tests because the fast set() method happened to be installed when calling set() with offset.
I added a separate test which calls the set with a valid offset at the beginning. There is not need for testing invalid offset because in such case copyElements is not called at all.
Comment on attachment 121997 [details] Pass the offset argument to copyElements View in context: https://bugs.webkit.org/attachment.cgi?id=121997&action=review Fix looks good. Is it possible to also incorporate tests for this into the array-unit-tests.html test? If not (because it only happens the first time set() is called for each view type) then the separate test case is fine. Marking cq- until this question is answered. > ChangeLog:6 > + Reviewed by Kenneth Russell. Leave the "Reviewed by NOBODY" line alone when uploading patches. The commit process patches it up automatically. > Is it possible to also incorporate tests for this into the array-unit-tests.html test? If not (because it only happens the first time set() is called for each view type) then the separate test case is fine. Yes, it happens only once on the first call, so I would prefer a separate test, otherwise this test could be accidentally disabled when changing array-unit-tests in future. > Leave the "Reviewed by NOBODY" line alone when uploading patches. The commit process patches it up automatically. I see, thanks. Comment on attachment 121997 [details]
Pass the offset argument to copyElements
OK. Code and tests look good. Setting cq+.
Comment on attachment 121997 [details] Pass the offset argument to copyElements Clearing flags on attachment: 121997 Committed r104736: <http://trac.webkit.org/changeset/104736> All reviewed patches have been landed. Closing bug. |