Skip to content

Refine GSplatRenderer API: rename methods for clarity #8522

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.

Sign up for GitHub

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

Merged
mvaligursky merged 1 commit into from
Mar 11, 2026
Merged

Refine GSplatRenderer API: rename methods for clarity #8522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 26 additions & 27 deletions src/scene/gsplat-unified/gsplat-manager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,37 +450,40 @@ class GSplatManager {
this.keyGenerator = null;
this.gpuSorter?.destroy();
this.gpuSorter = null;
// Disable once when destroying both compaction systems together.
const disableIndirectDraw = false;
this.renderer.disableIndirectDraw();
this.destroyIntervalCompaction(disableIndirectDraw);
this.destroyCompaction(disableIndirectDraw);

// Switch renderer to CPU mode once, before destroying both compaction systems.
const useCpuSort = false;
this.renderer.setCpuSortedRendering();
this.destroyIntervalCompaction(useCpuSort);
this.destroyCompaction(useCpuSort);
Comment on lines +454 to +458
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

GSplatManager.destroy() calls renderer.destroy() before destroyGpuSorting(), but destroyGpuSorting() now calls renderer.setCpuSortedRendering(). If destroyGpuSorting() runs after the renderer has been destroyed, this will touch destroyed MeshInstance/Material state and can throw. Either call destroyGpuSorting() before renderer.destroy(), or guard/skip the renderer mode switch when the manager is being destroyed (e.g. based on this._destroyed / renderer existence).

Copilot uses AI. Check for mistakes.
Comment on lines +455 to +458
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The local useCpuSort constant is set to false but is named as if it indicates switching to CPU sorting; it's only used to suppress additional mode switches in the helper teardown calls. Consider renaming it to something like switchRendererToCpuSort / alreadySwitchedToCpuSort (or just pass false directly) to avoid confusion with the useCpuSort parameter name in the teardown methods.

Suggested change
const useCpuSort = false;
this.renderer.setCpuSortedRendering();
this.destroyIntervalCompaction(useCpuSort);
this.destroyCompaction(useCpuSort);
this.renderer.setCpuSortedRendering();
this.destroyIntervalCompaction(false);
this.destroyCompaction(false);

Copilot uses AI. Check for mistakes.
}

