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

useSurfaceSampler is not a composable #440

Open
5 tasks done
alvarosabu opened this issue Jun 15, 2024 · 2 comments
Open
5 tasks done

useSurfaceSampler is not a composable #440

alvarosabu opened this issue Jun 15, 2024 · 2 comments
Labels
bug Something isn't working dx p4-important-bug Violate documented behavior or significantly improve performance (priority)

Comments

@alvarosabu
Copy link
Member

Describe the bug

While adding on-demand invalidation on the useSurfaceSampler and Sampler on #436 I noticed that useSurfaceSampler is not a vue composable per definition (it doesn't deal with any type of reactivity or state), is just a function. See why here https://vuejs.org/guide/reusability/composables#what-is-a-composable

Since we are providing to our users as it was a composable, I would expect that the options are reactive.

export const useSurfaceSampler = (
mesh: Mesh,
count: number = 16,
instanceMesh?: InstancedMesh | null,
weight?: string,
transform?: TransformFn,
) => {
const arr = new Float32Array(count * 16)
const buffer = ref(new InterleavedBuffer(arr, 16))
const updateBuffer = () => {
if (!mesh) { return }
const sampler = new MeshSurfaceSampler(mesh)
if (weight) {
sampler.setWeightAttribute(weight)
}
sampler.build()
const position = new Vector3()
const normal = new Vector3()
const color = new Color()
const dummy = new Object3D()
mesh.updateMatrixWorld(true)
for (let i = 0; i < count; i++) {
sampler.sample(position, normal, color)
if (typeof transform === 'function') {
transform(
{
dummy,
sampledMesh: mesh,
position,
normal,
color,
},
i,
)
}
else {
dummy.position.copy(position)
}
dummy.updateMatrix()
if (instanceMesh) {
instanceMesh.setMatrixAt(i, dummy.matrix)
}
dummy.matrix.toArray(buffer.value.array, i * 16)
}
if (instanceMesh) {
instanceMesh.instanceMatrix.needsUpdate = true
}
buffer.value.needsUpdate = true
}
updateBuffer()
return { buffer }
}

Reproduction

Local playground, run useSurfaceSampler

Steps to reproduce

No response

System Info

System:
    OS: macOS 14.5
    CPU: (8) arm64 Apple M1 Pro
    Memory: 89.09 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.12.2 - ~/.nvm/versions/node/v20.12.2/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.5.0 - ~/.nvm/versions/node/v20.12.2/bin/npm
    pnpm: 8.15.4 - ~/Library/pnpm/pnpm
    bun: 1.0.2 - ~/.bun/bin/bun
  Browsers:
    Brave Browser: 120.1.61.116
    Chrome: 125.0.6422.142
    Safari: 17.5
  npmPackages:
    @tresjs/core: 4.0.2 => 4.0.2 
    @tresjs/leches: ^0.14.0 => 0.14.0

Used Package Manager

pnpm

Code of Conduct

@alvarosabu alvarosabu added bug Something isn't working dx p4-important-bug Violate documented behavior or significantly improve performance (priority) labels Jun 15, 2024
@alvarosabu
Copy link
Member Author

Hi @andretchen0 could you please help me out with this one? It's blocking full support of the on-demand for the sampler #436

@andretchen0
Copy link
Contributor

Hey @alvarosabu ,

Since we are providing to our users as it was a composable, I would expect that the options are reactive.

Which options are you referring to? The arguments to the function?

   mesh: Mesh, 
   count: number = 16, 
   instanceMesh?: InstancedMesh | null, 
   weight?: string, 
   transform?: TransformFn, 

E.g.: https://vuejs.org/guide/reusability/composables.html#input-arguments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dx p4-important-bug Violate documented behavior or significantly improve performance (priority)
Projects
Status: Todo
Development

No branches or pull requests

2 participants