Code review comment for lp:~registry/stellarium/scenery3d_Qt5

Revision history for this message
Florian Schaukowitsch (fschauk) wrote :

Hello, original developer here. Thanks for the kind words!

Some notes:
> - Util.hpp/.cpp : try to move part of this in the main Stellarium code. Get
> rid of useless extern, try to use QString versions instead of including part
> of the STL
Yeah, that is an artifact of some older parts of the codebase (~2012). I was unhappy with that when I started "repairing" the code, but never really found time to change this.

> - there are still some fixed pipeline code in the opengl code (glBegin & co)
The fixed-function stuff is only needed for some debugging output (rendering of bounding boxes/view frusta). During normal usage it will never be called. On pure ES devices or on ANGLE, the code is disabled.

> - on real OpenGL ES devices you also need to specify the precision of the
> variable in the shaders (low, medium, high), they can be added as Qt
> automatically remove them on desktop platforms
Most of the shaders should be compatible with OpenGL ES2. As far as I know, it is not strictly required to specify precision for everything in a vertex shader (because there is already a default precision for floats, see https://stackoverflow.com/questions/5366416/in-opengl-es-2-0-glsl-where-do-you-need-precision-specifiers ), and the fragment shaders should contain the precision specifiers where needed.

The only shaders which will not work with OpenGL ES2 should be the optional geometry shaders (.geom files, require #version 150/OpenGL 3.2, are completely disabled on ES2 or if 3.2 is not supported) and the s3d_pixellit.frag shader (which uses some #version 120 stuff, but has an alternative simplified implementation for ES2 in s3d_pixellit_es.frag).

The plugin was tested by me on an AMD HD5850, an integrated Intel chip (OpenGL 2.1) and on one of the new NVidia GTX 970. Additionally I tested it on ANGLE builds, and on an OpenGL ES2 emulator (http://malideveloper.arm.com/develop-for-mali/tools/software-tools/opengl-es-emulator/). Georg also tested it on an Odroid C1 (which has embedded Mali graphics with "real" OpenGL ES2), seemingly without problems.

OpenGL ES2/ANGLE has some limitations. To support larger models (with more than 65535 vertices) the plugin depends on the presence of the "GL_OES_element_index_uint" extension. Shadows require "GL_OES_depth_texture" or "GL_ANGLE_depth_texture". The only thing really missing on ES2/ANGLE is shadow filtering (including PCSS).

« Back to merge proposal