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.

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

Implementation of baseVertex support #6978

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+51 −4
Open

Implementation of baseVertex support #6978

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
87833dc
Implementation of baseVertex support
AlexAPPi Sep 20, 2024
3461a1c
Code refactoring: https://github.com/playcanvas/engine/pull/6978#disc…
AlexAPPi Sep 20, 2024
fed4c64
Code refactoring, documentation clarification.
AlexAPPi Sep 21, 2024
215258f
Fix batch baseVertex offset.
AlexAPPi Sep 22, 2024
819041d
Merge branch 'playcanvas:main' into main
AlexAPPi Nov 19, 2024
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
2 changes: 1 addition & 1 deletion src/platform/graphics/webgpu/webgpu-graphics-device.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ class WebgpuGraphicsDevice extends GraphicsDevice {
if (ib) {
this.indexBuffer = null;
passEncoder.setIndexBuffer(ib.impl.buffer, ib.impl.format);
passEncoder.drawIndexed(primitive.count, numInstances, primitive.base, 0, 0);
passEncoder.drawIndexed(primitive.count, numInstances, primitive.base, primitive.baseVertex || 0, 0);
} else {
passEncoder.draw(primitive.count, numInstances, primitive.base, 0);
}
Expand Down
7 changes: 5 additions & 2 deletions src/scene/batching/batch-manager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ class BatchManager {
batch = new Batch(meshInstances, dynamic, batchGroupId);
this._batchList.push(batch);

let indexBase, numIndices, indexData;
let indexBase, indexBaseVertex, numIndices, indexData;
let verticesOffset = 0;
let indexOffset = 0;
let transform;
Expand Down Expand Up @@ -738,6 +738,7 @@ class BatchManager {
// index buffer
if (mesh.primitive[0].indexed) {
indexBase = mesh.primitive[0].base;
indexBaseVertex = mesh.primitive[0].baseVertex || 0;
numIndices = mesh.primitive[0].count;

// source index buffer data mapped to its format
Expand All @@ -746,6 +747,8 @@ class BatchManager {

} else { // non-indexed

indexBaseVertex = 0;

const primitiveType = mesh.primitive[0].type;
if (primitiveType === PRIMITIVE_TRIFAN || primitiveType === PRIMITIVE_TRISTRIP) {
if (mesh.primitive[0].count === 4) {
Expand All @@ -760,7 +763,7 @@ class BatchManager {
}

for (let j = 0; j < numIndices; j++) {
indices[j + indexOffset] = indexData[indexBase + j] + verticesOffset;
indices[j + indexOffset] = indexData[indexBase + indexBaseVertex + j] + verticesOffset;
}

indexOffset += numIndices;
Expand Down
3 changes: 2 additions & 1 deletion src/scene/mesh.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,11 @@ class Mesh extends RefCountedObject {
*
* - `base` is the offset of the first index or vertex to dispatch in the draw call.
* - `count` is the number of indices or vertices to dispatch in the draw call.
* - `baseVertex` is the number added to each index value before indexing into the vertex buffers. (available only for WebGPU and Batching)
* - `indexed` specifies whether to interpret the primitive as indexed, thereby using the
* currently set index buffer.
*
* @type {{type: number, base: number, count: number, indexed?: boolean}[]}
* @type {{type: number, base: number, count: number, baseVertex?: number, indexed?: boolean}[]}
Copy link
Contributor

Choose a reason for hiding this comment

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

we should initialize it to 0 here as well, instead of leaving it undefined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In most cases, this parameter will be of little use, and there is no need to store it in memory for each primitive.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's really not much memory. It's more important to have these objects the same "layout" to avoid chrome possibly deoptimizing functions as they accept polymorphic parameters.

*/
primitive = [{
type: 0,
Expand Down