Fix T48044: Smoke rendered color differs from color in viewport
AbandonedPublic
Actions

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

Details

Reviewers

Bastien Montagne (mont29)
Brecht Van Lommel (brecht)

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_srgb

Event Timeline

Sergey Sharybin (sergey) updated this revision to Diff 6360.Apr 5 2016, 4:48 PM

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

Sergey Sharybin (sergey) added reviewers: Campbell Barton (campbellbarton), Brecht Van Lommel (brecht), Bastien Montagne (mont29).

Sergey Sharybin (sergey) updated this object.

Sergey Sharybin (sergey) added a subscriber: Gottfried Hofmann (gottfried).

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):

P346 Snippet for D1897

1	diff --git a/source/blender/blenkernel/BKE_blender.h b/source/blender/blenkernel/BKE_blender.h
2	index 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
14	diff --git a/source/blender/blenloader/intern/versioning_270.c b/source/blender/blenloader/intern/versioning_270.c
15	index 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	}
52	diff --git a/source/blender/gpu/shaders/gpu_shader_smoke_frag.glsl b/source/blender/gpu/shaders/gpu_shader_smoke_frag.glsl
53	index 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
94	diff --git a/source/blender/makesrna/intern/rna_smoke.c b/source/blender/makesrna/intern/rna_smoke.c
95	index 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.

Store colors in linear space and solve alpha premul issues.

Sergey Sharybin (sergey) updated this object.Apr 6 2016, 2:14 PM

Sergey Sharybin (sergey) added inline comments.Apr 6 2016, 2:21 PM

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

Sergey Sharybin (sergey) mentioned this in T48044: Smoke Colour from flow source in Cycles differs from colour set and viewport colour..Apr 12 2016, 1:32 PM

Campbell Barton (campbellbarton) removed a reviewer: Campbell Barton (campbellbarton).Aug 11 2016, 6:06 AM

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.