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

Index not considered for FloatVertexAttribute in IndexedFaceSet #596

Closed
YuanxiangFranck opened this issue Jan 12, 2016 · 22 comments
Closed

Comments

@YuanxiangFranck
Copy link
Contributor

Hello,

In my project, I have a mesh with data associated to each vertices and I wanted to hide part of the mesh if the data are to lower than a chosen value.
To do that, I used an IndexedFaceSet for the geometry and add the attribute to the shaders with FloatVertexAttribute.
In the example below I created 2 triangles with 4 points, so I can set normal, texture coordinate, for the 4 points: 3* 4 values for coordinate, 2*4 for texture coordinate...
The problem is when I tired to set only 4 values I got the webgl warning :

[GroupMarkerNotSet(crbug.com/242999)!:D0DAD8CDB47F0000]GL ERROR :GL_INVALID_OPERATION : glDrawArrays: attempt to access out of range vertices in attribute 3

For x3dom there is 6 points (some points are duplicated during the creation of the faces). When I tried to set the FloatVertexAttribute, it asked for 6 values not 4. After reading a parts of x3dom code I believe that FloatVertexAttribute should be parsed with the other attributes (normal, positions...)

Franck

<shape >
  <appearance>
    <ComposedShader id='ComposedShader'>
      <field name='threshold' type='SFFloat' value='0.'></field>
      <ShaderPart type='VERTEX' style="display:none;" >
        #ifdef GL_FRAGMENT_PRECISION_HIGH
        precision highp float;
        #else
        precision mediump float;
        #endif

        attribute vec3 position;
        attribute vec3 color;
        attribute float data;

        uniform mat4 modelViewProjectionMatrix;

        varying float fragData;
        varying vec3 fragColor;

        void main()
        {
        fragData = data;
        fragColor = color;
        gl_Position = modelViewProjectionMatrix * vec4(position,1.);
        }
      </ShaderPart>
      <ShaderPart type='FRAGMENT' style="display:none;">
        #ifdef GL_FRAGMENT_PRECISION_HIGH
        precision highp float;
        #else
        precision mediump float;
        #endif

        uniform float threshold;

        varying vec3 fragColor;
        varying float fragData;

        void main()
        {
        if ( threshold > fragData) { discard; }
        gl_FragColor = vec4(fragColor, 1.0);
        }
      </ShaderPart>
    </ComposedShader>
  </Appearance>
  <IndexedFaceSet id="faceSet" solid="false"
                  coordIndex='0 2 1 -1 0 1 3'
                  normalPerVertex='false'
                  ccw='false'>
    <Color color="1 1 0, 0 1 0, 0 0 1, 1 0 0,"></Color>
    <Coordinate point="0 0 0, 0 1 0, 0 0 1, 1 0 0,"></Coordinate>
    <Normal vector="1 0 0, 0 0 1" ></Normal>
    <FloatVertexAttribute
       name="data" numComponents="1" value="1 2 -3 4 -5 6">
    </FloatVertexAttribute>
    <!-- Works -->
    <!-- Return webGL out of range error
         <FloatVertexAttribute id="faceSetAttr" name="data"
                               numComponents="1" value="1 2 3 4">
         </FloatVertexAttribute>
         -->
  </IndexedFaceSet>
</shape>

[EDIT] : I changed the exemple to make it simpler

@andreasplesch
Copy link
Contributor

Could you try creaseAngle='3.14' as an IndexedFaceSet attribute ? I believe the default creaseAngle='0' leads to duplication of shared vertices to be able to have segmented shading with the default shading. Unfortunately, the default is required by the spec. to be 0.
If this works, it could be documented in the FloatVertexAttribute docs as a workaround.
May you be able to fix parsing since you started to look at the code ?

@YuanxiangFranck
Copy link
Contributor Author

It worked, It indeed removed the duplicated points, and no out of range error!
But the problem is not over:
Without duplication that would not work. The FloatVertexAttribute are set following the order of the given indices.
For exemple if if only draw one triangle with coordIndex="0 2 1" for the IndexedFaceSet
If set value="0.1 0.2 0.3" for the FloatVertexAttribute

in the shaders the data would be associated like that : 0.1 for point 0, 0.2 for point 2, 0.3 for point 1
This is because it seems to set values following the given indices.
(This can be tested in my exemple by changing the values and indices)

@YuanxiangFranck
Copy link
Contributor Author

My mistake, it works well!

@andreasplesch
Copy link
Contributor

Ok ! But the mapping of the FloatVertexAttributes uses the coordIndex indices as you explained ?
The spec. does not seem to clearly define how the association should be done:
http://www.web3d.org/documents/specifications/19775-1/V3.3/Part01/components/shaders.html#FloatVertexAttribute

@YuanxiangFranck
Copy link
Contributor Author

Without duplication, there is no problem the first data is associated to the first color and position...
I think that it is important to add it to the documentation.

But the problem is when someone need duplicate points for each faces (it is the case for some scientific data mesh).

@andreasplesch
Copy link
Contributor

