BGE: Skybox
AbandonedPublic

Authored by Thomas Szepe (hg1) on Aug 12 2015, 7:17 PM.

Details

Summary

This patch adds a skcybox rendering to the BGE and the 3D-Viewport.
The Skybox is working in multitexture and GLSL mode.
If the skybox is enabled in the world settings, the first 6 world texture slots are used for it.

The initial patch done by Martin Sell (Moerdn).

Diff Detail

Test file:
Skybox.blend

Changes since Moerden's initial version.

  1. Update for actual master.
  2. Fix crash if no world exist.
  3. Move duplicate render code to gpu_draw.c.
  4. Invert bottom texture vertical.

Planed improvements.

  1. Displaylist for skybox rendering.
  2. Use GL_CLAMP_TO_EDGE for texture coordinates.
  3. Disable skybox in orthographic view.

There some points are open for me.

  1. Do we need features like invert horizontal, vertical and rotate for the image? Or should the user do this with an image program?
  2. Is it really good to use the 6 world texture slots for this or should the textures assigned in a own texture slot window or should be tried to reuse the existing Background images function?
  3. Actually the existing background rendering only works if you look in orthographic view strait to the view. Is it better to try to modify the existing background rendering function, that it render also in perspective on all angels?
  4. Should we support single texture texture cubemaps with different alignments. And when which one?
    • Blender
    • Horizontal (Cross OpenGL)
    • Horizontal right
    • Horizontal T (Terragen, Halo)
    • Horizontal T right (Tensil, Source Engine)
    • Vertical Cross (ATI CubeMapGen)
    • Horizontal Strip
    • Horizontal Strip (Nvidia DDS Exporter Layout)
    • Vertical Stripe or column
  5. Should the skybox disabled in orthographic view or should it rendered and scaled up by an value in the UI.
  6. Should GLSL use a shader for the skybox rendering?
source/gameengine/Rasterizer/RAS_OpenGLRasterizer/RAS_OpenGLRasterizer.cpp
123

I forgotten to change this.

source/gameengine/Ketsji/KX_WorldInfo.cpp
73

you can use && operator, when one of condition is wrong the check is stopped else we check the next condition :

if (blenderscene->world->mtex[i] && blenderscene->world->mtex[i]->tex && blenderscene->world->mtex[i]->tex->ima)

It works well, nicely done.

  1. Do we need features like invert horizontal, vertical and rotate for the image? Or should the user do this with an image program?

For this patch, I wouldn't add those -- you can always add such functionality in the future.

  1. Is it really good to use the 6 world texture slots for this or should the textures assigned in a own texture slot window or should be tried to reuse the existing Background images function?

I think world texture slots are fine, but I also feel that this should be discussed well before diving into the implementation. Maybe include the UI team in the discussion?

Maybe you could update the texture UI when the "Sky box" checkbox is enabled, so that it's more intuitive to use. Right now people don't know they have to switch to the textures tab, and once there they don't know they have to enable the world textures, and once there they don't know how many textures (and in which order) they have to put in.

  1. Should we support single texture texture cubemaps with different alignments. And when which one?
    • Blender
    • Horizontal (Cross OpenGL)
    • Horizontal right
    • Horizontal T (Terragen, Halo)
    • Horizontal T right (Tensil, Source Engine)
    • Vertical Cross (ATI CubeMapGen)
    • Horizontal Strip
    • Horizontal Strip (Nvidia DDS Exporter Layout)
    • Vertical Stripe or column

I would welcome single-texture skyboxes, as it makes things easier to manage file-wise. Clicking one file is easier than clicking 6. However, requiring an image per side of the cube does give you a good answer to which alignment to support: none. Clicking 6 files can also be resolved by auto-detecting common naming standards, and loading all 6 sides into the correct slots.

Code-wise, I have an issue with the numbers 0-5. There is no indication what so ever as to which number belongs to which side. To determine this, you have to dive into the big switch-statement, decompile the math used in there, and hunt for the differences between the cases. This should be improved in the code itself (the switch can be heavily simplified) and in documentation (both in user documentation and code comments). Of course I understand that you'll only write docs after the code itself is finalized ;-)

source/blender/editors/space_view3d/view3d_draw.c
3609

I would store scene->world->mtex[iter] in a temporary variable, to avoid repeating array lookups and improve readability, and short-cut using && operators.

3800

Use (scene->world && (scene->world->flag & WO_DS_SHOW_SKYBOX)) here, instead of two nested if-statements.

source/blender/gpu/intern/gpu_draw.c
2071

What does iter do? It seems to choose a side of the sky cube, so why isn't it called cube_side instead?

2081

It's very difficult to see the differences between the different cases. It may be better to define some variables, give them their values based on iter, then use those variables in the glXXX calls.

Use the appropriate code style.

source/blender/makesrna/intern/rna_world.c
485

Why does this patch contain changes to mist-specific code? In what way is this required to get the skybox to work?

519

Replace "texture Skydome" with "textured skybox" -- a dome is spherical.

source/gameengine/Ketsji/KX_WorldInfo.cpp
73

It's probably also better to negate the condition, and use a continue statement, like above.

103

The function name should be plural (getSkyBoxTextureIds) as multiple IDs are returned.

Is there a reason that you use int *sides here instead of int sides[6]?

200

Why use getSkyBoxTextureId(tex) here, instead of just using m_skyboxtex? It seems like an unnecessary copy. If the copy *is* necessary, please add a comment to indicate why.

source/gameengine/Rasterizer/RAS_OpenGLRasterizer/RAS_OpenGLRasterizer.cpp
251

Code style: space after for (also in other places).

source/gameengine/Rasterizer/RAS_OpenGLRasterizer/RAS_OpenGLRasterizer.h
198

Please add a comment as to what this method does. The sides pointer must point to a large enough block of data, or it'll crash -- such cases should be documented.

This patch is still valid since world rendering with cube map was implemented ?

As I know the BGE use the cube map only to render object refections and still not render a background cube map. Actually Blender only render it in the viewport but not in the BGE.
I think you're confusing that with UPBGE where world background rendering is working.

I thought I will keep this open as long it is not implemented. But if you think it is better to close it, I will do it.

This has become obsolete with the changes in the UPBGE branch. Closing