vdbfusion icon indicating copy to clipboard operation
vdbfusion copied to clipboard

Color support

Open paucarre opened this issue 2 years ago • 13 comments

Adding support for colors mostly with work from @swarmt but I added the last piece which is marching cubes support.

paucarre avatar Jun 24 '22 08:06 paucarre

This is an example from Lady and Cow: Image: https://drive.google.com/file/d/1LF35dqW4x5z_Fu2rQ-UFAJuwukVjfsPn/view?usp=sharing Mesh: https://drive.google.com/file/d/1OiCMfb4HFpLIdbGzFEW7ZGTqLMBxbomL/view?usp=sharing

paucarre avatar Jun 24 '22 08:06 paucarre

This PR is related to https://github.com/PRBonn/vdbfusion_ros/pull/1

paucarre avatar Jun 24 '22 08:06 paucarre

Wow! @paucarre thanks a lot for opening the PR!

I will allocate some time to deep-analize the PR before merging, but over the surface looks quite good :)

nachovizzo avatar Jun 24 '22 09:06 nachovizzo

@paucarre I started to review this, is there any chance you run clang-format on your end? Seems like the format has changed a lot, and makes the reviewing a bit challenging.

You can run this command on the root of the project and that would do the magic:

 clang-format -i $(find . -regextype posix-extended -regex  ".*\.(cpp|cxx|cc|hpp|hxx|h)" | grep -vE "^./(build|3rdparty)/")

nachovizzo avatar Jun 27 '22 12:06 nachovizzo

@nachovizzo I will update the PR this (late) evening

paucarre avatar Jun 28 '22 08:06 paucarre

@nachovizzo updated. Note that I forced C++17 as it seems it no longer builds unless it uses C++14. I think C++17 has advantages so if you force a a C++ maybe leave at least C++17

paucarre avatar Jun 28 '22 21:06 paucarre

@paucarre thanks for the PR, I tested the code and it works fine. The only thing is that you did not multiply the color values by 255 at the end. Not sure if you did this intentionally.

melhashash avatar Jul 13 '22 16:07 melhashash

@melhashash Thanks for giving this a try. I will review this next week and merge it, so it's also accessible through the python packages

nachovizzo avatar Jul 14 '22 13:07 nachovizzo

@melhashash Thanks for giving this a try. I will review this next week and merge it, so it's also accessible through the python packages

Can you please review and merge it?

vnmsklnk avatar Sep 02 '22 14:09 vnmsklnk

Hello @vnmsklnk @paucarre . I'm honestly sorry for the delay. I currently had no time for reviewing this.... and sadly no one else can do it for me :(....

So this will have to wait in the meantime. You can always use the @paucarre and build from source in case you need it ;_)

nachovizzo avatar Sep 02 '22 14:09 nachovizzo

@paucarre ,Hi, can you give me some advice on how to download the cow datasets?

debuleilei avatar Sep 28 '22 09:09 debuleilei

Excellent, thanks for this. Thought I may need to implement it myself.

mpottinger avatar Sep 28 '22 22:09 mpottinger

@paucarre I'd fork from your repo and add my changes on top if you don't mind. One of the reasons is that the entire CI infrastructure for this project is not running on Github but in our private Gitlab servers, therefore I need to make sure all checks and tests are passing

nachovizzo avatar Oct 04 '22 08:10 nachovizzo

Will merge this in #33 . Thanks @paucarre

nachovizzo avatar Feb 20 '23 10:02 nachovizzo