Page MenuHome

Fix T48044: Smoke rendered color differs from color in viewport
Needs ReviewPublic

Authored by Sergey Sharybin (sergey) on Apr 5 2016, 4:48 PM.

Details

Summary

There are couple of things were wrong about render and viewport draw:

  • Smoke color was stored in sRGB space, but render engines were treating it as linear one, which was causing washed out colors comparing to OpenGL viewport.
  • Cycles and OpenGL viewport were straight alpha for the smoke color, but it was provided as premultiplied one.

    Interestingly, BI was doing un-premultiplication so it was kind of known of how alpha is stored in volumes.

    This was causing darker outlines around volume in Cycles on OpenGL.

This commit switches Blender to store colors in scene linear space and makes it so smoke is always providing colors with straight alpha so no un-premultiplication required.

Color space changes will only affect new smoke caches, existing ones should be rendering with same color as they used to. Alpha changes will affect both new and old smoke caches.

Diff Detail

Repository
rB Blender
Branch
smoke_color

Event Timeline

Sergey Sharybin (sergey) retitled this revision from to Fix T48044: Smoke rendered color differs from color in viewport.
Sergey Sharybin (sergey) updated this object.

Alternative approach would be to only keep compatibility of baked simulations and switch Blender to use linear color for smoke/file.

Here's patch which implements this (fire needs more verificaiton / work tho):

