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

refactor(rendering)!: migrate chunk vertex attribute changes #4735

Conversation

pollend
Copy link
Member

@pollend pollend commented May 30, 2021

This migrates chunkMesh to take advantage of this attribute scheme. I also migrated the code to take advantage of GLSL 330. There isn't really any significant performance changes from what I can tell. one nice thing about this change is that renderdoc is able to properly render frames from the game with debug information about incoming vertices and different state of the rendering peipline. should make it easier to debug our render pipeline.

PR: Terasology/CoreRendering#59

image

looks like we don't really take good advantage of the depth pass well. most of the texture is unused from what I can tell.
image

@github-actions github-actions bot added the Type: Refactoring Request for or implementation of pure and automatic refactorings, e.g. renaming, to improve clarity label May 30, 2021
@pollend pollend changed the title refactor(rendering): migrate chunk vertex attribute changes refactor(rendering)!: migrate chunk vertex attribute changes May 30, 2021
pollend added a commit to Terasology/CoreRendering that referenced this pull request May 31, 2021
@pollend pollend requested a review from skaldarnar May 31, 2021 00:01
@pollend
Copy link
Member Author

pollend commented May 31, 2021

so this should be a first pass on this logic. we probably want to tweak this more.

…ks/Terasology into refactor/rendering-migrate-chunk-vertex-attribute
@pollend
Copy link
Member Author

pollend commented May 31, 2021

here is how I get this to work under renderdoc

I've just been building the project in intellij and executing ps aux | grep .Terasology and copying the classpath into RenderDoc

image

/usr/lib/jvm/java-1.11.0-openjdk-amd64/bin/java -Dvisualvm.id=189314906091785 -Xms256m -Xmx1536m -javaagent:/home/michaelpollind/.local/share/JetBrains/Toolbox/apps/IDEA-U/ch-0/211.7142.45/lib/idea_rt.jar=38783:/home/michaelpollind/.local/share/JetBrains/Toolbox/apps/IDEA-U/ch-0/211.7142.45/bin -Dfile.encoding=UTF-8 -classpath /tmp/classpath973167907.jar org.terasology.engine.Terasology -homedir -noCrashReport

@Cervator Cervator added the Topic: Rendering Requests, Issues and Changes related to lighting, meshes, camera, etc. label May 31, 2021
Copy link
Member

@jdrueckert jdrueckert left a comment

Choose a reason for hiding this comment

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

Works on my machine, but there's some minor issues

Comment on lines 90 to 92
//bool checkFlag(int flag, float val) {
// return val > float(flag) - 0.5 && val < float(flag) + 0.5;
//}
Copy link
Member

Choose a reason for hiding this comment

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

if we don't use this anymore, can we remove it?

Comment on lines 33 to 35
//bool checkFlag(uint flag, uint val) {
// return flag == val;
//}
Copy link
Member

Choose a reason for hiding this comment

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

see above


#if defined (NORMAL_MAPPING)
normalMatrix = gl_NormalMatrix;
worldSpaceNormal = gl_Normal;
// normalMatrix = in_normal;
Copy link
Member

Choose a reason for hiding this comment

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

can this be removed?

Comment on lines 127 to 129
vertexWorldPos = in_vert + chunkPositionWorld.xyz;

if (in_frames > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

there seem to be tabs here

Comment on lines 138 to 140
sunVecView = (modelViewMatrix * vec4(sunVec.x, sunVec.y, sunVec.z, 0.0)).xyz;

isUpside = in_normal.y > 0.9 ? 1 : 0;
Copy link
Member

Choose a reason for hiding this comment

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

see above

Comment on lines +13 to +14
gl_Position = projectionMatrix * modelViewMatrix * vec4(in_vert, 1.0);
v_pos = gl_Position;
Copy link
Member

Choose a reason for hiding this comment

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

tabs

varying vec3 worldSpaceNormal;
varying mat3 normalMatrix;
out vec3 worldSpaceNormal;
//out mat3 normalMatrix;
Copy link
Member

Choose a reason for hiding this comment

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

remove?

public final VertexAttributeBinding<Vector3fc, Vector3f> position;
public final VertexAttributeBinding<Vector3fc, Vector3f> normals;
public final VertexAttributeBinding<Vector2fc, Vector2f> uv0;
// public final VertexAttributeBinding<Colorc, Color> color; // color data is unused
Copy link
Member

Choose a reason for hiding this comment

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

remove?

Copy link
Member Author

Choose a reason for hiding this comment

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

I want to keep a note here because we had color data that we're not using. I guess the plan was to have colored lights of some kind :?

Copy link
Member

Choose a reason for hiding this comment

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

There were beautiful colored torches long ago, in either case. This might have been left in place by somebody disabling them "temporarily" hoping they'd be back?

@jdrueckert jdrueckert merged commit 6591cd2 into feature/mesh-attribute-scheme May 31, 2021
@jdrueckert jdrueckert deleted the refactor/rendering-migrate-chunk-vertex-attribute branch May 31, 2021 20:36
@pollend
Copy link
Member Author

pollend commented Jun 1, 2021

here is how I get this to work under renderdoc

I've just been building the project in intellij and executing ps aux | grep .Terasology and copying the classpath into RenderDoc

image

/usr/lib/jvm/java-1.11.0-openjdk-amd64/bin/java -Dvisualvm.id=189314906091785 -Xms256m -Xmx1536m -javaagent:/home/michaelpollind/.local/share/JetBrains/Toolbox/apps/IDEA-U/ch-0/211.7142.45/lib/idea_rt.jar=38783:/home/michaelpollind/.local/share/JetBrains/Toolbox/apps/IDEA-U/ch-0/211.7142.45/bin -Dfile.encoding=UTF-8 -classpath /tmp/classpath973167907.jar org.terasology.engine.Terasology -homedir -noCrashReport

pollend added a commit to Terasology/CoreRendering that referenced this pull request Jun 5, 2021
* refactor(rendering)!: update chunk rendering logic with changes

PR: MovingBlocks/Terasology#4735

* chore: clean up states changes in chunkmesh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Topic: Rendering Requests, Issues and Changes related to lighting, meshes, camera, etc. Type: Refactoring Request for or implementation of pure and automatic refactorings, e.g. renaming, to improve clarity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants