Mir

Merge lp:~vanvugt/mir/fix-1271853 into lp:mir

  1. fix-1271853
  2. Merge into development-branch
Proposed by Daniel van Vugt
Status: Merged
Approved by: Kevin DuBois
Approved revision: no longer in the source branch.
Merged at revision: 1352
Proposed branch: lp:~vanvugt/mir/fix-1271853
Merge into: lp:mir
Diff against target: 50 lines (+7/-5)
2 files modified
src/server/compositor/gl_renderer.cpp (+3/-3)
tests/unit-tests/compositor/test_gl_renderer.cpp (+4/-2)
To merge this branch: bzr merge lp:~vanvugt/mir/fix-1271853
Reviewer Review Type Date Requested Status
Kevin DuBois (community) Approve
Alan Griffiths Needs Information
PS Jenkins bot (community) continuous-integration Approve
Alexandros Frantzis (community) Approve
Review via email: mp+202821@code.launchpad.net

Commit message

Don't ask glUniformMatrix4fv to transpose your matrix. That option was
officially deprecated between OpenGL and OpenGL|ES. And some drivers
like the Nexus 10 don't implement it, resulting in incorrect transformations
and even nothing on screen! (LP: #1271853)

To post a comment you must log in.
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

OK.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1351
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/715/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/715
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/711
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/316
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/445
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/445/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/449
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/449/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/317
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/317/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/329
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3283

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

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

should we be checking for errors after calling glUniformMatrix4fv() (it doesn't return an error code but the documentation seems to indicate there is one somewhere - we would presumably have seen GL_INVALID_OPERATION if we'd checked).

review: Needs Information
Revision history for this message
Kevin DuBois (kdub) wrote :

lgtm, gets pixels back on my screen

review: Approve
Revision history for this message
Kevin DuBois (kdub) wrote :

> should we be checking for errors after calling glUniformMatrix4fv() (it
> doesn't return an error code but the documentation seems to indicate there is
> one somewhere - we would presumably have seen GL_INVALID_OPERATION if we'd
> checked).

Seems a reasonable check, but I'll top approve in the meantime to get pixels back on our screens

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/server/compositor/gl_renderer.cpp'
2--- src/server/compositor/gl_renderer.cpp 2014-01-22 08:47:16 +0000
3+++ src/server/compositor/gl_renderer.cpp 2014-01-23 10:03:07 +0000
4@@ -288,12 +288,12 @@
5 float rad = rotation * M_PI / 180.0f;
6 GLfloat cos = cosf(rad);
7 GLfloat sin = sinf(rad);
8- GLfloat rot[16] = {cos, -sin, 0.0f, 0.0f,
9- sin, cos, 0.0f, 0.0f,
10+ GLfloat rot[16] = {cos, sin, 0.0f, 0.0f,
11+ -sin, cos, 0.0f, 0.0f,
12 0.0f, 0.0f, 1.0f, 0.0f,
13 0.0f, 0.0f, 0.0f, 1.0f};
14 glUseProgram(program);
15- glUniformMatrix4fv(display_transform_uniform_loc, 1, GL_TRUE, rot);
16+ glUniformMatrix4fv(display_transform_uniform_loc, 1, GL_FALSE, rot);
17 glUseProgram(0);
18 glClear(GL_COLOR_BUFFER_BIT);
19 }
20
21=== modified file 'tests/unit-tests/compositor/test_gl_renderer.cpp'
22--- tests/unit-tests/compositor/test_gl_renderer.cpp 2014-01-22 08:47:16 +0000
23+++ tests/unit-tests/compositor/test_gl_renderer.cpp 2014-01-23 10:03:07 +0000
24@@ -38,6 +38,7 @@
25 using testing::ReturnRef;
26 using testing::Pointee;
27 using testing::AnyNumber;
28+using testing::AtLeast;
29 using testing::_;
30
31 namespace mt=mir::test;
32@@ -253,8 +254,8 @@
33 EXPECT_CALL(mock_gl, glClear(_)).Times(AnyNumber());
34 EXPECT_CALL(mock_gl, glUseProgram(_)).Times(AnyNumber());
35 EXPECT_CALL(mock_gl, glActiveTexture(_)).Times(AnyNumber());
36- EXPECT_CALL(mock_gl, glUniformMatrix4fv(_, _, _, _))
37- .Times(AnyNumber());
38+ EXPECT_CALL(mock_gl, glUniformMatrix4fv(_, _, GL_FALSE, _))
39+ .Times(AtLeast(1));
40 EXPECT_CALL(mock_gl, glUniform1f(_, _)).Times(AnyNumber());
41 EXPECT_CALL(mock_gl, glBindBuffer(_, _)).Times(AnyNumber());
42 EXPECT_CALL(mock_gl, glVertexAttribPointer(_, _, _, _, _, _))
43@@ -303,6 +304,7 @@
44
45 InSequence seq;
46
47+ EXPECT_CALL(mock_gl, glUniformMatrix4fv(display_transform_uniform_location, 1, GL_FALSE, _));
48 EXPECT_CALL(mock_gl, glClear(_));
49 EXPECT_CALL(mock_gl, glUseProgram(stub_program));
50 EXPECT_CALL(criteria, shaped())

Subscribers

People subscribed via source and target branches