/**
* Destroys interval compaction resources and disables indirect draw on the renderer.
* Destroys interval compaction resources.
*
* @param {boolean} [useCpuSort] - Whether to switch the renderer to CPU-sorted mode.
* @private
*/
destroyIntervalCompaction(disableIndirectDraw = true) {
destroyIntervalCompaction(useCpuSort = true) {
if (this.intervalCompaction) {
if (disableIndirectDraw) {
this.renderer.disableIndirectDraw();
if (useCpuSort) {
this.renderer.setCpuSortedRendering();
}
this.intervalCompaction.destroy();
this.intervalCompaction = null;
}
}

/**
* Destroys compaction resources and disables indirect draw on the renderer.
* Destroys compaction resources.
*
* @param {boolean} [useCpuSort] - Whether to switch the renderer to CPU-sorted mode.
* @private
*/
destroyCompaction(disableIndirectDraw = true) {
destroyCompaction(useCpuSort = true) {
if (this.compaction) {
if (disableIndirectDraw) {
this.renderer.disableIndirectDraw();
if (useCpuSort) {
this.renderer.setCpuSortedRendering();
}
this.compaction.destroy();
this.compaction = null;
Expand Down Expand Up @@ -531,8 +534,8 @@ class GSplatManager {
this.cpuSorter.updateCentersForSplats(currentState.splats);
}

// Disable indirect draw on the renderer (also hides until update() restores visibility)
this.renderer.disableIndirectDraw();
// Switch renderer to CPU-sorted mode (also hides until update() restores visibility)
this.renderer.setCpuSortedRendering();
}

get material() {
Expand Down Expand Up @@ -1281,7 +1284,7 @@ class GSplatManager {
this.workBuffer.destroy();
this.workBuffer = new GSplatWorkBuffer(this.device, currentFormat);
this.renderer.workBuffer = this.workBuffer;
this.renderer.configureMaterial();
this.renderer.onWorkBufferFormatChanged();
this._workBufferFormatVersion = this.workBuffer.format.extraStreamsVersion;
this._workBufferRebuildRequired = true;
this.sortNeeded = true;
Expand Down Expand Up @@ -1723,10 +1726,7 @@ class GSplatManager {
*/
applyGpuSortResults(worldState, sortedIndices) {
const ic = /** @type {GSplatIntervalCompaction} */ (this.intervalCompaction);
this.renderer.setIndirectDraw(this.indirectDrawSlot, sortedIndices, /** @type {StorageBuffer} */ (ic.numSplatsBuffer));

// Update renderer for indirect draw (instancingCount and numSplats are GPU-driven)
this.renderer.updateIndirect(worldState.textureSize);
this.renderer.setGpuSortedRendering(this.indirectDrawSlot, sortedIndices, /** @type {StorageBuffer} */ (ic.numSplatsBuffer), worldState.textureSize);
}

/**
Expand Down Expand Up @@ -1775,14 +1775,13 @@ class GSplatManager {
this.allocateAndWriteIntervalIndirectArgs(this.lastCompactedNumIntervals);
const gpuSorter = /** @type {ComputeRadixSort} */ (this.gpuSorter);
const ic = /** @type {GSplatIntervalCompaction} */ (this.intervalCompaction);
this.renderer.setIndirectDraw(this.indirectDrawSlot, /** @type {StorageBuffer} */ (gpuSorter.sortedIndices), /** @type {StorageBuffer} */ (ic.numSplatsBuffer));
this.renderer.setGpuSortedRendering(this.indirectDrawSlot, /** @type {StorageBuffer} */ (gpuSorter.sortedIndices), /** @type {StorageBuffer} */ (ic.numSplatsBuffer), sortedState.textureSize);
} else {
// CPU sort path: compacted buffer already contains sorted visible splat IDs
this.allocateAndWriteIndirectArgs(this.lastCompactedTotalSplats);
const compaction = /** @type {GSplatCompaction} */ (this.compaction);
this.renderer.setIndirectDraw(this.indirectDrawSlot, /** @type {StorageBuffer} */ (compaction.compactedSplatIds), /** @type {StorageBuffer} */ (compaction.numSplatsBuffer));
this.renderer.setGpuSortedRendering(this.indirectDrawSlot, /** @type {StorageBuffer} */ (compaction.compactedSplatIds), /** @type {StorageBuffer} */ (compaction.numSplatsBuffer), sortedState.textureSize);
}
this.renderer.updateIndirect(sortedState.textureSize);
}

/**
Expand Down Expand Up @@ -1824,13 +1823,13 @@ class GSplatManager {

this.allocateAndWriteIndirectArgs(elementCount);

// Set up indirect draw: compacted buffer already contains sorted visible splatIds
this.renderer.setIndirectDraw(
// Set up GPU-sorted rendering: compacted buffer already contains sorted visible splatIds
this.renderer.setGpuSortedRendering(
this.indirectDrawSlot,
/** @type {StorageBuffer} */ (this.compaction.compactedSplatIds),
/** @type {StorageBuffer} */ (this.compaction.numSplatsBuffer)
/** @type {StorageBuffer} */ (this.compaction.numSplatsBuffer),
sortedState.textureSize
);
this.renderer.updateIndirect(sortedState.textureSize);
}

/**
Expand Down
44 changes: 20 additions & 24 deletions src/scene/gsplat-unified/gsplat-quad-renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ class GSplatQuadRenderer extends GSplatRenderer {
return this._material;
}

onWorkBufferFormatChanged() {
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

onWorkBufferFormatChanged() triggers a full material reconfigure, but it doesn't update the inherited _workBufferFormatVersion cache to the new workBuffer.format.extraStreamsVersion. After a workBuffer swap, this makes _syncWithWorkBufferFormat() think the format changed again and perform another sync/update pass on the next frame. Consider updating _workBufferFormatVersion here (or calling super.onWorkBufferFormatChanged() if the base is updated to do it) to keep the cache consistent and avoid redundant work.

Suggested change
onWorkBufferFormatChanged() {
onWorkBufferFormatChanged() {
super.onWorkBufferFormatChanged();

Copilot uses AI. Check for mistakes.
this.configureMaterial();
}

configureMaterial() {
const { workBuffer } = this;

Expand Down Expand Up @@ -213,48 +217,40 @@ class GSplatQuadRenderer extends GSplatRenderer {
}

/**
* Updates renderer for indirect draw mode. The instance count and numSplats
* are GPU-driven via indirect draw args and a storage buffer.
*
* @param {number} textureSize - The work buffer texture size.
*/
updateIndirect(textureSize) {
this._material.setParameter('splatTextureSize', textureSize);
this.meshInstance.visible = true;

// Ensure instancingCount is non-zero so the forward/shadow renderers don't
// skip this draw call. The actual instance count is GPU-driven via indirect args.
if (this.meshInstance.instancingCount <= 0) {
this.meshInstance.instancingCount = 1;
}
}

/**
* Configures indirect draw on the mesh instance and binds compaction buffers.
* Must be called each frame when compaction is active (slots are per-frame).
* Configures the renderer to use GPU-sorted data for rendering.
*
* @param {number} drawSlot - The indirect draw slot index in the device's buffer.
* @param {StorageBuffer} compactedSplatIds - Buffer containing sorted visible splat IDs.
* @param {StorageBuffer} sortedIds - Buffer containing sorted visible splat IDs.
* @param {StorageBuffer} numSplatsBuffer - Buffer containing numSplats for vertex shader.
* @param {number} textureSize - The work buffer texture size.
*/
setIndirectDraw(drawSlot, compactedSplatIds, numSplatsBuffer) {
setGpuSortedRendering(drawSlot, sortedIds, numSplatsBuffer, textureSize) {
this.meshInstance.setIndirect(null, drawSlot, 1);

// Bind compaction buffers for vertex shader
this._material.setParameter('compactedSplatIds', compactedSplatIds);
this._material.setParameter('compactedSplatIds', sortedIds);
this._material.setParameter('numSplatsStorage', numSplatsBuffer);

// Set GSPLAT_INDIRECT_DRAW define if not already set
if (!this._material.getDefine('GSPLAT_INDIRECT_DRAW')) {
this._material.setDefine('GSPLAT_INDIRECT_DRAW', true);
this._material.update();
}

this._material.setParameter('splatTextureSize', textureSize);
this.meshInstance.visible = true;

// Ensure instancingCount is non-zero so the forward/shadow renderers don't
// skip this draw call. The actual instance count is GPU-driven via indirect args.
if (this.meshInstance.instancingCount <= 0) {
this.meshInstance.instancingCount = 1;
}
}

/**
* Disables indirect draw, restoring the renderer to direct (CPU-sorted) mode.
* Switches the renderer to CPU-sorted rendering mode.
*/
disableIndirectDraw() {
setCpuSortedRendering() {
this.meshInstance.setIndirect(null, -1);

if (this._material.getDefine('GSPLAT_INDIRECT_DRAW')) {
Expand Down
30 changes: 12 additions & 18 deletions src/scene/gsplat-unified/gsplat-renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ class GSplatRenderer {
this._workBufferFormatVersion = workBuffer.format.extraStreamsVersion;
}

destroy() {
}

/**
* Sets the render mode for this renderer.
*
Expand All @@ -76,9 +79,10 @@ class GSplatRenderer {
}

/**
* Configures the renderer's material after a work buffer format change.
* Called when the work buffer format has changed. Derived classes reconfigure
* their rendering resources (materials, pipelines, bindings, etc.).
*/
configureMaterial() {
onWorkBufferFormatChanged() {
}

/**
Expand All @@ -91,27 +95,20 @@ class GSplatRenderer {
}

/**
* Updates the renderer for indirect (GPU-driven) draw mode.
*
* @param {number} textureSize - The work buffer texture size.
*/
updateIndirect(textureSize) {
}

/**
* Configures indirect draw and binds compaction buffers.
* Configures the renderer to use GPU-sorted data for rendering.
*
* @param {number} drawSlot - The indirect draw slot index.
* @param {StorageBuffer} compactedSplatIds - Buffer containing sorted visible splat IDs.
* @param {StorageBuffer} sortedIds - Buffer containing sorted visible splat IDs.
* @param {StorageBuffer} numSplatsBuffer - Buffer containing the visible splat count.
* @param {number} textureSize - The work buffer texture size.
*/
setIndirectDraw(drawSlot, compactedSplatIds, numSplatsBuffer) {
setGpuSortedRendering(drawSlot, sortedIds, numSplatsBuffer, textureSize) {
}

/**
* Disables indirect draw, restoring the renderer to direct (CPU-sorted) mode.
* Switches the renderer to CPU-sorted rendering mode.
*/
disableIndirectDraw() {
setCpuSortedRendering() {
}

/**
Expand All @@ -135,9 +132,6 @@ class GSplatRenderer {
*/
updateOverdrawMode(params) {
}

destroy() {
}
}

export { GSplatRenderer };
Loading