1diff --git a/source/blender/blenkernel/BKE_blender.h b/source/blender/blenkernel/BKE_blender.h
2index fdb3474..3eb1c30 100644
3--- a/source/blender/blenkernel/BKE_blender.h
4+++ b/source/blender/blenkernel/BKE_blender.h
5@@ -42,7 +42,7 @@ extern "C" {
6 * and keep comment above the defines.
7 * Use STRINGIFY() rather than defining with quotes */
8 #define BLENDER_VERSION 277
9-#define BLENDER_SUBVERSION 0
10+#define BLENDER_SUBVERSION 1
11 /* Several breakages with 270, e.g. constraint deg vs rad */
12 #define BLENDER_MINVERSION 270
13 #define BLENDER_MINSUBVERSION 6
14diff --git a/source/blender/blenloader/intern/versioning_270.c b/source/blender/blenloader/intern/versioning_270.c
15index b9191d5..6cd636f 100644
16--- a/source/blender/blenloader/intern/versioning_270.c
17+++ b/source/blender/blenloader/intern/versioning_270.c
18@@ -50,6 +50,7 @@
19 #include "DNA_particle_types.h"
20 #include "DNA_linestyle_types.h"
21 #include "DNA_actuator_types.h"
22+#include "DNA_smoke_types.h"
23 #include "DNA_view3d_types.h"
24
25 #include "DNA_genfile.h"
26@@ -1072,5 +1073,25 @@ void blo_do_versions_270(FileData *fd, Library *UNUSED(lib), Main *main)
27 }
28 }
29 }
30+
31+ for (Object *object = main->object.first;
32+ object != NULL;
33+ object = object->id.next)
34+ {
35+ for (ModifierData *md = object->modifiers.first;
36+ md != NULL;
37+ md = md->next)
38+ {
39+ if (md->type == eModifierType_Smoke) {
40+ SmokeModifierData *smd = (SmokeModifierData *)md;
41+ if ((smd->type & MOD_SMOKE_TYPE_DOMAIN) && smd->domain != NULL) {
42+ srgb_to_linearrgb_v3_v3(smd->domain->flame_smoke_color, smd->domain->flame_smoke_color);
43+ }
44+ else if ((smd->type & MOD_SMOKE_TYPE_FLOW) && smd->flow != NULL) {
45+ srgb_to_linearrgb_v3_v3(smd->flow->color, smd->flow->color);
46+ }
47+ }
48+ }
49+ }
50 }
51 }
52diff --git a/source/blender/gpu/shaders/gpu_shader_smoke_frag.glsl b/source/blender/gpu/shaders/gpu_shader_smoke_frag.glsl
53index 4d1feb5..724dc43 100644
54--- a/source/blender/gpu/shaders/gpu_shader_smoke_frag.glsl
55+++ b/source/blender/gpu/shaders/gpu_shader_smoke_frag.glsl
56@@ -13,6 +13,21 @@ uniform sampler3D flame_texture;
57 uniform sampler1D spectrum_texture;
58 #endif
59
60+float linearrgb_to_srgb(float c)
61+{
62+ if(c < 0.0031308)
63+ return (c < 0.0) ? 0.0: c * 12.92;
64+ else
65+ return 1.055 * pow(c, 1.0/2.4) - 0.055;
66+}
67+
68+void linearrgb_to_srgb(vec3 col_from, out vec3 col_to)
69+{
70+ col_to.r = linearrgb_to_srgb(col_from.r);
71+ col_to.g = linearrgb_to_srgb(col_from.g);
72+ col_to.b = linearrgb_to_srgb(col_from.b);
73+}
74+
75 void main()
76 {
77 /* compute color and density from volume texture */
78@@ -24,6 +39,7 @@ void main()
79 else {
80 soot_color = vec3(0, 0, 0);
81 }
82+ linearrgb_to_srgb(soot_color, soot_color);
83 float soot_density = density_scale * soot.a;
84
85 /* compute transmittance and alpha */
86@@ -40,6 +56,7 @@ void main()
87 #ifdef USE_FIRE
88 /* fire */
89 float flame = texture3D(flame_texture, coords).r;
90+ flame = linearrgb_to_srgb(flame);
91 vec4 emission = texture1D(spectrum_texture, flame);
92 color.rgb += (1 - color.a) * emission.a * emission.rgb;
93 #endif
94diff --git a/source/blender/makesrna/intern/rna_smoke.c b/source/blender/makesrna/intern/rna_smoke.c
95index ba3198a..3027846 100644
96--- a/source/blender/makesrna/intern/rna_smoke.c
97+++ b/source/blender/makesrna/intern/rna_smoke.c
98@@ -607,7 +607,7 @@ static void rna_def_smoke_domain_settings(BlenderRNA *brna)
99 RNA_def_property_ui_text(prop, "Maximum", "Maximum temperature of flames");
100 RNA_def_property_update(prop, NC_OBJECT | ND_MODIFIER, "rna_Smoke_resetCache");
101
102- prop = RNA_def_property(srna, "flame_smoke_color", PROP_FLOAT, PROP_COLOR_GAMMA);
103+ prop = RNA_def_property(srna, "flame_smoke_color", PROP_FLOAT, PROP_COLOR);
104 RNA_def_property_array(prop, 3);
105 RNA_def_property_ui_text(prop, "Smoke Color", "Color of smoke emitted from burning fuel");
106 RNA_def_property_update(prop, NC_OBJECT | ND_MODIFIER, "rna_Smoke_resetCache");
107@@ -683,7 +683,7 @@ static void rna_def_smoke_flow_settings(BlenderRNA *brna)
108 RNA_def_property_ui_text(prop, "Density", "");
109 RNA_def_property_update(prop, NC_OBJECT | ND_MODIFIER, "rna_Smoke_reset");
110
111- prop = RNA_def_property(srna, "smoke_color", PROP_FLOAT, PROP_COLOR_GAMMA);
112+ prop = RNA_def_property(srna, "smoke_color", PROP_FLOAT, PROP_COLOR);
113 RNA_def_property_float_sdna(prop, NULL, "color");
114 RNA_def_property_array(prop, 3);
115 RNA_def_property_ui_text(prop, "Smoke Color", "Color of smoke");

Not sure what's least crap here. Ideas?

I would switch Blender to consider smoke colors linear, but do no version patching. That way Cycles and Blender Internal renders keep backwards compatibility.

For the viewport smoke display you could then consider adding a compatibility option, enabled on existing smoke domains. I'm not sure if viewport display backwards compatibility is really a thing that we have ever worried about though.

Sergey Sharybin (sergey) edited edge metadata.

Store colors in linear space and solve alpha premul issues.

source/blender/blenkernel/intern/smoke.c
525

Default "regular" smoke color would also need to be adjusted so it's results matches old behavior (basically, just linearize default value).

Any updates on this? I could not test the patch as I have yet to find a way to build Blender with Cycles :(

The biggest issue here is the compatibility thing. But guess we should just commit this to 2.8 branch and add it to the buildbot.

Since 2.8 is on it's way how about committing this to the 2.8 branch? I would be very pleased to have smoke colors provided linear in Cycles.

Smoke colors are already linear in Cycles and Eevee.

As far as I can tell the only thing to be done here still is changing PROP_COLOR_GAMMA to PROP_COLOR, so that the little color button previews look correct.

Premultiplication was addressed in rBacd619d7c925: Cycles: change smoke color grid to not include density multiplied in. which I think is better than straight alpha for the same reason that we need premultiplied alpha for 2D image filtering.