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

Track the number of tiles loading since loading had finished previously #920

Merged
merged 4 commits into from
Jan 13, 2025

Conversation

gkjohnson
Copy link
Contributor

@gkjohnson gkjohnson commented Jan 11, 2025

Fix #906

TODO

  • Rename inCacheBeforeReset - inCacheSinceLoad?
  • Consider exposing convenience members / function - loadProgress / loadProgressSinceCompletion, getLoadProgress( sinceLoad = true )

@gkjohnson gkjohnson marked this pull request as draft January 11, 2025 03:42
@gkjohnson
Copy link
Contributor Author

gkjohnson commented Jan 11, 2025

cc @sguimmara This should be what you're looking for regarding #906. Test it out and let me know what you think. Calculating the load percent since the last load completed should work like so:

const active = tiles.stats.downloading + tiles.stats.parsing;
const total = tiles.stats.inCacheSinceLoad;
console.log( ( 100 * ( 1.0 - active / total ) ).toFixed( 2 ) );

Thought inCacheSinceLoad will reset to 0 once a load has completed so you'll have to check for that case to avoid dividing by 0.

I'm open to any suggestions for exposing both load percents as public members, as well.

@sguimmara
Copy link
Contributor

That seems to do the job ! Thanks. Just one slight change: if total is zero, the value is NaN. In my case, I consider that if total is zero, then the progress is 100%.

@sguimmara
Copy link
Contributor

I also updated TilesRendererBase.d.ts with the following additions:

+ export type Statistics = {
+	inCacheSinceLoad: number,
+	inCache: number,
+	parsing: number,
+	downloading: number,
+	failed: number,
+	inFrustum: number,
+	used: number,
+	active: number,
+	visible: number,
+ };

export class TilesRendererBase {

	readonly rootTileSet : object | null;
	readonly root : object | null;

+	stats: Statistics;

But that might not be a good idea to expose the stats if we use a method to query the progress instead

@gkjohnson
Copy link
Contributor Author

gkjohnson commented Jan 13, 2025

I'd like to expose this as a member but I'm struggling with naming. Perhaps both percentages don't need to be exposed and "loadPercent" is enough.

@sguimmara
Copy link
Contributor

I'd like to expose this as a member but I'm struggling with naming. Perhaps both percentages don't need to be exposed and "loadPercent" is enough.

I wouldn't expose a percentage (as in the [0, 100] range), but a normalized value in the [0, 1] range though.

What about a .getLoadingProgress() method ?

@gkjohnson
Copy link
Contributor Author

I wouldn't expose a percentage (as in the [0, 100] range), but a normalized value in the [0, 1] range though.

I plan to expose it as [0, 1]

What about a .getLoadingProgress() method ?

Is this preferred to a tiles.loadPercent or loadProgress getter?

@sguimmara
Copy link
Contributor

Is this preferred to a tiles.loadPercent or loadProgress getter?

Will loadPercent avoid NaN values ? If so, then let's use it directly.

@gkjohnson
Copy link
Contributor Author

Will loadPercent avoid NaN values ? If so, then let's use it directly.

Yes it returns 1 if the total in cache value is 0.

@gkjohnson gkjohnson marked this pull request as ready for review January 13, 2025 12:06
@gkjohnson gkjohnson merged commit dc8d203 into master Jan 13, 2025
2 checks passed
@gkjohnson gkjohnson deleted the load-amount branch January 13, 2025 12:06
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.

Granular progress tracking ?
2 participants