Merge lp:~albaguirre/mir/fix-eglplasma-example-animation-slowdown into lp:mir

[Alberto Aguirre]

Avoid degradation of floating point precision as the parameter of the mod
function increases. That was causing eglplasma to stutter after some time
on some GPUs (LP: #1189753)

[Alan Griffiths (alan-griffiths)]

LGTM

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1443
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/978/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1112
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1110
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/696
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/710
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/710/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/715
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/715/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/697
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/697/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/664
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4460

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/978/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

okay

review: Approve

[Daniel van Vugt (vanvugt)]

I'll have to check this. I thought I had eliminated all such continuity artefacts. Also, it's sub-optimal moving math outside of the shader.

This needs fixing too:
if (angle > pi2) angle = 0.0f;
angle is no longer smoothly continuous and could stutter slightly when it reaches pi2.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Could you log a bug with instructions on how to reproduce the problem?

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
Approved revid is not set in launchpad. This is most likely a launchpad issue and re-approve should fix it. There is also a chance (although a very small one) this is a permission problem of the ps-jenkins bot.
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-autolanding/439/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1118
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1116
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/702
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-autolanding/344
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-autolanding/344/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-autolanding/345
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-autolanding/345/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/703
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/703/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/669
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4468

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

Elaborating; the problem with:
  if (angle > pi2) angle = 0.0f;
is that the increment 0.005 does not divide evenly into the period 6.283185308. That will lead to regular visible frame skip kind of artefacts.

The solution is to wrap around smoothly to the correct next angle, which is:
    angle = fmod(angle, pi2);
However that's exactly the same as the old code which does the same more efficiently in the shader with "mod(theta, pi2)". So what I'm saying is the existing code is both more correct and more efficient.

If you've observed bugs, can you detail them in a bug report? Unless it's bug 1189753 ?

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Hmm, OK, maybe not more efficient with the added logic in the shader being per-fragment instead of your per-frame. But certainly this proposal is less correct and would be expected to cause glitches.

[Alberto Aguirre (albaguirre)]

> I'll have to check this. I thought I had eliminated all such continuity
> artefacts. Also, it's sub-optimal moving math outside of the shader.
>
You can see this in android devices.

> This needs fixing too:
> if (angle > pi2) angle = 0.0f;
> angle is no longer smoothly continuous and could stutter slightly when it
> reaches pi2.

I don't see any stutter.

[Alberto Aguirre (albaguirre)]

> Could you log a bug with instructions on how to reproduce the problem?

https://bugs.launchpad.net/mir/+bug/1288059

[Alberto Aguirre (albaguirre)]

> Elaborating; the problem with:
> if (angle > pi2) angle = 0.0f;
> is that the increment 0.005 does not divide evenly into the period
> 6.283185308. That will lead to regular visible frame skip kind of artefacts.
>
> The solution is to wrap around smoothly to the correct next angle, which is:
> angle = fmod(angle, pi2);
> However that's exactly the same as the old code which does the same more
> efficiently in the shader with "mod(theta, pi2)". So what I'm saying is the
> existing code is both more correct and more efficient.
>

I understand that. But in android devices, with large angles, the mod operation will return coarse values, which leads to the "stuttering" or "slowdown" effect.

> If you've observed bugs, can you detail them in a bug report? Unless it's bug
> 1189753 ?

Actually yes it's does look like #1189753

[Alberto Aguirre (albaguirre)]

> This needs fixing too:
> if (angle > pi2) angle = 0.0f;

fixed.

[Daniel van Vugt (vanvugt)]

Oh, I see. So it's a bug in droid/mobile GLSL/GPU implementations. All makes sense now. In that case a simple fix should suffice. Replace:
   if (angle > pi2) angle = 0.0f;
with:
   angle = fmod(angle, pi2);

Excitingly, this sounds like a resolution for bug 1189753. Should we link it?

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Ah. The subtraction approach is good too.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1445
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/984/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1123
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1121
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/706
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/716
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/716/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/721
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/721/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/707
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/707/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/672
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4471

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/984/rebuild

review: Approve (continuous-integration)

=== modified file 'examples/eglplasma.c'
--- examples/eglplasma.c	2013-09-19 13:24:22 +0000
+++ examples/eglplasma.c	2014-03-05 06:12:02 +0000
@@ -85,14 +85,13 @@
         "    const float pi2 = 6.283185308;                      \n"
         "    float u = texcoord.x * pi2;                         \n"
         "    float v = texcoord.y * pi2;                         \n"
-        "    float t = mod(theta, pi2);                          \n"
-        "    float us = (cos(1.1 * u + 7.0 * t) +                \n"
-        "                cos(2.3 * v * cos(1.0 * t)) +           \n"
-        "                cos(0.3 * u * cos(3.0 * t))             \n"
+        "    float us = (cos(1.1 * u + 7.0 * theta) +            \n"
+        "                cos(2.3 * v * cos(1.0 * theta)) +       \n"
+        "                cos(0.3 * u * cos(3.0 * theta))         \n"
         "               ) / 3.0;                                 \n"
-        "    float vs = (cos(2.3 * v + 8.0 * t) +                \n"
-        "                cos(1.3 * u * cos(3.0 * t)) +           \n"
-        "                cos(1.7 * v * cos(2.0 * t))             \n"
+        "    float vs = (cos(2.3 * v + 8.0 * theta) +            \n"
+        "                cos(1.3 * u * cos(3.0 * theta)) +       \n"
+        "                cos(1.7 * v * cos(2.0 * theta))         \n"
         "               ) / 3.0;                                 \n"
         "    float x = (us * vs + 1.0) / 2.0;                    \n"
         "    gl_FragColor = vec4(gradient(x), 1.0);              \n"
@@ -105,6 +104,7 @@
         1.0f,-1.0f,
        -1.0f,-1.0f,
     };
+    const float pi2 = 6.283185308f;
     GLuint vshader, fshader, prog;
     GLint linked, low_color, high_color, vpos, theta;
     unsigned int width = 0, height = 0;
@@ -151,6 +151,8 @@
     {
         glUniform1f(theta, angle);
         angle += 0.005f;
+        if (angle > pi2)
+            angle -= pi2;
         glDrawArrays(GL_TRIANGLE_FAN, 0, 4);
         mir_eglapp_swap_buffers();
     }