With r71 I could update vertices of my BoxGeometry for as below:
var vertices = [];
vertices.push(new THREE.Vector3(0.5, 0.5, 0.5));
vertices.push(new THREE.Vector3(0.5, 0.5, -0.5));
vertices.push(new THREE.Vector3(0.5, -0.5, 0.5));
vertices.push(new THREE.Vector3(0.5, -0.5, -0.5));
vertices.push(new THREE.Vector3(-0.5, 0.5, -0.5));
vertices.push(new THREE.Vector3(-0.5, 0.5, 0.5));
vertices.push(new THREE.Vector3(-0.5, -0.5, -0.5));
vertices.push(new THREE.Vector3(-0.5, -0.5, 0.5));
this._mesh.geometry.vertices = vertices;
this._mesh.geometry.applyMatrix(
new THREE.Matrix4().makeTranslation(
dataCoordinates.x,
dataCoordinates.y,
dataCoordinates.z));
this._mesh.geometry.verticesNeedUpdate = true;
It doesn't work anymore on r72.
It doesn't raise any error.
Is it supposed to work? If not, which is the current best practice to update vertices of a geometry?
Do you mind creating a working jsfiddle that we can work with?
Here you go:
r71 (working): http://codepen.io/nicolasrannou/pen/xwVJRr
r72 (NOT working): http://codepen.io/nicolasrannou/pen/KdzBBz
(in the JS settings of codepen, you can change the target THREE.JS build)
Thanks!
@NicolasRannou You are creating a new array of vertices each frame. That is not a very good practice. Instead, use copy
and replace existing values.
@mrdoob The problem is that he is creating a new array of vertices each frame and DirectGeometry.vertices
is assigned Geometry.vertices
.
Maybe we are going to have to make Geometry.vertices
-- and other select properties -- immutable.
@WestLangley Thanks - just calling "set" on every single vertex of the vertices array seems to work.
r72 (working): http://codepen.io/nicolasrannou/pen/zvqyrj
@mrdoob The problem is that he is creating a new array of vertices each frame and
DirectGeometry.vertices
is assignedGeometry.vertices
.
Uhm... but I think it should still work...?
https://github.com/mrdoob/three.js/blob/dev/src/core/DirectGeometry.js#L110
@mrdoob I believe the problem is here:
https://github.com/mrdoob/three.js/blob/dev/src/core/BufferGeometry.js#L368
geometry.__directGeometry
is not being updated.
Any news on this ? I have the same problem here.
@tforgione Replace vertex values, do not reassign them.
Replacing vertex values whould make me change a lot of stuff, I just wanted to know if there will be a fix soon, or if I should stay with the r71.
@tforgione How this will be resolved has not been decided. But it will not occur until r.73.
Ok, thanks for the information !
uvsNeedUpdate flag does not work in r72. Probably it is related to this bug.
Simple example of the problem:
r71 version: https://jsfiddle.net/7hxz6h0c/ (green and red trias, correct)
r72 version: https://jsfiddle.net/a0q4ujfo/ (green and green trias, wrong)
@dimarudol
Try adding this code block to BufferGeometry.updateFromObject()
:
if ( geometry.uvsNeedUpdate ) {
var attribute = this.attributes.uv;
if ( attribute !== undefined ) {
attribute.copyVector2sArray( geometry.uvs );
attribute.needsUpdate = true;
}
geometry.uvsNeedUpdate = false;
}
three.js r.72 master
The lineDistance
logic needs to be looked at, also.
@dimarudol fixed. Thanks @WestLangley!
@mrdoob I'm thinking the uv fix should go in master, too.... Hmmm. Maybe it is time to begin a new release renumbering: 0.72.1
.
@mrdoob I'm thinking the uv fix should go in master, too.... Hmmm. Maybe it is time to being a new release renumbering:
0.72.1
.
It's not a super critical bug... r73 will be out in two weeks anyway 馃槉
OK, well we still have the main issue of this bug report: you can't reassign the vertices array after the first render.
Seems like a workaround is to do delete box.geometry.__directGeometry;
. Maybe we can do add a new flag that recreated the DirectGeometry
.
@mrdoob When I deleted geometry.__directGeometry,the geometry did update,but the rendering performance dropped down,is there a better way to make the geometry update?
Could this problem occur with r75 ?
It seems i got the same problem, when i check via the console the faceVertexUvs has changed but it still display the uv used at the creation of the mesh.
Should i produce a jsfiddle ?
@jniac If you believe you have identified a bug, please see "How to report a bug" in the guidelines.
well, maybe i should, i'm quite a noob on github, i dare not open a new discussion
here's the jsfiddle : https://jsfiddle.net/1opm3681/14/
using ThreeJS r75
if i do understand
geometry.uvsNeedUpdate = true
should be enough to allow the update of the uv, but it doesn't seems ti work.
@jniac Please read my comments above. Change UV values using Vector2.set()
-- do not replace the faceVertexUvs
array.
Thank you !
I was thinking of something like that. Nonetheless i thought that uvsNeedUpdate would allow to refresh the buffer / cache. I believed that uvsNeedUpdate was intended to dispense to update the uv points (by using set). Anyway, thank you.
here is the JSFiddle using Vector2.copy :
https://jsfiddle.net/1opm3681/16/
may it help someone
Thank you so much @WestLangley! It took me nearly all day to identify the fact that this was cursing me, but I'm glad to see your fix.
https://github.com/mrdoob/three.js/issues/7179#issuecomment-144210254
If the decision is to not support the reassignment of any attribute, and require copy()
or set()
instead, then this issue can be closed.
@WestLangley If your last statement is the intended direction, would it be possible to update the documentation on threejs.org? The section on How to update things reads:
The following flags control updating of various geometry attributes. Set flags only for attributes that you need to update, updates are costly. Once buffers change, these flags reset automatically back to false:
var geometry = new THREE.Geometry();
geometry.verticesNeedUpdate = true;
geometry.elementsNeedUpdate = true;
geometry.morphTargetsNeedUpdate = true;
geometry.uvsNeedUpdate = true;
geometry.normalsNeedUpdate = true;
geometry.colorsNeedUpdate = true;
geometry.tangentsNeedUpdate = true;
There should be some note there that one can use copy()
or set()
but that one can't just assign these booleans to true.
For those accustomed to older Three.js versions, it might be helpful to add an optional warning that springs if one attempts to mutate one of these values directly. The warning could be disabled in the constructor for those who don't need them--that would be very helpful to those who would otherwise end up here or reconsulting the docs or migration guide...
Is this open issue still not fixed?
@zwcloud can you share the code that is causing you problems?
If the decision is to not support the reassignment of any attribute, and require copy() or set() instead, then this issue can be closed.
I vote to consider reassignment as an invalid operation and close the issue.
Most helpful comment
Is this open issue still not fixed?