Details

Reviewers

Brecht Van Lommel (brecht)
Mike Erwin (merwin)
Porteries Tristan (panzergame)
Thomas Szepe (hg1)
Ulysse Martin (youle)
Daniel Stokes (kupoman)
Mitchell Stokes (moguri)

Summary

*Cubemap Reflections in the Viewport and in the Game Engine*

This patch adds cubemap reflections in the viewport (works also with the game engine) using existing UI.

Cubemap Reflections in images

Standard: http://www.pasteall.org/pic/show.php?id=98705
Reflection deformed by normal map: http://www.pasteall.org/pic/show.php?id=98706

The Cubemap format

http://www.pasteall.org/pic/show.php?id=98707
The cubemap has to be created in Blender: https://en.wikibooks.org/wiki/Blender_3D:_Noob_to_Pro/Build_a_skybox
Some of these textures can be found on the net.

The Options available

The cubemap behaves like other textures, except that texture coordinates are computed in the cubemap shader, and geometry options are not available, but you can easily mix a cubemap texture with other types of textures (normal maps (the reflection is deformed by normal map when the normal map is in a slot < cubemap texture slot), and you can have cool results playing with BlendTypes. The options in the material tab are of course also available. And the cubemap normally interacts with the scene lights.

The shader

https://en.wikibooks.org/wiki/GLSL_Programming/Blender/Reflecting_Surfaces

Diff Detail

Repository: rB Blender
Branch: arcpatch-D1756

Event Timeline

Ulysse Martin (youle) updated this revision to Diff 5938.Feb 2 2016, 2:01 PM

Ulysse Martin (youle) retitled this revision from to Cubemap reflections in the viewport and in the game engine.

Ulysse Martin (youle) updated this object.

Ulysse Martin (youle) added reviewers: Daniel Stokes (kupoman), Thomas Szepe (hg1), Mitchell Stokes (moguri), Brecht Van Lommel (brecht), Porteries Tristan (panzergame).

Ulysse Martin (youle) added subscribers: Porteries Tristan (panzergame), Thomas Szepe (hg1), Daniel Stokes (kupoman).

added GPU_link(mat, "srgb_to_linearrgb", tcol, &tcol); for cubemaps.
Test file: http://www.pasteall.org/blend/40446

EDIT: I'll remove mtex_normal_cubemap in gpu_shader_material.glsl

I replaced GPU_builtin(GPU_VIEW_POSITION) and GPU_builtin(GPU_VIEW_NORMAL) (uniform inputs for the cubemap shader mtex_cube_map_refl) with shi->view and shi->vn.

This way, the normal map texture can modify reflections IF the normal map texture is in a texture slot < cubemap texture slot.

if the normal modifies reflection: http://www.pasteall.org/pic/show.php?id=98678
else: http://www.pasteall.org/pic/show.php?id=98679

test file: http://www.pasteall.org/blend/40449

I propose to let the user choose if he wants the normal map texture to modify reflections or not (even if it seems not realistic).

@Brecht Van Lommel (brecht): What's your opinion on the subject?

I think implement the other shaders for cubemap reflections is not needed (no need for example to have a shader for cubemap normal (if tex->imaflag & TEX_NORMALMAP))... But maybe another shader would be nice to have? I don't know...

I'm not sure the #define TEX_CUBEMAP_REFL 1 in DNA_texture_types.h is at the right place...

Else I'm happy about the way it works :) Thanks to @Daniel Stokes (kupoman) for the help

EDIT: https://youtu.be/omhFKDO_vFw

Ulysse Martin (youle) updated this object.Feb 3 2016, 1:15 PM

Porteries Tristan (panzergame) added inline comments.Feb 3 2016, 1:27 PM

source/blender/gpu/shaders/gpu_shader_material.glsl
1275	the first vec4 is not redundant ? Does this will work ? : vec3 viewdirection = vec3(viewmatrixinverse * vec4(vp, 0.0));
1279	What the use of this value if it's constant ?
source/blender/makesdna/DNA_texture_types.h
137	Why use a new flag for one option ? There's an existing flag ? Does Tex->env->reflag contains something else ? May it's for futur features ?
source/blender/makesrna/intern/rna_texture.c
837	Viewport and BGE are the only way to use cubmap, no ? If yes just said `Enable Cubemap Reflections`

Updated summary according to Kaito advices.
Done part of @Porteries Tristan (panzergame) inline comments.

Ulysse Martin (youle) marked 2 inline comments as done.Feb 3 2016, 1:56 PM

Ulysse Martin (youle) added inline comments.

source/blender/gpu/shaders/gpu_shader_material.glsl
1275	Yes you're right. I change that.
1279	value is a normal output for the shader as for mtex_image. It corresponds to &trgb and &trgb can be modified by other shaders. It's initialized in this shader.
source/blender/makesdna/DNA_texture_types.h
137	It's the only flag in EnvMap struct. I think it's better to have a flag in EnvMap struct, cleaner. But I may be wrong.
source/blender/makesrna/intern/rna_texture.c
837	ok

Porteries Tristan (panzergame) added inline comments.Feb 3 2016, 2:12 PM

source/blender/gpu/intern/gpu_material.c
1226	tex->type can be something else than TEX_ENVMAP if it's not TEX_IMAGE ? if yes do : else if (tex->type = TEX_ENVMAP {

Porteries Tristan (panzergame) accepted this revision.Feb 3 2016, 2:18 PM

Porteries Tristan (panzergame) edited edge metadata.

Porteries Tristan (panzergame) added inline comments.

source/blender/gpu/intern/gpu_material.c
1226	i'm wrong the condition above do it.

This revision is now accepted and ready to land.Feb 3 2016, 2:18 PM

Campbell Barton (campbellbarton) requested changes to this revision.Feb 3 2016, 2:54 PM

Campbell Barton (campbellbarton) added a reviewer: Campbell Barton (campbellbarton).

Campbell Barton (campbellbarton) added a subscriber: Campbell Barton (campbellbarton).

Campbell Barton (campbellbarton) added inline comments.

source/blender/gpu/intern/gpu_material.c
1222	Would format this so its easier to follow logic for both cases. if ((tex && tex->ima) && ((tex->type == TEX_IMAGE) \|\| (tex->type == TEX_ENVMAP && tex->env->reflag & TEX_CUBEMAP_REFL))) {
source/blender/makesdna/DNA_texture_types.h
137	Would just call this `flag`, can make this and `ok` a short and avoid adding the pad.
396	Since this is for envmap shouldn't it be called `TEX_ENVMAP_REFL`
source/blender/makesrna/intern/rna_texture.c
835	naming convention for rna is to have `use_` or `show_` prefix for booleans. in this case `use_envmap_reflection`

This revision now requires changes to proceed.Feb 3 2016, 2:54 PM

@Campbell Barton (campbellbarton) inline comments done.

Thanks everyone for review :)

Alexander Romanov (a.romanov) edited edge metadata.Feb 3 2016, 3:57 PM

Alexander Romanov (a.romanov) added a subscriber: Alexander Romanov (a.romanov).

Yuri Kovelenov (yurikovelenov) added a subscriber: Yuri Kovelenov (yurikovelenov).Feb 3 2016, 4:03 PM

Ulysse Martin (youle) added a reviewer: Mike Erwin (merwin).Feb 3 2016, 4:10 PM

Campbell Barton (campbellbarton) commandeered this revision.Feb 3 2016, 5:06 PM

Campbell Barton (campbellbarton) edited reviewers, added: Ulysse Martin (youle); removed: Campbell Barton (campbellbarton).

This revision is now accepted and ready to land.Feb 3 2016, 5:06 PM

So let's go for Cubemap reflections in the viewport! Thanks @Campbell Barton (campbellbarton) :)

(I don't know how to commit hahaha)

Sorry I didn't know what meant commandeer... Thanks for the last corrections Campbell Barton. I accepted my own revision. Shame on me.

Dont make lastframe a short and minor edits

Daniele De Luca (sparazza) added a subscriber: Daniele De Luca (sparazza).Feb 3 2016, 6:26 PM

Was the texture order issue sorted out? Also, I added some inline comments about some things that looked a little strange in the UI and shader to me.

release/scripts/startup/bl_ui/properties_texture.py
622	What render engines does this UI show up for? Also, this UI seems a little strange to me. Shouldn't enabling reflections be a part of the material settings? Is there any way to control how the reflections mix with the material color?
source/blender/gpu/shaders/gpu_shader_material.glsl
1290	Isn't the view normal already in viewspace, why multiply it by the viewmatrix again? It looks like viewdirection is in world space, should normaldirection also be in world space?

Maybe this should be exposed as a "format" enum, with options "Blender environment map" and "Cubemap".

Regarding control over the use of a normal map or not, I think that's determined by the texture stack order, no options needed for that.

Ulysse Martin (youle) added inline comments.Feb 3 2016, 9:55 PM

release/scripts/startup/bl_ui/properties_texture.py
622	The button enable display of cubemaps reflections in the viewport and in the game engine. When you press F12, the cubemap render is always done (it doesn't use my code to be rendered). The default option for the button is Disabled because we don't want always the cubemap to be rendered in the viewport (for loud/heavy/enormous scenes with tons of cubemaps). For the game engine, it's different. If you can't have a flowing/fluid viewport display with cubemap enabled, there is no reason that you're game will work "fluidly" so it's to the user to choose what he wants. (About realtime enable/disable cubemap reflections at runtime in the GE (discussed on IRC), could we do another patch for that if needed?) The UI button has to be in the texture panel because it is related to a texture (cubemap) and you can have several textures per material.
source/blender/gpu/shaders/gpu_shader_material.glsl
1290	The shader is correct. Tested with vec3 reflecteddirection = reflect(vp, normaldirection), vec3 reflecteddirection = reflect(viewdirection, vn), vec3 reflecteddirection = reflect(vp, vn) and it doesn't work :) https://en.wikibooks.org/wiki/GLSL_Programming/Blender/Reflecting_Surfaces

Duarte Farrajota Ramos (duarteframos) added a subscriber: Duarte Farrajota Ramos (duarteframos).Feb 4 2016, 8:15 AM

Alexander Romanov (a.romanov) added inline comments.Feb 4 2016, 10:06 AM

release/scripts/startup/bl_ui/properties_texture.py
622	I've tested this patch and made a video https://youtu.be/__qnSKnv-9A . It seems to me that use_reflection option for texture is not necessary. We can just disable its influence. I think it makes more sense to have a similar option with more global influence like "World Background" in viewport Display settings that would disable all cubemap material textures in viewport. Also, I think that support of "Mirror" influence is more desired by users, because it has more more correct physical behavior. But anyway, it's great!

@Alexander Romanov (a.romanov): Thanks for the test... It's weird, I can't reproduce the different display in the 2 views, and whatever I do, mirror button has no effect (maybe I don't know how to use it). Is there a conflict with world textures? Does it behaves the same if you remove world textures, what if you make the Suzanne texture single user? When my patch is not applied, it happens that you can see world texture on suzanne if you add the world texture as envmap texture for suzanne and play with the settings? Do you think the different display in the two views can be related to my patch (you can't reproduce it with other texture types?)?

Could you upload your testfile? This test file has been created or saved AFTER my patch was applied? I can't reproduce the same behavior when you play with mapping settings.

The mapping settings should not have impact when Cubemap reflections is enabled because the coordinates are computed in the mtex_cubemap_refl shader... So there is a shader after that impacts cubemap reflections.

For the UI, I don't know, I let the other decide. (Or we could make the cubemap reflection enabled by default, let the UI per texture and add another button to enable/disable all cubemaps) /// SEE EDIT 3

EDIT: Ok, this right window was rendered mode :)

EDIT2: I can't implement all the viewport shaders to match Blender internal render. I'm not competent enough.

EDIT3: Ok to remove the UI (it is a duplicata of influence>color button)

EDIT4: We can also add another condition to enable reflections: something like: if tex->mappingflag & REFLECTION_MAPPING (mtex->texco == TEXCO_REFL)
if (tex && tex->ima &&
((tex->type == TEX_IMAGE) ||
(tex->type == TEX_ENVMAP && mtex->texco == TEXCO_REFL)))

http://www.pasteall.org/64258/diff