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

Adding re-texturing option: #462

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Conversation

thunders82
Copy link

allows to backup and reuse the atlas to re-texture the mesh later using
the same UV's

geoffrey added 10 commits July 26, 2019 13:29
allows to backup and reuse the atlas to re-texture the mesh later using 
the same UV's
…e-texture the mesh later using the same UV's
…e-texture the mesh later using the same UV's

- fix forwin32 path creation
…e-texture the mesh later using the same UV's - fix forwin32 path creation
…e-texture the mesh later using the same UV's - fix forwin32 path creation

-sorry for the mess...
…e-texture the mesh later using the same UV's - fix forwin32 path creation -sorry for the mess...
…e-texture the mesh later using the same UV's

- fix for win32 path creation. 2nd attempt
…e-texture the mesh later using the same UV's - fix for win32 path creation. 3rd attempt
@thunders82
Copy link
Author

Sorry for the noise I created. I'm learning Atom+git+github the hard way I suppose.
I'm closing this pull request until I understood how to fix this windows compilation issue without having any windows computers around :) Cheers

@thunders82 thunders82 closed this Jul 27, 2019
@thunders82
Copy link
Author

re-opening for testing

@thunders82 thunders82 reopened this Jul 27, 2019
@thunders82
Copy link
Author

Alright. It looks like everything is compiling properly on win32 now. Sorry again for the noise...

geoffrey added 2 commits August 3, 2019 17:48
…and 8bit (log) data. Works as expected with current openMVS code but limited to 8bit output due to current implementation.

pImage->ReadData(image.data, PF_R8G8B8, 3, (CImage::Size)image.step)
@thunders82
Copy link
Author

thunders82 commented Aug 3, 2019

The 2 last commits gives the ability to read 32bit EXR files and load the data as float 32bit (linear) or 8bit (log) output.

It's seems it doesn't compile for windows as the openEXR library can't be found. I will need the help of someone to fix it as I don't have a windows computer.

…g TextureMesh (hasn't been tested for anything else than TextureMesh but could work )

-changing default output to float 32bit EXR file format.
@thunders82
Copy link
Author

thunders82 commented Aug 4, 2019

Last commit is quick conversion to float/32F. TextureMesh has been tested and seems to work as expected.

It now outputs 32bit EXR instead of jpg or png

@cdcseacave
Copy link
Owner

Sry for the late reply!

This changes look very professional, it does not look like you are only starting to learn programming.

Here are some suggestions:

I like a lot the idea of using Image32F3 instead of Image8U3 in order to support all types of inputs. It is very simple and should just work. The only downside is that this will increase enven farther the memory requirements. It would be best if we can use a dynamic image type, that stores pixels in uint8_t or float depending on the input image. Or at least use a typedef that can easily be changed at compile time (so instead of manually replacing all instances of Image8U3 to Image32F3, to make a typedef Image32F3 InputImage that can easy be changed back to Image8U3 and use InputImage everywhere.

To make your life easier on Windows, use VCPKG to install and compile 3rd party libs. Not only that will save you time to manage all those libs, but should make it easy also to find them using CMake in a consistent way (no need for custom FindPackage).

This PR grew very large, and targets two different problems. Pls split it in 2 PR, one for each problem.

Thank you for the HARD work.

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