Chapter 3 Part 1: Model Loading #90
Conversation
Version with same approach than LearnOpenGL for textures
There was a problem hiding this comment.
I think we should mark the OBJ file as a binary file or something like that so that git doesn't track 200k lines in this commit. Can you mark a file as binary and still have automatic EOL conversions happen?
NogginBops
left a comment
NogginBops
left a comment
There was a problem hiding this comment.
I have some things I want to change with this in the future but I think it's fine to get this merged as it is right now and then I can use my review comments as TODOs in #89.
Common/Model.cs
Outdated
| { | ||
| TextureSlot str; | ||
| mat.GetMaterialTexture(type, i, out str); | ||
| string filename = directory + "/" + str.FilePath; |
There was a problem hiding this comment.
This should be Path.Combine(directory, str.FilePath).
| public string path; | ||
|
|
||
| public static Texture LoadFromFile(string path) | ||
| public static Texture LoadFromFile(string filename, string type = "texture_diffuse") |
There was a problem hiding this comment.
We generally avoid default arguments in opentk code. Also I don't think this field is a good idea, and if we where going to do it I would rather this be an enum rather than a string.
| string number = new string("0"); | ||
| string name = textures[i].type; | ||
| if (name == "texture_diffuse") | ||
| number = new string(""+diffuseNr++); | ||
| else if (name == "texture_specular") | ||
| number = new string("" + specularNr++); // transfer int to string | ||
| else if (name == "texture_normal") | ||
| number = new string("" + normalNr++); // transfer int to string | ||
| else if (name == "texture_height") | ||
| number = new string("" + heightNr++); // transfer int to string |
There was a problem hiding this comment.
I would like to rework this texture solution in it's entirety.
But I think it's fine for this PR, I'll be able to do the rework as part of #89.
|
I just realized that there are two model loading PRs. |
|
Also, after marking |
|
Ok i have removed the backpack.obj and then added it again. |
|
bin and obj and all their combinations are in .gitignore, because of bin and obj folders. I am not sure if we should include rather large model files in this repo |
|
I don't think we should add large files to this repository.
|
|
Looks like this PR got resurrected but here's my two cents on the matter. I'm personally against storing large files outside of the repository because:
Storing large objects in git is fine as long as they don't change a lot. Which, in this case, is not what I think will happen. As for the .gitignore, we can either modify the gitignore rule to keep with |
|
The obj file with textures should be part of the repo. They are not that big and with them properly marked as binaries and the fact that they are not going to be updated frequently makes their size not really a problem. |
|
I confirm the obj file is not supposed to be updated once published |
6 participants
This version is inspired by adamsmith1990 initial work but with the same approach than LearnOpenGL website for textures rendering.
Camera movement is via the left-click on mouse and wireframe/plain texture via the N key.
Tested OK on dotnet6.0 (and revert to dotnet8.0 before commit)