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
=== modified file 'src/server/compositor/gl_renderer.cpp'
--- src/server/compositor/gl_renderer.cpp 2014-01-22 08:47:16 +0000
+++ src/server/compositor/gl_renderer.cpp 2014-01-23 10:03:07 +0000
@@ -288,12 +288,12 @@
288 float rad = rotation * M_PI / 180.0f;288 float rad = rotation * M_PI / 180.0f;
289 GLfloat cos = cosf(rad);289 GLfloat cos = cosf(rad);
290 GLfloat sin = sinf(rad);290 GLfloat sin = sinf(rad);
291 GLfloat rot[16] = {cos, -sin, 0.0f, 0.0f,291 GLfloat rot[16] = {cos, sin, 0.0f, 0.0f,
292 sin, cos, 0.0f, 0.0f,292 -sin, cos, 0.0f, 0.0f,
293 0.0f, 0.0f, 1.0f, 0.0f,293 0.0f, 0.0f, 1.0f, 0.0f,
294 0.0f, 0.0f, 0.0f, 1.0f};294 0.0f, 0.0f, 0.0f, 1.0f};
295 glUseProgram(program);295 glUseProgram(program);
296 glUniformMatrix4fv(display_transform_uniform_loc, 1, GL_TRUE, rot);296 glUniformMatrix4fv(display_transform_uniform_loc, 1, GL_FALSE, rot);
297 glUseProgram(0);297 glUseProgram(0);
298 glClear(GL_COLOR_BUFFER_BIT);298 glClear(GL_COLOR_BUFFER_BIT);
299}299}
300300
=== modified file 'tests/unit-tests/compositor/test_gl_renderer.cpp'
--- tests/unit-tests/compositor/test_gl_renderer.cpp 2014-01-22 08:47:16 +0000
+++ tests/unit-tests/compositor/test_gl_renderer.cpp 2014-01-23 10:03:07 +0000
@@ -38,6 +38,7 @@
38using testing::ReturnRef;38using testing::ReturnRef;
39using testing::Pointee;39using testing::Pointee;
40using testing::AnyNumber;40using testing::AnyNumber;
41using testing::AtLeast;
41using testing::_;42using testing::_;
4243
43namespace mt=mir::test;44namespace mt=mir::test;
@@ -253,8 +254,8 @@
253 EXPECT_CALL(mock_gl, glClear(_)).Times(AnyNumber());254 EXPECT_CALL(mock_gl, glClear(_)).Times(AnyNumber());
254 EXPECT_CALL(mock_gl, glUseProgram(_)).Times(AnyNumber());255 EXPECT_CALL(mock_gl, glUseProgram(_)).Times(AnyNumber());
255 EXPECT_CALL(mock_gl, glActiveTexture(_)).Times(AnyNumber());256 EXPECT_CALL(mock_gl, glActiveTexture(_)).Times(AnyNumber());
256 EXPECT_CALL(mock_gl, glUniformMatrix4fv(_, _, _, _))257 EXPECT_CALL(mock_gl, glUniformMatrix4fv(_, _, GL_FALSE, _))
257 .Times(AnyNumber());258 .Times(AtLeast(1));
258 EXPECT_CALL(mock_gl, glUniform1f(_, _)).Times(AnyNumber());259 EXPECT_CALL(mock_gl, glUniform1f(_, _)).Times(AnyNumber());
259 EXPECT_CALL(mock_gl, glBindBuffer(_, _)).Times(AnyNumber());260 EXPECT_CALL(mock_gl, glBindBuffer(_, _)).Times(AnyNumber());
260 EXPECT_CALL(mock_gl, glVertexAttribPointer(_, _, _, _, _, _))261 EXPECT_CALL(mock_gl, glVertexAttribPointer(_, _, _, _, _, _))
@@ -303,6 +304,7 @@
303304
304 InSequence seq;305 InSequence seq;
305306
307 EXPECT_CALL(mock_gl, glUniformMatrix4fv(display_transform_uniform_location, 1, GL_FALSE, _));
306 EXPECT_CALL(mock_gl, glClear(_));308 EXPECT_CALL(mock_gl, glClear(_));
307 EXPECT_CALL(mock_gl, glUseProgram(stub_program));309 EXPECT_CALL(mock_gl, glUseProgram(stub_program));
308 EXPECT_CALL(criteria, shaped())310 EXPECT_CALL(criteria, shaped())

Subscribers

People subscribed via source and target branches