Mir

Merge lp://qastaging/~vanvugt/mir/fix-1423462 into lp://qastaging/mir

  1. fix-1423462
  2. Merge into development-branch
Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 3057
Proposed branch: lp://qastaging/~vanvugt/mir/fix-1423462
Merge into: lp://qastaging/mir
Diff against target: 191 lines (+103/-15)
5 files modified
playground/demo-shell/demo_renderer.cpp (+0/-4)
src/renderers/gl/renderer.cpp (+48/-11)
tests/include/mir/test/doubles/mock_gl.h (+1/-0)
tests/mir_test_doubles/mock_gl.cpp (+7/-0)
tests/unit-tests/renderers/gl/test_gl_renderer.cpp (+47/-0)
To merge this branch: bzr merge lp://qastaging/~vanvugt/mir/fix-1423462
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alan Griffiths Approve
Daniel van Vugt Abstain
Andreas Pokorny (community) Approve
Alexandros Frantzis (community) Approve
Review via email: mp+275661@code.qastaging.launchpad.net

Commit message

Avoid spuriously blending RGBX as RGBA (LP: #1423462)

Not only was it sub-optimal in performance but more importantly caused
unpredictable discolouration of RGBX clients (first observed by 3v1n0 in
mplayer-SDL but more recently also seen in Xmir -sw). Any clients that
filled the 'X' byte with 0xff were unaffected.

Description of the change

Manual test case:
Force a client to use an RGBX pixel format with alpha values below 0xff:
  $ mir_demo_client_multiwin -p2 # or -p4
Before the fix: Transparent windows.
After the fix: No transparency.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3057
http://jenkins.qa.ubuntu.com/job/mir-ci/5392/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4627
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3534
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4569
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/527/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1546
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1546/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/527
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/527/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4570
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4570/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7194
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24539
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/528/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5392/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks good.

Style nit:

+ if (renderable.shaped())

+ if (blend_func_dest == GL_ZERO)
+ glDisable(GL_BLEND);

If any clause of the if-elseif-else statement uses curly braces then all clauses (even single line ones) should use it for consistency.

review: Approve
Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

> Looks good.
>
> Style nit:
>
> + if (renderable.shaped())
>
> + if (blend_func_dest == GL_ZERO)
> + glDisable(GL_BLEND);
>
> If any clause of the if-elseif-else statement uses curly braces then all
> clauses (even single line ones) should use it for consistency.

yes this looks a bit awkward...
feel free to cleanup, lgtm otherwise

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

Hmm, if the framebuffer honours alpha then there's still going to be a minor problem with translucent RGBX windows passing through some uninitialized alpha data.

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

FAILED: Continuous integration, rev:3058
http://jenkins.qa.ubuntu.com/job/mir-ci/5393/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4629
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3536
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4571/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/529/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1547
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1547/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/529
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/529/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4572
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4572/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7196/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24544
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/530/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5393/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) :
review: Abstain
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3060
http://jenkins.qa.ubuntu.com/job/mir-ci/5394/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4630
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3537
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4572
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/530/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1548
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1548/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/530/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4573
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4573/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7197
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24547
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/531/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5394/rebuild

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

I see no obvious problems

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

PASSED: Continuous integration, rev:3061
http://jenkins.qa.ubuntu.com/job/mir-ci/5402/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4639
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3546
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4581
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/539/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1556
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1556/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/539
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/539/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4582
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4582/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7205
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24563
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/540/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5402/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches