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

setAttribute works on FloatVertexAttribute #610

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

YuanxiangFranck
Copy link
Contributor

Fix to allow the users to update the values in FloatVertexAttribute with setAttribute.
I changed the following geometries :
IndexedFaceSet, IndexedQuadSet, IndexedTriangleSet, IndexedTriangleStripSet, QuadSet, TriangleSet,

Note : For the IndexedFaceSet I did some code cleaning (eebe316),
In the triangulation of the faces the case 2 and case 3 was almost identical, I fused the two cases.

@@ -791,8 +791,21 @@ x3dom.registerNodeType(
hasColor = false;
}
this._mesh._numColComponents = numColComponents;
if (fieldName.startsWith("attrib")) {
var attrName = fieldName.slice(7);
Copy link
Contributor

Choose a reason for hiding this comment

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

I see. But the name of the attribute is not provided after an underscore ("attrib_temperature") but only in the name field of the attrib child node.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The name is given during the call of FiledChanged on the FloatVertexAttribute element
then the FloatVertexAttribute call the fieldChanged from IndexedFaceSet (the code above) with attrib__name_
see : 16fefd1

fieldChanged: function (fieldName) {
                 var that = this;
                 if (fieldName == "value" ||  fieldName == "numComponents") {
                     Array.forEach(this._parentNodes, function (node) {
                         node.fieldChanged("attrib_"+that._vf.name);
                     });
                 }
             }

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. While that probably works. I do not think the node.fieldChanged() function is intended to be reused for internal purposes like this. Why not make a new function updateAttrib(name) or in IndexedFaceSet.js (and all possible parentNodes of FloatVertexAttribute) ? Or do the update right here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea of the new function updateAttrib(name) is not bad, it can be similar to the handleAttibs in X3domComposedGeometryNode.
The only problem is in IFS, I cannot imagine a (proper) way to know which attribute needs update.
The ugly solution is to save in the node the name of the modified attribute (in _vf for exemple).

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, the name in node.updateAttrib(name) would have the name of the changed attribute, no ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you call node.updateAttrib in the IFS, the only information given is FieldChanged("attribs").
A solution would be to write the function node.updateAttrib(name) in FloatVertexAttribute.js, and call it before (or after) the call of FieldChanged on IndexedFaceSet.

@andreasplesch
Copy link
Contributor

This assumes that you have attrib fields with names like "attrib_temperate" in containerfield ?

@YuanxiangFranck
Copy link
Contributor Author

Let's assume the following element :

<FloatVertexAttribute id="temp" name="temperate" numCompenents="1" value="...."><FloatVertexAttribute>

when temp.setAttribute("value", "....") is called :
1- FieldChanged (FloatVertexAttribute) is called with "value" as filedName
then it call the FieldChanged (parent node : IndexedFaceSet for exemple)
2- FieldChanged (IndexedFaceSet) is called with "attrib_temperate" as FieldName
That is why I test FieldName.statsWith(""attrib_), then get the name using FieldName.split(7)

This allow to avoid useless computation time by updating all the attributes.

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.

None yet

2 participants