[Bf-codereview] BGE Harmony Branch (issue 5491068)

[mogurijin at gmail.com]

Most everything else has been pretty simple/straight-forward fixes, but
here's a few I had some more questions/comments on.


https://codereview.appspot.com/5491068/diff/2001/source/blender/gpu/intern/gpu_extensions.c
File source/blender/gpu/intern/gpu_extensions.c (right):

https://codereview.appspot.com/5491068/diff/2001/source/blender/gpu/intern/gpu_extensions.c#newcode89
source/blender/gpu/intern/gpu_extensions.c:89: } GG = {1, 0, 0, 0, 0,
0};
Actually the ", 0" doesn't even end up affecting the shaders part of GG,
since the sixth member of GG is npotdisabled (and only six members are
explicitly specified). All members after npotdisabled are set to 0.
Maybe the line should be "} GG = {1, 0};"?

On 2012/02/29 13:57:22, brechtvl wrote:
> Does this work, shouldn't it be ",  {0, 0}"? I would expect this to
give a
> compile error with some compilers.

https://codereview.appspot.com/5491068/diff/2001/source/blender/gpu/intern/gpu_extensions.c#newcode873
source/blender/gpu/intern/gpu_extensions.c:873: GPUShader *blur_shader =
GPU_shader_get_builtin_shader(GPU_SHADER_SEP_GAUSSIAN_BLUR);
Should something be printed to the console if blur_shader is NULL? Some
sort of warning perhaps? Is there a specific function to use, or can
printf() be used in this situation?

On 2012/02/29 13:57:22, brechtvl wrote:
> There should be a NULL pointer check before using blur_shader, it
might fail to
> compile for some reason.

https://codereview.appspot.com/5491068/diff/2001/source/blender/gpu/intern/gpu_extensions.c#newcode878
source/blender/gpu/intern/gpu_extensions.c:878:
glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, blurfb->object);
I didn't want to push more matrices onto the stack (and I think OpenGL
even errored out on me here), so I bound the FBO myself without pushing
matrices. I'll make sure to add a comment.

On 2012/02/29 13:57:22, brechtvl wrote:
> Is there a particular reason why GPU_framebuffer_texture_bind is not
used?

https://codereview.appspot.com/5491068/