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

Tone Mapping parameter missing for mvs_texturing #492

Closed
pierotofy opened this issue Feb 21, 2017 · 6 comments
Closed

Tone Mapping parameter missing for mvs_texturing #492

pierotofy opened this issue Feb 21, 2017 · 6 comments
Assignees
Milestone

Comments

@pierotofy
Copy link
Member

While digging through mvs_texturing's code, I noticed that we currently don't have a way to enable the tone mapping feature, here:

https://github.com/nmoehrle/mvs-texturing/blob/master/apps/texrecon/arguments.cpp#L69

Which I think is directly related to the "bland" colors you sometimes observe as output on the orthophotos:
image

Notice the white areas near trees.

This seems directly related to the tone mapping feature, which is disabled by default: nmoehrle/mvs-texturing#24

@dakotabenjamin dakotabenjamin self-assigned this Feb 21, 2017
@dakotabenjamin
Copy link
Member

I will see if exposing this parameter will help.

@dakotabenjamin
Copy link
Member

https://github.com/dakotabenjamin/OpenDroneMap/tree/tone-mapping-param

Note that this doesn't include an update to MVS-Texturing. You will have to pull #471 changes in or update manually.

I don't have a dataset on hand to find any tangible differences between 'gamma' and 'none' so I'm hoping you can test that. Also, I was unsure whether to use the mvs-tex args (gamma and none) or just a boolean flag, but can easily switch if you think one is preferable over the other.

@pierotofy
Copy link
Member Author

Thank you @dakotabenjamin! I will do some testing and report my results.

@pierotofy
Copy link
Member Author

Mm, no significant improvements over the faded trees. This is what I got out with the latest mvs texturing and tone mapping set to gamma:

image

I think the tone mapping option should be included, but perhaps have it set by default to none. Leaving it as a string is fine by me.

@pierotofy
Copy link
Member Author

Settings tone mapping to none with the latest mvs texturing resulted in much nicer results:

image

So while just upgrading to the latest mvs texturing code will fix the problem, it might be beneficial for some users to expose the tone mapping option. But better to leave the default to none.

@pierotofy
Copy link
Member Author

I'll close this since it has been taken care of as part of #499

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

No branches or pull requests

2 participants