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.

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

Add mat4 instanceModelMatrix to ScenegraphLayer #2767

Merged
merged 1 commit into from Mar 11, 2019

Conversation

Projects
None yet
3 participants
@georgios-uber
Copy link
Contributor

georgios-uber commented Mar 8, 2019

For #

Background

Change List

@georgios-uber georgios-uber self-assigned this Mar 8, 2019

@georgios-uber georgios-uber requested review from Pessimistress and ibgreen Mar 8, 2019

@ibgreen

ibgreen approved these changes Mar 8, 2019

@ibgreen ibgreen requested a review from tsherif Mar 8, 2019

@ibgreen

ibgreen approved these changes Mar 8, 2019

@@ -28,6 +29,7 @@ const vs = `
attribute vec3 instancePositions;
attribute vec2 instancePositions64xy;
attribute vec3 instancePickingColors;
attribute mat4 instanceModelMatrix;

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 8, 2019

Contributor

@tsherif What do you think about using a mat4 declaration for the attribute in the shader?

It looks a little cleaner this way, are there any caveats?

This comment has been minimized.

Copy link
@georgios-uber

georgios-uber Mar 11, 2019

Author Contributor

I hope it is also faster this way because the compiler knows to keep all 16 values together so it can potentially optimize.

This comment has been minimized.

Copy link
@tsherif

tsherif Mar 11, 2019

Member

No caveats, as long as the attribute pointers are set up correctly. No performance wins either though, since layout is exactly the same.

@georgios-uber georgios-uber force-pushed the mat4scenegraph branch from 72a0b67 to 1e34579 Mar 11, 2019

@georgios-uber georgios-uber changed the title Add mat4 to scenegraph-layer Add mat4 instanceModelMatrix to scenegraph-layer Mar 11, 2019

@georgios-uber georgios-uber changed the title Add mat4 instanceModelMatrix to scenegraph-layer Add mat4 instanceModelMatrix to ScenegraphLayer Mar 11, 2019

@georgios-uber georgios-uber merged commit a671898 into master Mar 11, 2019

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@georgios-uber georgios-uber deleted the mat4scenegraph branch Mar 11, 2019

ajduberstein added a commit to ajduberstein/deck.gl that referenced this pull request Apr 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.