Samsung / GearVRf

The GearVR framework(GearVRf) is an Open Source VR rendering library for application development on VR-supported Android devices.
http://www.gearvrf.org
Apache License 2.0
407 stars 217 forks source link

Fixing two post effect #1929

Closed apjagdale closed 6 years ago

apjagdale commented 6 years ago

Wrong number of parameters were passed.

GearVRF DCO signed off by: Abhijit Jagdale a.jagdale@samsung.com

ragner commented 6 years ago

It is working for me. 👍 Thanks

ragner commented 6 years ago

My test at gvr-blurfilter from https://github.com/ragner/GearVRf-Demos/tree/blurfilter2Pass may be showing some side effect of this PR.

It is showing the blurred image in front of the camera instead of on the scene object.

liaxim commented 6 years ago

@ragner You need to update pos_tex.vsh in similar fashion:

#extension GL_ARB_separate_shader_objects : enable
#extension GL_ARB_shading_language_420pack : enable

@MATRIX_UNIFORMS

precision mediump float;
layout ( location = 0 ) in  vec3 a_position;
layout ( location = 1 ) in vec2 a_texcoord;

layout ( location = 0 ) out vec2 vTextureCoord;

void main()
{
  vTextureCoord = a_texcoord.xy;
  gl_Position = u_mvp * vec4(a_position, 1.0);
}
liaxim commented 6 years ago

@ragner Bear in mind that you will be rendering the same scene-object three times with different settings on top of each other. You are not going to combine the vertical and the horizontal blur using GVRRenderPass-es.

You might want to examine the https://github.com/Samsung/GearVRf/issues/1155 issue and the convolution shader @thomasflynn added there. It only needs a small patch to pass the mvp to the shader. I had a note to make the solution to that issue built-in "feature" for android views but looks like you are doing something like that already. Please consider making it part of the core framework. Thanks.

liaxim commented 6 years ago

Planning to merge on Friday, it restores the gvr-blurfilter to its 3.x functionality. @ragner I think you have other, separate issues too we could deal with in additional PRs.

ragner commented 6 years ago

Hi @liaxim about https://github.com/thomasflynn/GearVRf-Demos/blob/blur/gvr-events/app/src/main/res/raw/convolution_fragment.fsh

The convolutionMatrix is not been used in the shader. Should I just remove it?

liaxim commented 6 years ago

It has been a while since I looked at this code but if that matrix is unused, then go ahead and remove it.

thomasflynn commented 6 years ago

yeah, i originally intended to be able to update the convolution matrix via uniform, but had troubles with it and just ended up hard-coding it in the shader. if that's fine with you too, then go ahead and remove the convolution matrix.

ragner commented 6 years ago

Hi @thomasflynn and @liaxim , I have got some nice result after some small adjustments ... nice shader! I will make a PR soon, thank you.