Page MenuHome
Differential D154

BGE: World color management fix
ClosedPublic
Actions

Authored by Thomas Szepe (hg1) on Dec 29 2013, 4:20 PM.

Details

Reviewers
Brecht Van Lommel (brecht)
Mitchell Stokes (moguri)
Commits
rBSce40fb9ab261: BGE: World color management fix
rBce40fb9ab261: BGE: World color management fix
Summary

This patch will fix the color management for the mist and global ambient color.
It will remove the old "Color Management" switch in the BGE "Render > Shading" panel and will use the "Display Device" setting in the "Scene > Color Management" panel instead.

Diff Detail

Event Timeline

Brecht Van Lommel (brecht) requested changes to this revision.Dec 31 2013, 1:05 AM

Looks ok except for the version patch.

source/blender/blenloader/intern/versioning_260.c
2649–2670 ↗(On Diff #456)

Can you do this only when the render engine is set to BLENDER_GAME? I would like to avoid breaking files that are not intended for the game engine.

2663–2665 ↗(On Diff #456)

I don't think this is needed.

2666–2668 ↗(On Diff #456)

Is this necessary? As I understand it, with color management images are converted from sRGB to linear, and then it gets converted back to sRGB at the end of shading. So disabling color management should already cancel out both, and setting the image color space to raw would in fact change the result.

In any case it seems a bit scary to change all image color spaces based on this setting.

Thomas Szepe (hg1) added a comment.Dec 31 2013, 12:13 PM

From my side I think the version patch in not necessary.
But I want to decide this by my own. Let's hear what moguri thinks.

source/blender/blenloader/intern/versioning_260.c
2649–2670 ↗(On Diff #456)

I am not sure how to get this value, I don't found any similar code. But I think I will find out a way to do this.

But I think it is not necessary because the old option was only visible if BLENDER_GAME was chosen.

2663–2665 ↗(On Diff #456)

I am also unsure if we should convert the old GAME_GLSL_NO_COLOR_MANAGEMENT setting. The user can do this in the display settings now.
I only have done this because I think a lot of people's will not know that the setting now can be found under display device.

2666–2668 ↗(On Diff #456)

I don't know if that is necessary, but I think so. I copied the code from line 1529-1568.

Original comment:
/* if colormanagement not used, set image's color space to raw, so no sRGB->linear conversion

  • would happen on display and render
  • there's no clear way to check whether color management is enabled or not in render engine
  • so set all images to raw if there's at least one scene with color management disabled
  • this would still behave incorrect in cases when color management was used for only some
  • of scenes, but such a setup is crazy anyway and think it's fair enough to break compatibility
  • in that cases
Thomas Szepe (hg1) updated this revision to Unknown Object (????).Jan 8 2014, 11:24 PM

I removed the converter for the old GAME_GLSL_NO_COLOR_MANAGEMENT setting.
I think it is not necessary.

Brecht Van Lommel (brecht) accepted this revision.Jan 9 2014, 6:01 PM

I guess that's reasonable but I'll leave the final decision for that on game engine developers. It's easy to solve the backwards compatibility problem for users when they know, but it breaks compatibility of course.

Thomas Szepe (hg1) updated this revision to Unknown Object (????).Jan 11 2014, 11:11 AM

Update because the mist render in GLSL solid mode (= shadeless Multitexture) was displaying wrong.

Thomas Szepe (hg1) updated this revision to Unknown Object (????).Jul 19 2014, 7:22 PM

Update for the actual master.

Thomas Szepe (hg1) mentioned this in D151: BGE: Fix T43592: World GLSL.Mar 4 2015, 7:05 PM
Closed by commit rBce40fb9ab261: BGE: World color management fix (authored by Thomas Szepe (hg1)). · Explain WhyMar 23 2015, 11:58 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
release/
scripts/
startup/
bl_ui/
1 line
source/
blender/
gpu/
intern/
2 lines
makesrna/
intern/
1 line
gameengine/
Converter/
2 lines
4 lines
40 lines

Diff 2155

View Options

release/scripts/startup/bl_ui/properties_game.py

Loading...
View Options

source/blender/gpu/intern/gpu_material.c

Loading...
View Options

source/blender/makesrna/intern/rna_color.c

Loading...
View Options

source/gameengine/Converter/BL_BlenderDataConversion.cpp

Loading...
View Options

source/gameengine/Converter/BlenderWorldInfo.h

Loading...
View Options

source/gameengine/Converter/BlenderWorldInfo.cpp

Loading...