-
Notifications
You must be signed in to change notification settings - Fork 262
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
Added onLoadStart & onLoadComplete callbacks #440
base: master
Are you sure you want to change the base?
Conversation
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.
Awesome, thank you! I've added a couple comments. A few other things to that should be added:
- add initialization for
onLoadStart
andonLoadComplete
to the constructor - update d.ts file
- update README
if ( this.onLoadComplete ) { | ||
|
||
this.onLoadComplete( ); | ||
this.loading = false; |
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.loading
should be set to false regardless of whether onLoadComplete is set.
@@ -675,6 +676,29 @@ export class TilesRenderer extends TilesRendererBase { | |||
cached.scene = scene; | |||
cached.metadata = metadata; | |||
|
|||
if ( ! this.loading && this.parseQueue.items.length > 0 ) { |
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 think we should check this.parseQueue.items.length > 0 || this.downloadQueue.items.length > 0
|
||
} | ||
|
||
if ( this.loading && this.parseQueue.items.length === 0 && this.downloadQueue.items.length === this.parseQueue.items.length ) { |
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.
Why not check if this.downloadQueue.items.length === 0
?
if ( ! this.loading && this.parseQueue.items.length > 0 ) { | ||
|
||
if ( this.onLoadStart ) { | ||
|
||
this.onLoadStart( ); | ||
|
||
} | ||
this.loading = true; | ||
|
||
} |
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.
It occurs to me that this will not trigger if we're in a case where only one tile has downloaded - because this condition will be checked only once a file has been fully parsed. Perhaps this "onStart" condition should go at the end of the update function, instead?
This pr adds the callback
onLoadStart
&onLoadComplete
for all loading starts and ends.#389
How to test:
I used mars.js example for the test and assigned
onLoadStart
&onLoadComplete
callbacks to the tiles.