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

Remove need to check ready before calling getGeometryInstanceAttributes #2174

Open
pjcozzi opened this issue Oct 2, 2014 · 5 comments
Open
Labels

Comments

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 2, 2014

This leads to workarounds like:

var frameNumber = 0;
var scratch = new Cesium.Cartesian3();
var ready = false;

viewer.clock.onTick.addEventListener(function() {
    var length = updates.length;
    var k;
    var u;

    if (!ready && primitive.ready) {
        ready = true;
        for (k = 0; k < length; ++k) {
            u = updates[k];
            u.attributes = primitive.getGeometryInstanceAttributes(k);
        }

        return;
    }

    if (ready) {
        for (k = 0; k < length; ++k) {
            u = updates[k];
            scratch = Cesium.Cartesian3.multiplyByScalar(u.direction, (Math.sin(frameNumber * u.speed) * u.maxHeight), scratch);
            u.attributes.offset = Cartesian3InstanceAttribute.toValue(scratch, u.offsetArray);
        }
    }

    ++frameNumber;
});

Full example: https://gist.github.com/pjcozzi/4bc8fbf5a2907de6c3a9

We can just buffer the attributes, which will also eliminate the need for a scratch variable each time an attribute is set (u.offsetArray above), which caught me recently. I suspect this can be done without creating GC pressure so the performance drop should be insignificant compared to the usability gain.

CC #766

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Nov 3, 2016

@bagnell any chance this was part of the batch table changes?

@bagnell
Copy link
Contributor

bagnell commented Nov 3, 2016

The batch table fixes this mostly. The only thing that needs to wait for the primitive to be ready is the bounding sphere. If it's OK to return undefined for the bounding sphere until it's ready, this would be a simple change.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Nov 3, 2016

Yeah, I think it will be fine for the bounding sphere property to require the primitive being ready. Labeled this as beginner.

@pjcozzi pjcozzi added the good first issue An opportunity for first time contributors label Nov 3, 2016
@bampakoa
Copy link
Contributor

bampakoa commented Mar 31, 2017

@pjcozzi will the bounding sphere wait synchronously or asynchronously for the primitive to be ready?

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Apr 3, 2017

The bounding sphere get property function could just throw a DeveloperError if the primitive is not ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants