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

Fix full uri #409

Merged
merged 1 commit into from
Sep 23, 2019
Merged

Fix full uri #409

merged 1 commit into from
Sep 23, 2019

Conversation

xintongxia
Copy link

@xintongxia xintongxia commented Sep 17, 2019

baseUri is missing, so the tiles' fullUri ended up something like

undefined/0/0.b3dm.

@coveralls
Copy link

coveralls commented Sep 17, 2019

Coverage Status

Coverage increased (+4.09%) to 59.498% when pulling c9c56df on xx/fux into a83a104 on master.

Copy link
Collaborator

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could have sworn that @loshjawrence already fixed this. I wonder if it is the same place?

Can someone look up the previous PR?

@xintongxia can you provide some more information about why this was needed?

@loshjawrence
Copy link
Collaborator

loshjawrence commented Sep 17, 2019

Sorry just saw this. The previous PR didn't have this change. I think without this change you could have multiple nodes in the tree who have a key of / or a key of _basePath because they are empty tiles, but I'm not sure. If that's the case, we don't render empty tiles so it's ok that their keys are the same. But yea, I'm not 100% on the cases/issues this fixes if @xintongxia could shed some light on that.

@xintongxia xintongxia merged commit 5c0bfd8 into master Sep 23, 2019
@xintongxia xintongxia deleted the xx/fux branch September 23, 2019 19:02
xintongxia pushed a commit that referenced this pull request Sep 26, 2019
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.

None yet

4 participants