But it does here in the third paragraph:

http://www.web3d.org/documents/specifications/19775-1/V3.3/Part01/components/shaders.html#Pervertexattributes

So it should be like you had expected in the first place.

@andreasplesch
Copy link
Contributor

If you need duplicate points for each faces, could you just specify duplicate points in the coords ?

@YuanxiangFranck
Copy link
Contributor Author

Yes that a solution.

I can try to fix the parsing of the FloatVertexAttribute.

I encountered another problem:

It seems that creaseAngle='3.14' disable normalPerVertex='false'

<IndexedFaceSet id="faceSet" solid="false"
                          coordIndex='0 1 2 -1 0 1 3 -1 '
                          creaseAngle='3.14'
                          normalPerVertex='false'>
<Coordinate point="0 0 0, 0 1 0, 1 0 0, 0 0 1,"></Coordinate>
<Normal vector="1 0 0, 0 0 1, 0 0 1, 0 0 1" ></Normal>
<!-- Expected but did not work : 
<Normal vector="1 0 0, 0 0 1," ></Normal>
-->

Should I open a new issue?

@andreasplesch
Copy link
Contributor

Yes, I would suggest to open a new issue.

@andreasplesch
Copy link
Contributor

Thinking a bit more about what the goal was, a solution is often to use a 1d texture map to visualize data on geometry. The data then become the texture coordinates for each vertex.

@YuanxiangFranck
Copy link
Contributor Author

I already have a function code using textureCooridinate (the texCoord are computed from the data, so I computed the data from the texCoord), but in order to have a simpler and more direct code I would like to use FloatVertexAttribute.

@andreasplesch
Copy link
Contributor

OK.

@YuanxiangFranck
Copy link
Contributor Author

The issue can be solved by redefining the handleAttribs function in handle attribut. (inherited from X3DComposedGeometry ).
I belive the problem is the same for all Indexed___ elements. A solution is to change handleAttribs in each class, but that mean lot of copy pasted code...
I think the best is to creates a new class that separate Indexed geometries with the other.

@andreasplesch
Copy link
Contributor

This brings up if you could just use IndexedTriangleSet or TriangleSet since they do not have creaseAngle ?
Although they still may duplicate vertices in the normals per face case.

@andreasplesch
Copy link
Contributor

http://stackoverflow.com/questions/4580690/normals-per-index
And
http://stackoverflow.com/questions/1664402/how-can-i-specify-per-face-colors-when-using-indexed-vertex-arrays-in-opengl-3-x

Explain how normals per face are really multiple normals per vertex for the GPU which means duplication of vertices.

So it is best to supply normals per vertex in the first place.

@YuanxiangFranck
Copy link
Contributor Author

In this issue I think that duplication isn't the problem. The problem is to have a natural input for the FloatVertexAttribute for the Indexed elements. (IndexedFaceSet , IndexedTriangleSet... )

@andreasplesch
Copy link
Contributor

I thought duplication in the creaseAngle=0 case was the problem, since without duplication the FloatVertexAttributes are actually associated with correct vertices ?
Both, creaseAngle=3.14 or normalsPerVertex=false, require duplication of vertices (and normals) for the GPU. So I think the solution you have in mind is to also duplicate the FloatVertexAttributes (just like normals) in this case, and I think this makes sense and should be done.
However, it may be easier to just produce and provide vertices, (constant) per vertex normals and FloatVertexAttributes, eg. all information for each triangle individually, in the first place.
Could you also export (averaged) normals per vertex in a .vtk file ?

@YuanxiangFranck
Copy link
Contributor Author

well, for my project I can duplicate the normals in order to have the normal per vertex.
To fix the issues my idea is to allow anyone to use the FloatVertexAttribute easily and naturally.
So my solution was to fix the parsing for the attributes in the IndexesFaceSet , so one can use FloatVertexAttribute without settings crease angle=3.14 or have problems with normals

@andreasplesch
Copy link
Contributor

Sounds good to me.

@mlimper
Copy link
Contributor

mlimper commented Jan 18, 2016

Thanks to you two for the analysis!

With the new open issue, would it be fine to close this one?

@andreasplesch
Copy link
Contributor

I think a fix which treats FloatVertexAttributes the same way as colors and normals would be good. Could it go into X3DComposedGeometry.js ?
Franck, the spec. defines how nodes should be inherited, so it would better to not have a new class in my view. But x3dom does not always exactly adhere to the spec. in this regard, I believe.

@YuanxiangFranck
Copy link
Contributor Author

Well I have two solution,
The first is to do a little modification in the FieldChanged function in IndexedFaceSet. Like that from the raw data in _cf._attrib I can use indices to change the data befor saving the in node._shape._dynamyField

The second solution change the handleAttritut function in X3DComposedGeometry, and I can change the data before saving them into _cf._attrib.

The problem with the first solution is that I sould copy/past the modification for each Indexed Geometry like : IndexedTriangleSet, IndexedLineSet ...

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

No branches or pull requests

3 participants