Theverat / NormalmapGenerator

A simple program that converts images into normal maps
GNU General Public License v3.0
416 stars 51 forks source link

Doubt about average calculation in SpecularMap #33

Open azagaya opened 5 years ago

azagaya commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

azagaya commented 5 years ago

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

azagaya commented 5 years ago

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

Theverat commented 5 years ago

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 commented 5 years ago

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.