-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Support instanced models in 2D #4996
Conversation
Part of #4678 |
CC @pmconne |
Sounds OK to me. |
@bagnell can you please review and merge when happy? |
I thought about my comments some more and I think handling that case can pass for now. Even with the fixes for 2D there would still be issues with 3D since it renders instances relative to center. To fix that completely (using |
OK, please make sure the limitation is in the reference doc. |
var scratchMatrix = new Matrix4(); | ||
var scratchRotation = new Matrix3(); | ||
|
||
function getRtcTransform2D(frameState, rtcTransform, result) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does the same thing as result = Transforms.basisTo2D(frameState.mapProjection, rtcTransform, result);
. Why don't you use that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish I realized this ahead of time... anyways is the version here functionally equal to the Transforms.basisTo2D
, could that be simplified with the code here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that would be great. That function does the swizzling manually where this does it with a matrix multiply. It also ensures that the rotation matrix is orthogonal, but that shouldn't be necessary.
This looks good. Just that one comment. Something unrelated to this PR: why is the camera minimum zoom set for the example? |
I must have just modeled it after the |
Updated. |
For #3241
I tested out a variety of cases like regular i3dm tiles, i3dm with tile transform / changing the transform dynamically,
3D Models Instancing
sandcastle with instancing on/off.The approach here transforms the instances to 2D with a transform derived from the collection's center.
I think this approach will be fine for most uses cases, like most i3dm tiles, but will definitely break down if instances are located all across the globe. For that the
getRtcTransform2D
function would have to be on an instance-by-instance basis and the vertex buffer would need to be updated when switching to 2D. Since it is a one time operation it may be worth just doing that.