Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Test babylon vertex buffer instance divisor #12996

Merged
merged 4 commits into from
Sep 19, 2022
Merged

Test babylon vertex buffer instance divisor #12996

merged 4 commits into from
Sep 19, 2022

Conversation

Dok11
Copy link
Contributor

@Dok11 Dok11 commented Sep 19, 2022

Hi! There are some tests again :)

Some points here:

  1. Simplify the code.
  2. Setter was recompute the hash after every set call, but it shuld be only after change the this._instanced as it we can see in the _computeHashCode method.
  3. I found shadowed variable (data) in the constructor and fix it (to vertexData in the local scope).

And just in case I explain my pipeline:

  1. First I wrote test cases.
  2. Only after it I started to modify the code.

@Dok11
fix(VertexBuffer): simplify the assign statement in the instantDivisor
9467192
@Dok11
fix(VertexBuffer): compute the hash code only if isInstanced changed
4778fb2 
early the instanceDivisor setter recomputed the hash every time,
but it was necessary only when `this._instanced` changes
@Dok11
fix(VertexBuffer): fix shadowed variable in the class constructor
084a3ee 
the data variable was defined as constructor's argument and reassigned,
but it was not necessary
@Dok11
test(VertexBuffer): add test cases for instanceDivisor setter and getter
e0f3185
@azure-pipelines
Copy link

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@azure-pipelines
Copy link

Snapshot stored with reference name:
refs/pull/12996/merge

Test environment:
https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12996/merge/index.html

To test a playground add it to the URL, for example:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12996/merge/index.html#WGZLGJ#4600

Links to test babylon tools with this snapshot:

https://playground.babylonjs.com/?snapshot=refs/pull/12996/merge
https://sandbox.babylonjs.com/?snapshot=refs/pull/12996/merge
https://gui.babylonjs.com/?snapshot=refs/pull/12996/merge
https://nme.babylonjs.com/?snapshot=refs/pull/12996/merge

To test the snapshot in the playground with a playground ID add it after the snapshot query string:

https://playground.babylonjs.com/?snapshot=refs/pull/12996/merge#BCU1XR#0

@sebavan sebavan merged commit 7d397a3 into BabylonJS:master Sep 19, 2022
RaananW pushed a commit that referenced this pull request Dec 9, 2022
@sebavan
Merge pull request #12996 from Dok11/test-babylon-vertex-buffer-insta…
402c795 
…nce-divisor

Test babylon vertex buffer instance divisor

Former-commit-id: 01bc5fe36eec61925ca04ece8ad6f288edfa869a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebavan sebavan sebavan approved these changes

Assignees
No one assigned
Labels
skip changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Dok11 @sebavan