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

Stats update #953

Merged
merged 5 commits into from Mar 6, 2019

Conversation

Projects
None yet
3 participants
@tsherif
Copy link
Member

tsherif commented Mar 6, 2019

  • Update probe.gl to latest alpha
  • Update stat tracking in animation loop to latest API. Now also tracking FPS
  • Use probe.gl StatsWidget to display render time, FPS in instancing, DOF, mandelbrot examples

@tsherif tsherif requested review from ibgreen and Pessimistress Mar 6, 2019


timerElement.update();
if (widgetUpdateCount++ % 60 === 10) {

This comment has been minimized.

Copy link
@Pessimistress

Pessimistress Mar 6, 2019

Contributor

You can use animationProps.tick

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 6, 2019

Contributor

Yes, AnimationLoop supports tick and tock.

  • One counts actual frames
  • The other represents the number of frames that would have fired assuming 60 frames were fired per second (allows animations to remain unaffected by frame rate drops).
@ibgreen

ibgreen approved these changes Mar 6, 2019

Copy link
Contributor

ibgreen left a comment

This is moving forward nicely!

@@ -4,6 +4,7 @@ import {
Program, Texture2D, VertexArray, Buffer, isWebGL2
} from 'luma.gl';
import {Matrix4, radians} from 'math.gl';
import {StatsWidget} from '@probe.gl/stats-widget'

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 6, 2019

Contributor

Longer term idea: We could make it so that the website injects this into all the examples. It would only be available when run as part of website, but we wouldn't need to add this code in 30 different files...


if (!isDemoSupported) {
return;
}

timerElement.update();
if (widgetUpdateCount++ % 60 === 10) {
statsWidget.update();

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 6, 2019

Contributor

Suggestions:

  • Maybe manage the throttling inside the widget.update(), so that we could just call update here and relieve the app of the burden?

  • This "resetting business" seems a little tedious for the app? Could there be a reset() method that just calls reset on all the stats? If some stat needs to be manually managed, maybe allow an option {autoReset: false} when adding that stat to the Stats object?

This comment has been minimized.

Copy link
@tsherif

tsherif Mar 6, 2019

Author Member

This makes a lot of sense. The throttling count could be an argument to the constructor. And a reset loop could be associated with each stat, similar to the formatters.

const statsWidget = new StatsWidget(this.stats, {
containerStyle: 'position: absolute;top: 20px;left: 20px;'
});
statsWidget.setFormatter('CPU Time', stat => `CPU Time: ${stat.getAverageTime().toFixed(2)}`);

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 6, 2019

Contributor

Suggestions:

  • Looks like we could have a default formatter that would get us close to this?
  • And maybe extend the Stats API to offer a more custom formatter to be registered by the stats produced (luma.gl) when it first registers each stat.

Then apps would not need to deal with formatting except when they had special requirements...

This comment has been minimized.

Copy link
@tsherif

tsherif Mar 6, 2019

Author Member

I was thinking of maybe having a set of default formatters like "time", "framerate", "memory", etc. and then have the function argument for full control.

In an earlier iteration I had the formatter associated directly with the Stat, but I think I prefer having anything relating to display of a stat being in the StatsWidget.


timerElement.update();
if (widgetUpdateCount++ % 60 === 10) {

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 6, 2019

Contributor

Yes, AnimationLoop supports tick and tock.

  • One counts actual frames
  • The other represents the number of frames that would have fired assuming 60 frames were fired per second (allows animations to remain unaffected by frame rate drops).
@tsherif

This comment has been minimized.

Copy link
Member Author

tsherif commented Mar 6, 2019

I made the update using tick. I'm tracking the suggestions for API changes to stats here: uber-web/probe.gl#63 Feel free to add more.

@tsherif tsherif merged commit add578a into master Mar 6, 2019

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@tsherif tsherif deleted the stats-update branch Mar 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.