Skip to content

Comments

fix(engine): Re-enable GPU timing metrics in AnimationLoop#2505

Open
chrisgervang wants to merge 2 commits intomasterfrom
claude/fix-gpu-timing-metrics-mcFD1
Open

fix(engine): Re-enable GPU timing metrics in AnimationLoop#2505
chrisgervang wants to merge 2 commits intomasterfrom
claude/fix-gpu-timing-metrics-mcFD1

Conversation

@chrisgervang
Copy link
Contributor

@chrisgervang chrisgervang commented Feb 3, 2026

The GPU timing code in AnimationLoop was commented out during the v9
migration when the old Query class was removed. This commit re-enables
GPU time measurement using the new QuerySet API:

  • Initialize GPU time query when 'timer-query-webgl' feature is available
  • Use WEBGLQuerySet's beginTimestampQuery/endTimestampQuery methods
  • Record GPU time results in _beginFrameTimers() when available
  • Gracefully handle unsupported environments (WebGPU, missing extension)
  • Clean up query resources in destroy()

This fixes deck.gl's gpuTimePerFrame metric always reporting 0.

Related: visgl/deck.gl#9958

https://claude.ai/code/session_01H614UvusgZQfwVssKtezh9


Note

Medium Risk
Touches core render-loop timing/instrumentation and introduces WebGL timer-query usage; failures could affect frame timing or stability on some drivers, though the code is feature-gated and has fallback/cleanup.

Overview
Re-enables per-frame GPU timing in AnimationLoop using the new QuerySet timer-query API when the timer-query-webgl feature is available, and records results into the existing GPU Time stat.

Adds graceful fallback when timer queries aren’t supported/initialization fails, and ensures any created query resources are destroyed in destroy(). Includes a new test asserting feature-gated query creation and cleanup.

Written by Cursor Bugbot for commit d3c34fe. This will update automatically on new commits. Configure here.

The GPU timing code in AnimationLoop was commented out during the v9
migration when the old Query class was removed. This commit re-enables
GPU time measurement using the new QuerySet API:

- Initialize GPU time query when 'timer-query-webgl' feature is available
- Use WEBGLQuerySet's beginTimestampQuery/endTimestampQuery methods
- Record GPU time results in _beginFrameTimers() when available
- Gracefully handle unsupported environments (WebGPU, missing extension)
- Clean up query resources in destroy()

This fixes deck.gl's gpuTimePerFrame metric always reporting 0.

Related: visgl/deck.gl#9958

https://claude.ai/code/session_01H614UvusgZQfwVssKtezh9
Verifies that AnimationLoop:
- Creates gpuTime and cpuTime stats regardless of timer support
- Initializes _gpuTimeQuery only when timer-query-webgl is available
- Cleans up query resources on destroy without errors

https://claude.ai/code/session_01H614UvusgZQfwVssKtezh9
@chrisgervang chrisgervang requested a review from ibgreen February 3, 2026 01:24
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

// GPU timing not available - ignore
this._gpuTimeQuery = null;
}
}
Copy link

Choose a reason for hiding this comment

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

WebGPU devices crash on missing QuerySet methods

High Severity

The feature check device.features.has('timer-query-webgl') passes on WebGPU devices because 'timer-query-webgl' is in WEBGPU_ALWAYS_FEATURES. However, WebGPUQuerySet does not implement the methods isResultAvailable(), isTimerDisjoint(), getTimerMilliseconds(), beginTimestampQuery(), or endTimestampQuery() that the code calls via type assertion. This causes a runtime crash like "query.isResultAvailable is not a function" on any WebGPU device. The check needs to also verify device.type === 'webgl'.

Additional Locations (1)

Fix in Cursor Fix in Web

isTimerDisjoint(): boolean;
getTimerMilliseconds(): number;
beginTimestampQuery(): void;
};
Copy link

Choose a reason for hiding this comment

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

Duplicate inline type assertions for QuerySet methods

Low Severity

Two separate inline type assertions define the same WEBGLQuerySet interface in different places. Lines 526-531 define isResultAvailable, isTimerDisjoint, getTimerMilliseconds, beginTimestampQuery, while line 549 separately defines endTimestampQuery. These could be consolidated into a single private type alias at the file level.

Fix in Cursor Fix in Web

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.

2 participants