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

Primitive Mode TRIANGLE_STRIP and TRIANGLE_FAN implementation, LINE_LOOP render fix #22

Closed

Conversation

Hexer611
Copy link

@Hexer611 Hexer611 commented Aug 6, 2024

According to these two issues;

These primitive modes are not implemented:

  • TRIANGLE_STRIP
  • TRIANGLE_FAN

And this primitive mode is not rendered correctly:

  • LINE_LOOP

TRIANGLE_STRIP and TRIANGLE_FAN implementation

First of all, we need example assets for the two primitive mode implementations (TRIANGLE_STRIP, TRIANGLE_FAN)
We can find those assets from these two links;

First link includes a gltf file with all the primitive modes embedded in one file. Second one has seperate gltf files where each one is a different primitive mode.

In order to correctly implement these triangle primitive modes gltf-2.0 specification documentation of Khronos Group was used;

From this documentation these two formulas was used;

Triangle Strips
One triangle primitive is defined by each vertex and the two vertices that follow it, according to the equation:

  • pi = {vi, vi+(1+i%2), vi+(2-i%2)}

Triangle Fans
Triangle primitives are defined around a shared common vertex, according to the equation:

  • pi = {vi+1, vi+2, v0}

Since it was necessary to create the indices from scratch this change was necessary;
before

var usage = (
    primitive.mode == DrawMode.Triangles
    || primitive.mode == DrawMode.TriangleStrip
    || primitive.mode == DrawMode.TriangleFan
    )
? AccessorUsage.IndexFlipped
: AccessorUsage.Index;

after

var usage = ( primitive.mode == DrawMode.Triangles )
? AccessorUsage.IndexFlipped
: AccessorUsage.Index;

In order to check if the results was correct gltf viewer of Khronos group was used;

Since I couldn't find any other example assets I used blender to generate them. These are the steps to do so;

  • Import or draw any shape in Blender (I used Blender 4.2)
  • Export using gltf seperate option
  • Add primitive mode to the glft file (or change the existing from 4 TRIANGLE to 5 TRIANGLE_STRIP or 6 TRIANGLE_FAN)
  • Import the asset to Unity and gltf viewer of Khronos group, and compare them
  • The results were looking the same even though the topology was messed up

I am not sure if using blender was correct way to check that but the original assets were correctly imported.

LINE_LOOP render display fix

As stated in the issue it was needed to add one more index to the end of the original index list. In line 3661 Array.Copy was used (I don't know if Job system was supposed to be used here too)

Unity version 2022.3.38f1 was used in this PR

@unity-cla-assistant
Copy link

unity-cla-assistant commented Aug 6, 2024

CLA assistant check
All committers have signed the CLA.

@atteneder
Copy link
Collaborator

Excellent work and pull request!

I've given it minor finishing touches and merged it internally. It'll land in an upcoming release.

Thanks a lot!

@atteneder atteneder closed this Aug 26, 2024
@Hexer611
Copy link
Author

Thank you @atteneder !
I'll see if I can contribute more in the future :D
Is it possible for me to see those changes at the moment, or do I wait for the release?

Needle-Mirror-Bot pushed a commit to needle-mirror/com.unity.cloud.gltfast that referenced this pull request Sep 6, 2024
## [6.8.0] - 2024-09-05

### Added
- (Import) Setting to create textures readable. This allows users to access resulting textures from their scripts.
- (Export) Non-readable meshes can be exported as well now.
- (Export) Added support for exporting meshes with vertex compression enabled (effectively converting 16-bit float positions/normals/tangents/texture coordinates to 32-bit floats).
- (Export) [Buffer view targets](https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#_bufferview_target) are set properly now.
- (Import) Support for mesh primitive modes `TRIANGLE_STRIP` and `TRIANGLE_FAN` (thanks [Hexer611][Hexer611] for [#22](Unity-Technologies/com.unity.cloud.gltfast#22))

### Fixed
- (Export) Writing to files on the web via IndexedDB now works (fixes [#625](atteneder/glTFast#625))
- (Export) test results are validated again.
- (Export) Removed expendable JSON content when exporting unlit materials without color or texture applied.
- Primitve mode LINE_LOOP works as expected (thanks [Hexer611][Hexer611] for [#22](Unity-Technologies/com.unity.cloud.gltfast#22)).
- (Test) Fail export test if glTF JSON contains unexpected or misses expected properties.
- Increased resilience against invalid animation data.
- Broken link in `CONTRIBUTING.md` (thanks [Hexer611][Hexer611] for [#22](Unity-Technologies/com.unity.cloud.gltfast#23)).
- Loading glTFs with unknown texture extensions (e.g. WebP, `EXT_texture_webp`) now works (fixes [#705](atteneder/glTFast#705)).
This pull request was closed.
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.

3 participants