Skip to content

Conversation

@vlucendo
Copy link
Contributor

@vlucendo vlucendo commented Apr 4, 2018

The size params this function passes to THREE.DataTexture were wrong. Also fixed the indentation 🌈

Edit: disabled depthBuffer on another commit too, it isn't needed by GPUComputationRenderer right?

@vlucendo vlucendo changed the title fix createTexture in GPUComputationRenderer GPUComputationRenderer: fix createTexture and disable depthBuffer Apr 4, 2018
@WestLangley
Copy link
Collaborator

@vlucendo What is your use case for calling this method with args different from sizeX, sizeY?

gpuCompute.createTexture( sizeXTexture, sizeYTexture );

... and does your application work correctly when you do that?

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 5, 2018

Edit: disabled depthBuffer on another commit too, it isn't needed by GPUComputationRenderer right?

AFAIK, it is not needed.

@vlucendo
Copy link
Contributor Author

vlucendo commented Apr 5, 2018

What is your use case for calling this method with args different from sizeX, sizeY?

My use case is to create a texture with the same height but different width to store some data that won't be used in a variable but would be passed as a uniform instead. After looking for createTexture on the three.js codebase it never gets called with params, so I supposed the reason for it to have them is to be able to use it as a helper function sometimes.

const texture = gpuCompute.createTexture( 64 );

@WestLangley
Copy link
Collaborator

I think this is a design flaw or bug, and the size should be hardwired to be the same as the gpuComputationRenderer size.

/ping @yomboprime

@yomboprime
Copy link
Collaborator

...to store some data that won't be used in a variable but would be passed as a uniform instead.

I don't fully understand this statement.

so I supposed the reason for it to have them is to be able to use it as a helper function sometimes.

That is correct, it is a utility function as it is createRenderTarget.

I think this is a design flaw or bug, and the size should be hardwired to be the same as the gpuComputationRenderer size.

The parameters are there because usually you create and manipulate textures when using the GPUComputationRenderer. Those textures could be of different size for parallel things not directly under management of the GPUComputationRenderer. You could even have more than one GPUComputationRenderer to compute different things, and pass manually (i.e with extra uniforms) textures (variables) to one another, and they could be of different sizes.

There was a try to refactor the GPUComputationRenderer by expanding it to have variables with different sizes, but it did not settle.

So for me the change is suitable (and also the z-buffer disabling), even though I don't fully understand the use case.

@mrdoob mrdoob added this to the r92 milestone Apr 6, 2018
@vlucendo
Copy link
Contributor Author

vlucendo commented Apr 6, 2018

My use case is not very relevant, I'm making instanced gpgpu meshlines.

In the width of the texture are stored the different path positions of the lines, and every pixel of height is a different line. Only the first point of the path gets the position updated and the rest just copy the data of the previous one. This makes a texture with a size of 128 (path points) * 1024 (number of lines) for example.

To set the initial position of the lines (or to reset it when their life reaches 0) I need a texture of different size (1 pixel per line, 1024px in total) with the initial positions because I only care about the first point of the path.

const computation = new GPUComputationRenderer(pathPoints, linesNum, this.renderer);
const positionsTexture = computation.createTexture();
const positionsVariable = computation.addVariable('tPositions', fragment, positionsTexture);
computation.setVariableDependencies(positionsVariable, [positionsVariable]);

//create initial positions texture
const initialPositions = computation.createTexture(1);

//fill values
const data = initialPositions.image.data;
for(let i = 0; i < data.length; i += 4)
{
     //set x, y, z
}

//pass the texture to the variable as a uniform
positionsVariable.material.uniforms.initialPositions = { value: initialPositions };

const error = computation.init();

I could have done it manually, but createTexture seemed like a utility function that could be used for simple stuff like this or creating textures not processed by GPUComputationRenderer.

@yomboprime
Copy link
Collaborator

That makes sense.

@WestLangley
Copy link
Collaborator

I could have done it manually, but createTexture seemed like a utility function that could be used for simple stuff like this or creating textures not processed by GPUComputationRenderer.

I would change the signature.

this.createTexture = function() {

This is how it is intended to be used in the context of gpuComputationRenderer, and how it is used in the examples. So-called generic utility methods do not belong in this class.

@vlucendo
Copy link
Contributor Author

vlucendo commented Apr 8, 2018

Ok then, changed!

@mrdoob mrdoob merged commit e624c9b into mrdoob:dev Apr 18, 2018
@mrdoob
Copy link
Owner

mrdoob commented Apr 18, 2018

Thanks!

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.

5 participants