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

[DO NOT MERGE] Support gltf #5

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from
Draft

[DO NOT MERGE] Support gltf #5

wants to merge 21 commits into from

Conversation

0b5vr
Copy link

@0b5vr 0b5vr commented Sep 23, 2021

Description

  • Add gltf support.
  • TBD

Points need review

  • I don't really do Rust coding usually! Have no% confidence about my code this time
    • no confidence about resource management and stuff of course
    • module / file management is a part that is most mystery for me
  • Geometry / GeometryAttribute are having vbo / vao as a member. I'm not sure this is a good design decision...
  • We probably have to consider a lot of things about yaml and shader interface
    • yaml interface. Currently, there is a field called gltf that can be specified directly to vs / vs+fs type stages and I don't think this is a good decision
    • geometry attributes. position attribute is passed to shader as location=0 for now. we have several options about this part:
      • naming each attributes as like in vec3 position
      • define macros like #define ATTR_INDEX_POSITION 0 for forward compatibility
    • material informations. we are going to have a lot of uniforms related to materials
      • I have defined material_alpha_cutoff and material_base_color for now
      • Textures are not supported yet and I assume it's a spicy part considering resource management funnies
      • plus we also need macros like ALPHA_MODE_MASK (naming is also an issue)
    • matrices. I'm currently sending model_matrix based on world space model matrix comes with gltf node graph
      • We might want to have camera in near future?

...I realized this PR involves a lot of major design decisions 😩
You can take it a single sample of project exploration and just discard the PR 😅 ,
I already have enjoyed my rare opportunity reading and writing Rust code

@0b5vr
Copy link
Author

0b5vr commented Sep 23, 2021

wait, I was not meant to make a PR on slerpyyy/ instead of 0b5vr/

😩

Some(vao) => {
unsafe {
gl::DeleteVertexArrays(1, &vao);
// gl_debug_check!();
Copy link
Author

Choose a reason for hiding this comment

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

this leaves me an INVALID_OPERATION error on exit.

src/jockey/geometry.rs Outdated Show resolved Hide resolved
src/jockey/geometry.rs Outdated Show resolved Hide resolved
src/jockey/geometry.rs Outdated Show resolved Hide resolved
src/jockey/stage.rs Outdated Show resolved Hide resolved
@0b5vr 0b5vr changed the title [DO NOT MERGE] preview of my "geometry" branch !!!!!!111 [DO NOT MERGE] Support gltf Sep 24, 2021
0b5vr added a commit to 0b5vr/sh4der-jockey that referenced this pull request Sep 25, 2021
@0b5vr
Copy link
Author

0b5vr commented Sep 28, 2021

It's weird that it sometimes fails to get uniform location of textures......

let location = unsafe { gl::GetUniformLocation(program_id, *name) };

@sp4ghet
Copy link
Collaborator

sp4ghet commented Sep 29, 2021

Anything from gl_debug_check! immediately after & before? could be an error elsewhere that breaks OpenGL state if your name is guaranteed to be sound

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