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

Doubt about average calculation in SpecularMap #33

Open
azagaya opened this issue Feb 17, 2019 · 6 comments
Open

Doubt about average calculation in SpecularMap #33

azagaya opened this issue Feb 17, 2019 · 6 comments

Comments

@azagaya
Copy link
Contributor

azagaya commented Feb 17, 2019

Hi,
I was looking at the code, and i saw that in specular map, the average calculation was like this:

   double multiplierSum = (redMultiplier + greenMultiplier + blueMultiplier  + alphaMultiplier );
    if(multiplierSum == 0.0)
        multiplierSum = 1.0;
//some other code
    if(mode == IntensityMap::AVERAGE) {
       //take the average out of all selected channels
        intensity = (r + g + b + a) / multiplierSum;
    }

But imho this is not an average, because if every channel has a multiplier of let say 0.1, then the sum is going to be 0.4, and you are going to divide the whole sum by 0.4. Shouldn't the division be the sum of each channel divided by the ammount of channels used?
I really don't know how specular maps are calculated, so it's just a question.. i can fix it if it is a bug.

@azagaya
Copy link
Contributor Author

azagaya commented Feb 17, 2019

Hi, i've created an AppImage with some changes that fixes what i commented before and #11
You can find it in my fork releases... if you think those changes are ok, i can make a pull request.

@Theverat
Copy link
Owner

Have you already committed the fix for the specular map? I can't see it in your fork.

@azagaya
Copy link
Contributor Author

azagaya commented Feb 23, 2019

Oh, I think I forgot.. I'll do it when I reach home!

@azagaya
Copy link
Contributor Author

azagaya commented Feb 24, 2019

Hi,
I've commited the changes to my fork now. You can find them in the "fixes" branch.

@Theverat
Copy link
Owner

I had to comment out the "include cv.hpp" to compile, it doesn't seem to be used. Leftover of a test?
Otherwise it looks fine to me, thanks for your work.

I also tried to test your appimage, but it doesn't work on my ancient Ubuntu 14.04 - however I guess this is expected, because it's a very old distro.
It throws this error message:

./Normalmap_Generator-x86_64.AppImage: symbol lookup error: /tmp/.mount_NormalMOS1dK/usr/plugins/platforms/../../lib/libQt5XcbQpa.so.5: undefined symbol: FT_Get_Font_Format

@azagaya
Copy link
Contributor Author

azagaya commented Feb 24, 2019

Hi,
Yes, the opencv include was for a test.. some time ago I was working in my own normal map generator using opencv.
The appimage won't work in trusty because I made it in xenial. I know I'm supposed to make it on trusty, but as trusty is going to be deprecated in April this year, I made it on xenial.

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