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

Tile coloring #5358

Merged
merged 5 commits into from
May 25, 2017
Merged

Tile coloring #5358

merged 5 commits into from
May 25, 2017

Conversation

lilleyse
Copy link
Contributor

This lets the 3D Tiles inspector do hover-over coloring for tiles that don't have features, like a lot of photogrammetery tilesets and point clouds.

@pjcozzi
Copy link
Contributor

pjcozzi commented May 24, 2017

@hpinkos do you want to look at this and merge?

@hpinkos
Copy link
Contributor

hpinkos commented May 24, 2017

Picking isn't working correctly for the composite example in the 3dtiles sandcastle demo. The highlight color doesn't unselect. It might not happen the first mouseout, but if you keep hovering over different features it gets into a weird state fairly quickly

@hpinkos
Copy link
Contributor

hpinkos commented May 24, 2017

Everything else works though, thanks @lilleyse!

@lilleyse
Copy link
Contributor Author

Thanks for noticing that, should be fixed now.

I also had a small fix to jsep that I didn't catch before. We took that code out of the prior version too.

@hpinkos
Copy link
Contributor

hpinkos commented May 24, 2017

@lilleyse that's a prime example for why we shouldn't ever edit a third party library like that. It makes updating to a new version a nightmare because you lose track of custom changes you made. Why do we need to remove that section of code from jsep? Is there a bug?

@pjcozzi
Copy link
Contributor

pjcozzi commented May 24, 2017

Please contribute back to jsep if it is a change they would be interested in.

@lilleyse
Copy link
Contributor Author

I'm not sure why the changes were originally made, but I'm guessing it has to do with working with AMD.

@hpinkos
Copy link
Contributor

hpinkos commented May 25, 2017

If it's a problem with AMD, we should try to figure out what the issue is instead of hacking things out of the source file. Ask @mramato if you need help figuring it out.

@lilleyse lilleyse mentioned this pull request May 25, 2017
@lilleyse
Copy link
Contributor Author

Took the jsep changes out of here and moved to #5365.

@hpinkos
Copy link
Contributor

hpinkos commented May 25, 2017

Thanks @lilleyse!

@hpinkos hpinkos merged commit b1d2568 into 3d-tiles May 25, 2017
@hpinkos hpinkos deleted the tile-coloring branch May 25, 2017 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants