Skip to content

Conversation

@ffreyer
Copy link
Collaborator

@ffreyer ffreyer commented Oct 24, 2020

Currently

catmesh = FileIO.load(GLMakie.assetpath("cat.obj"));
texture = FileIO.load(GLMakie.assetpath("diffusemap.tga"))
mesh(catmesh, color=texture)

looks wrong because the normal and uv remapping discards points. (Specifically points along the edges of the texture, where one position has two (or more) uv coordinates associated with it.) I fixed that by duplicating the vertices in question and adjusting faces accordingly.

Before:
Screenshot from 2020-10-18 01-07-17

After:
Screenshot from 2020-10-24 04-13-53

@codecov
Copy link

codecov bot commented Oct 24, 2020

Codecov Report

Merging #64 (619ff12) into master (0465764) will increase coverage by 0.42%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #64      +/-   ##
==========================================
+ Coverage   96.49%   96.91%   +0.42%     
==========================================
  Files           8        8              
  Lines         342      357      +15     
==========================================
+ Hits          330      346      +16     
+ Misses         12       11       -1     
Impacted Files Coverage Δ
src/io/obj.jl 95.83% <100.00%> (+2.85%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0465764...1d4fd54. Read the comment docs.

@ffreyer ffreyer changed the title Fix remapping of obj files Fix remapping of obj files (WIP) Oct 24, 2020
@ffreyer
Copy link
Collaborator Author

ffreyer commented Oct 24, 2020

Did some performance tweaking. A mesh with 120k faces loaded in ~0.65s before this pr, ~12s after the first commit and ~1s after the latest.

@ffreyer ffreyer changed the title Fix remapping of obj files (WIP) Fix remapping of obj files Oct 24, 2020
@ffreyer ffreyer changed the title Fix remapping of obj files Fix vertex remapping of obj files Oct 24, 2020
@ffreyer
Copy link
Collaborator Author

ffreyer commented Mar 8, 2021

I noticed that if there is a vertex with indices (1,), (1, 1) (1, 1, 1) etc it gets overwritten by the next vertex with position index 1. I switched to using typemax because of that.

@SimonDanisch
Copy link
Member

Oh wow.... And also, completely forgot about this awesome PR... We should just merge it quickly!

@ffreyer
Copy link
Collaborator Author

ffreyer commented Mar 8, 2021

Same 😆 I think you asked me on Slack if these changes could be made/moved to GeometryBasics and I had no idea how to do that...

@ffreyer
Copy link
Collaborator Author

ffreyer commented Mar 8, 2021

If GeometryBasics could handle faces with different indices for positions, uvs and normals this would be unnecessary. I don't know if it can nor how to get there though.

@ffreyer
Copy link
Collaborator Author

ffreyer commented Mar 8, 2021

Actually there still seems to be something wrong with this. I'm losing a normal somehow...

Nvm

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.

2 participants