collada-exporter icon indicating copy to clipboard operation
collada-exporter copied to clipboard

Exporting wrong colors

Open set-killer opened this issue 8 years ago • 11 comments

As you might know currently BCE exports wrong colors. Instead of real RGBA color it exports only RGB color with "precalculated" alpha channel. This is wrong. The reason for the current hacky implementation is that Godot does not support alpha channel for some of the colors. I assume this will be improved in the near future. Also the current implementation does not work vary well when you change the ambient light or any other light in the environment.

By definition the collada format requires 4 float values for some of the colors (page 375).

This patch fixes the alpha channel for the colors. Removes the function numarr because it is not used anywhere. Renames strarr to strfloat3 and numarr_alpha to strfloat4.

I've taken some samples: samples

1 - The object rendered into blender 2 - The current situation with BCE 3 - Exported with this patch - Godot does not support alpha channel for the emission and specular colors (does only the diffuse color supports alpha?) 4 - Exported with this patch - changed the emission color to black

The reason for the difference between blender and godot is that blender applies gamma correction of 2.2 (whatever that means).

I guess it is not practical to merge this patch right now because the godot users should remove the emission color after they import a collada file. Nobody uses material colors anyway. I assume it is a known problem. Also if you don't plan to merge it, it is not rude to close this PR.

set-killer avatar Sep 12 '16 21:09 set-killer

Technically your patch seems to be right - what was the original purpose of multiplying the color by mult, I wonder? Of course using the right colors and the alpha channel too is a better solution.

Godot does not support alpha channel for the emission and specular colors (does only the diffuse color supports alpha?)

Isn't this a Godot bug? Even if nobody uses material colors, it's Godot that isn't playing by the rules, right?

ghost avatar Sep 29 '16 09:09 ghost

Yes, its a Godot problem. I hope that version 3 will fix the missing alpha channel for some of the colours.

To determine the problem more clearly try to add environmental light into the godot's scene. Then a totally different colour appears.

The reason for the current implementation is that originally Godot supports only RGB colours, and the RGBA support for the diffuse colour was introduced in version 2.0 (as far as I remember).

Another thing we need to discuss is: Should we apply that gamma correction when exporting the colours? In that way there wouldn't be any differences between that what you see in Blender and what you export.

set-killer avatar Sep 29 '16 11:09 set-killer

CC'ing @reduz who's working on the new renderer right now and would know what can and should be supported.

akien-mga avatar Sep 29 '16 11:09 akien-mga

Another thing we need to discuss is: Should we apply that gamma correction when exporting the colours? In that way there wouldn't be any differences between that what you see in Blender and what you export.

Turns out the gamma correction of 2.2 is actually standardized in sRGB. So, in short, it would not be wrong to apply gamma correction (so that Blender users won't accuse Godot of getting the colors wrong), but I'd put a checkbox to allow disabling it just in case some advanced user wants to.

ghost avatar Sep 29 '16 13:09 ghost

Okey, i've added the Apply Gamma Correction to the exporting options. So currently the result is the following:

gamma correction

1 - The object rendered in Blender 2 - The object imported in Godot with gamma correction 3 - The object imported in Godot with gamma correction and fixed specular and emission colors. 4 - The object imported in Godot without gamma correction and fixed specular and emission colors.

When we compare the hex colors from blender (which is gamma corrected already) and the hex color in Godot we still see difference:

  • #A5E7DB in Blender
  • #A3E6DA in Godot

I guess this difference comes from the rounding of the floats. Anyway, it is relatively small difference.

With the second patch I move the functions strfloat3 and strfloat4 inside the class so we can access the self.config variable. Adding new function apply_gamma which calculates the gamma correction. The formula for the gamma correction is something like:

gamma = 2.2
for C in [R, G, B]:
  C = pow(C, 1/gamma)

The alpha channel does not receive gamma correction 😃

set-killer avatar Oct 01 '16 13:10 set-killer

@set-killer excellent work! Please explain one thing, when Godot users export the model from Blender using the default setting (gamma correction on) and load it straight into Godot do they see #2 or #3?

ghost avatar Oct 01 '16 18:10 ghost

@paper-pauper Currently they see #3 from the first pic or #2 from the second image because the emission color in Godot does not support alpha blending. I hope the things will change with Godot 3.0

set-killer avatar Oct 01 '16 20:10 set-killer

I think your patch is good. There could be an improvement to match the colors with more precision, however as you noted it probably depends on the rounding (or some other precision issue) and I think nobody expects you to fix it (but if you do, props for your coding skillz :sunglasses:).

On the other hand, here we have a dilemma:

  1. This plugin is aimed at all Collada users, not just Godot users (the plan is to get it into upstream Blender), so it wouldn't be fair to adapt it to Godot's shortcomings
  2. If your PR is merged, people will complain about the plugin

In short, perhaps right now (until the issue is fixed) the best solution is to add yet another checkbox to "precalculate" the alpha channel (using the same formula as before), and leave it enabled for the time being. This way is fair to Godot users, to Blender users and to Collada users, I think. What do you think about it?

ghost avatar Oct 01 '16 20:10 ghost

I think that would not be necessary.

Even if we submit this addon for merging into the mainline Blender now, i guess they will publish it with the version 2.80 which is not coming soon. This means that we have to distribute Godot 2.2 with BCE.

In this case we should merge this code and then submit it for merging into the Blenders repository. After that we should apply a patch into our repository which changes the algorithm in the strfloat4 function, and then we can distribute Godot 2.2 with BCE.

If nobody volunteers to submit this addon for merging into Blenders mainline code, we may ship Godot 2.2 without this patch.

(* by we i mean reduz, akien-mga and the other guys who are organizing the releases. not me)

set-killer avatar Oct 03 '16 09:10 set-killer

Now if we only had someone for #31...a whole month was wasted because nobody wants to do it. There are so many improvements which could benefit both BCE and Godot, such as this one.

This post says that Blender 2.8 will replace their Collada exporter with a Python one. Is it true? If so, that increases the chance they will merge BCE or at least use some code from it (unless their exporter is significantly better).

ghost avatar Oct 03 '16 10:10 ghost

Oh, that's bad. It feels like our work is wasted... We have missed the Blender's 2.78 milestone. Don't know if there are gonna be 2.78a and 2.78b releases, but it looks like our best chance is 2.79 (if happen at all) :disappointed:

set-killer avatar Oct 03 '16 12:10 set-killer