APP-14530 2 Snapshot component add traits #321
Conversation
|
src/lib/ecs/traits.ts
Outdated
| /** | ||
| * Control points buffer: [x, y, z, ox, oy, oz, theta, ...] | ||
| */ | ||
| export const ControlPoints = trait(() => new Uint8Array(0)) |
There was a problem hiding this comment.
the control points, knots, weights and degree is a very specific nurb thing, is it possible we create a single nurb trait with these props instead?
There was a problem hiding this comment.
I considered it. I am also open to just dropping nurbs for now. I realized the control points are poses but I think we actually could just use positions and use that trait instead. But also not sure how much value there is in supporting nurbs at this exact moment.
There was a problem hiding this comment.
I'd recommend we convert the nurbs into a LineGeometry before it ever hits a trait
There was a problem hiding this comment.
There's no value in keeping the nurbs definition in the traits themselves
There was a problem hiding this comment.
There's no value in keeping the nurbs definition in the traits themselves
src/lib/ecs/traits.ts
Outdated
| */ | ||
| export const ColorsRGBA = trait(() => new Uint8Array(0) as Uint8Array<ArrayBufferLike>) | ||
|
|
||
| export const MimeType = trait(() => '') |
There was a problem hiding this comment.
also wondering about these traits too since they will only be used for models, would maybe be good to keep them in a singular trait so they are always packed together and an entity cannot have one but not the others
There was a problem hiding this comment.
Yeah I was thinking I could see a world where they could be used elsewhere, but open to reducing them for now.
There was a problem hiding this comment.
@micheal-parks so we are keeping these as individual traits?
nvm you beat me too it with the update
src/lib/ecs/traits.ts
Outdated
| * RGBA colors buffer: [r, g, b, a, ...] as uint8 (0-255) | ||
| * Can be single color or per-vertex colors. | ||
| */ | ||
| export const ColorsRGBA = trait(() => new Uint8Array(0) as Uint8Array<ArrayBufferLike>) |
There was a problem hiding this comment.
There's already a VertexColors and Opacity trait that covers this
src/lib/ecs/traits.ts
Outdated
| /** | ||
| * Packed poses (sans-theta) buffer: [x, y, z, ox, oy, oz, ...] | ||
| */ | ||
| export const Arrows = trait(() => new Uint8Array(0)) |
There was a problem hiding this comment.
We shouldn't be stuffing binaries into traits, and by the time that we're at an ECS each object should have its own entity, so we should instead be putting this information into the Pose trait
src/lib/ecs/traits.ts
Outdated
| /** | ||
| * Packed positions buffer: [x, y, z, ...] | ||
| */ | ||
| export const Positions = trait(() => new Uint8Array(0)) |
There was a problem hiding this comment.
Same situation here, this information should be fed into the pose trait
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
| export const LineGeometry = trait(() => [] as Vector3[]) | ||
| export const PointsGeometry = trait(() => new Float32Array()) | ||
| /** format [x, y, z, ...] */ | ||
| export const LinePositions = trait(() => new Float32Array()) |
There was a problem hiding this comment.
why even have separate traits for the line and point positions now?
src/lib/ecs/traits.ts
Outdated
| */ | ||
| export const ColorsRGBA = trait(() => new Uint8Array(0) as Uint8Array<ArrayBufferLike>) | ||
|
|
||
| export const MimeType = trait(() => '') |
There was a problem hiding this comment.
@micheal-parks so we are keeping these as individual traits?
nvm you beat me too it with the update
src/lib/components/Line.svelte
Outdated
|
|
||
| {@render children?.()} | ||
| </InstancedMesh> | ||
| {#if dotColor.current && linePositions.current} |
There was a problem hiding this comment.
nit can we update to point color to stay consistent to the api naming
PR Stack: