Merge lp:~vanvugt/unity/fix-1036528 into lp:unity

Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 2566
Proposed branch: lp:~vanvugt/unity/fix-1036528
Merge into: lp:unity
Diff against target: 40 lines (+6/-8)
1 file modified
plugins/unityshell/src/unityshell.cpp (+6/-8)
To merge this branch: bzr merge lp:~vanvugt/unity/fix-1036528
Reviewer Review Type Date Requested Status
Sam Spilsbury (community) Approve
Unity Team Pending
Review via email: mp+119677@code.launchpad.net

This proposal supersedes a proposal from 2012-08-14.

Commit message

Fixed: Benchmark window was being rendered shrunken, misplaced and frequently
invisible if unity was built with USE_MODERN_COMPIZ_GL. (LP: #1036528)

This was caused by some rather important code being #ifdef'd out incorrectly.

Description of the change

Fixed: Benchmark window was being rendered shrunken, misplaced and frequently
invisible if unity was built with USE_MODERN_COMPIZ_GL. (LP: #1036528)

This was caused by some rather important code being #ifdef'd out incorrectly.

To post a comment you must log in.
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

 +#ifndef USE_GLES
18 #ifndef USE_MODERN_COMPIZ_GL
19 (*GL::bindFramebuffer)(GL_FRAMEBUFFER_EXT, _active_fbo);
20 +#endif

Hmm, are we not binding a default fbo in gles builds of nux ?

review: Needs Information
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Hmm, there might be a case where we want to GL::bindFramebuffer and USE_GLES. That is if we're using the legacy GLES patches instead of USE_MODERN_COMPIZ_GL.

I guess we should support that, though I'm not sure it's necessary any more...

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Okay all good.

Yikes dealing with #ifdefs in launchpad diffs is confusing :(

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I know it's confusing. That's Launchpad bug 917502.

Revision history for this message
Unity Merger (unity-merger) wrote :

No commit message specified.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Fixed commit message (thanks LP for deleting the old one). Re-approved.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/unityshell/src/unityshell.cpp'
2--- plugins/unityshell/src/unityshell.cpp 2012-08-14 19:27:17 +0000
3+++ plugins/unityshell/src/unityshell.cpp 2012-08-15 06:31:48 +0000
4@@ -479,7 +479,7 @@
5
6 void UnityScreen::nuxPrologue()
7 {
8-#ifndef USE_MODERN_COMPIZ_GL
9+#ifndef USE_GLES
10 /* Vertex lighting isn't used in Unity, we disable that state as it could have
11 * been leaked by another plugin. That should theoretically be switched off
12 * right after PushAttrib since ENABLE_BIT is meant to restore the LIGHTING
13@@ -514,7 +514,8 @@
14 {
15 #ifndef USE_MODERN_COMPIZ_GL
16 (*GL::bindFramebuffer)(GL_FRAMEBUFFER_EXT, _active_fbo);
17-
18+#endif
19+#ifndef USE_GLES
20 glMatrixMode(GL_PROJECTION);
21 glLoadIdentity();
22 glMatrixMode(GL_MODELVIEW);
23@@ -539,14 +540,11 @@
24 * NVIDIA compatibility reasons */
25 glEnableClientState(GL_VERTEX_ARRAY);
26 glEnableClientState(GL_TEXTURE_COORD_ARRAY);
27+ glDepthRange(0, 1);
28+ //glViewport(-1, -1, 2, 2);
29+ gScreen->resetRasterPos();
30 #else
31-#ifdef USE_GLES
32 glDepthRangef(0, 1);
33-#else
34- glDepthRange(0, 1);
35-#endif
36- //glViewport(-1, -1, 2, 2);
37- gScreen->resetRasterPos();
38 #endif
39
40 glDisable(GL_SCISSOR_TEST);