Merge lp:~gerboland/qtubuntu/adopt-more-eglconvenience2 into lp:qtubuntu

Proposed by Gerry Boland
Status: Merged
Approved by: Gerry Boland
Approved revision: 341
Merged at revision: 324
Proposed branch: lp:~gerboland/qtubuntu/adopt-more-eglconvenience2
Merge into: lp:qtubuntu
Diff against target: 671 lines (+163/-242)
7 files modified
README (+6/-7)
src/ubuntumirclient/glcontext.cpp (+30/-165)
src/ubuntumirclient/glcontext.h (+11/-19)
src/ubuntumirclient/integration.cpp (+27/-25)
src/ubuntumirclient/integration.h (+0/-4)
src/ubuntumirclient/window.cpp (+86/-21)
src/ubuntumirclient/window.h (+3/-1)
To merge this branch: bzr merge lp:~gerboland/qtubuntu/adopt-more-eglconvenience2
Reviewer Review Type Date Requested Status
Unity8 CI Bot continuous-integration Approve
Loïc Molinari (community) Approve
Emanuele Antonio Faraone (community) Approve
Michał Sawicz Needs Fixing
Daniel van Vugt Needs Fixing
Review via email: mp+295248@code.launchpad.net

Commit message

Convert UbuntuOpenGLContext to use QEGLPlatformContext, and allow clients to customize chosen GLConfig

Credit to Loïc Molinari for the initial implementation.

Several things come of this change:
1. Convert UbuntuOpenGLContext to use QEGLPlatformContext means we can use more built-in Qt code to configure and use the GL context, which should ensure that Qt-on-Mir applications behave more consistently with Qt-on-X apps.
2. Allow apps to configure the SurfaceFormat/GLConfig, as opposed to always overruling it to be ABGR8888. But choose ABGR8888 as the default if not specified by the app.
3. If app does not request alpha channel, but still gets a GLConfig with an alpha channel, communicate to Mir that the alpha channel is unused (aka XBGR8888), and thus compostor need not blend the surface.

To post a comment you must log in.
Revision history for this message
Gerry Boland (gerboland) wrote :

To test, verify that no visual regressions are evident in the application.

Also, use QT_LOGGING_RULES=ubuntumirclient=true to check things like this:

ubuntumirclient: EGL configuration attributes:
 EGL_BUFFER_SIZE: 32
 EGL_ALPHA_SIZE: 8
 EGL_BLUE_SIZE: 8
 EGL_GREEN_SIZE: 8
 EGL_RED_SIZE: 8
 EGL_DEPTH_SIZE: 24
 EGL_STENCIL_SIZE: 8
which should be the case for all QML apps.

Then, test that the "Actual format" contain:

Actual format: QSurfaceFormat(version 2.0, options QFlags(), depthBufferSize 24, redBufferSize 8, greenBufferSize 8, blueBufferSize 8, alphaBufferSize 8, stencilBufferSize 8, samples 0, swapBehavior 0, swapInterval 1, profile 0) with associated Mir pixel format: XBGR8888

for applications which do not need proper transparency (note the "Mir pixel format" being XBGR8888 - X=disabled).

But if it does need transparency (can use qmlscene --transparent to test this), it should report ABGR8888 instead.

Note that QML needs the alpha buffer to render correctly. But it is perfectly ok for the compositor to ignore the alpha channel when compositing an opaque surface.

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:328
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/56/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1697
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1647
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1647
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1640
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1640/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1640
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1640/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1640
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1640/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1640
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1640/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1640
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1640/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1640
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1640/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/56/rebuild

review: Approve (continuous-integration)
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:329
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/57/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1698
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1648
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1648
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1641
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1641/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1641
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1641/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1641
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1641/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1641
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1641/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1641
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1641/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1641
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1641/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/57/rebuild

review: Approve (continuous-integration)
Revision history for this message
Loïc Molinari (loic.molinari) wrote :

Cool!

Looks Good.

I guess we can get rid of this long waiting one [1] then (I still need to validate that there's nothing missing from there, but it doesn't seem so).

I don't remember exactly the details of [2] (IIRC I saw that somewhere in the Qt code base) but what do you think of not forcing the alpha size to 8 if not specified by the user? I guess if the user doesn't set the alpha size explicitely, he doesn't want his surface to support translucent surface anyway. You could then use mFormat instead of mWindow->requestedFormat() there: "if (mWindow->requestedFormat().alphaBufferSize() < 0) {".

Also, as it's done in [2], what do you think of forcing undefined color channels to 8 independently from the others instead of setting all of them to to 8 only if all are undefined? It sounds like a more predictive use case to me.

The env var part could be useful too for us to check the behaviour of some platforms. But that's definitely not a priority.

[1] https://code.launchpad.net/~unity-team/qtubuntu/requested-surface-format-fix/+merge/274379
[2] http://bazaar.launchpad.net/~loic.molinari/qtubuntu/requested-surface-format-fix-default-to-8-bit-rgb-config-and-improved-mir-pixel-format-retrieval/revision/267

review: Needs Information
Revision history for this message
Gerry Boland (gerboland) wrote :

> I guess we can get rid of this long waiting one [1] then (I still need to
> validate that there's nothing missing from there, but it doesn't seem so).

Yep done.

> I don't remember exactly the details of [2] (IIRC I saw that somewhere in the
> Qt code base) but what do you think of not forcing the alpha size to 8 if not
> specified by the user? I guess if the user doesn't set the alpha size
> explicitely, he doesn't want his surface to support translucent surface
> anyway. You could then use mFormat instead of mWindow->requestedFormat()
> there: "if (mWindow->requestedFormat().alphaBufferSize() < 0) {".

I did this actually to start with. But unless I'm mistaken, the QML renderer requires an alpha channel in order to do blending correctly. This is why I enforced an alpha channel being requested. I too would like to clarify this, because what you suggest does make more sense.

> Also, as it's done in [2], what do you think of forcing undefined color
> channels to 8 independently from the others instead of setting all of them to
> to 8 only if all are undefined? It sounds like a more predictive use case to
> me.
Well I presumed that if a user sets any channel size, then I'll assume they know what they're doing and so leave them alone. If they ask for RGB565, I'm not going to turn on alpha channel for them.

> The env var part could be useful too for us to check the behaviour of some
> platforms. But that's definitely not a priority.
True, that is nice. I'll include that.

Revision history for this message
Loïc Molinari (loic.molinari) wrote :

Nice, that looks perfect now!

I'm not approving right now since I'd like to do one more round of review next week.

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

Is that a typo?

+ case mir_pixel_format_rgb_565: return "RGB888";

review: Needs Fixing
Revision history for this message
Gerry Boland (gerboland) wrote :

> Is that a typo?
>
> + case mir_pixel_format_rgb_565: return "RGB888";

Quite right, nice catch, thanks

Revision history for this message
Gerry Boland (gerboland) wrote :

Hmm, I've broken phone apps (they only get a GLES1.1 context now), need to fix that.

Revision history for this message
Gerry Boland (gerboland) wrote :

Ok, should be good to go now.
Tested on
- intel i945
- Nexus 4 & 7
- Freiza
- BQ Aquaris e4.5
- Meizu MX4

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:332
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/60/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/1735
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1761
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1708
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1708
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1708
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1701
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1701/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1701
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1701/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1701
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1701/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1701
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1701/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1701
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1701/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1701
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1701/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1701
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1701/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1701
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1701/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1701
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1701/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/60/rebuild

review: Approve (continuous-integration)
Revision history for this message
Loïc Molinari (loic.molinari) wrote :

I'm a bit skeptical about the Mesa string hack from r335 since it's forcing a 1.0 context for on all the drivers based on Mesa, even those supporting higher versions. That, I guess, prevents all the apps depending on OpenGL ES 2 or OpenGL ES 3 features to create a context with Qt.

Couldn't we 1st test with the version requested by the user, and then if it fails, do a 1.4 request if the driver is based on Mesa?

or, if not possible, maybe for now we can do the 1.4 request only if it's i915? (what are the EGL_VENDOR and EGL_VERSION strings on i915?)

Revision history for this message
Gerry Boland (gerboland) wrote :

I did test on my more modern Intel machine and found the context returned by Mesa was the highest it supported. But yeah, I'll try to use it as a fallback-only position

Revision history for this message
Gerry Boland (gerboland) wrote :

Ok, the above remove the SurfaceFormat filtering stuff, instead
1. using QSurfaceFormat::defaultFormat to set the use of RGB888 by default. If user changes that, it's their responsibility.
2. adding a policy where if the user-requested config failed to be chosen on Mesa, it will retry with GL1.4.

Revision history for this message
Gerry Boland (gerboland) wrote :

It introduces a slight amount of code duplication unfortunately, but it is either that or replace a chunk of the EGLConvenience stuff. I think this is the easiest option

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:338
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/61/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/1742
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1768
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1715
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1715
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1715
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1708
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1708/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1708
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1708/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1708
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1708/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1708
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1708/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1708
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1708/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1708
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1708/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1708
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1708/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1708
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1708/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1708
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1708/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/61/rebuild

review: Approve (continuous-integration)
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:340
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/62/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/1744/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1770
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1717
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1717
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1717
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1710
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1710/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1710
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1710/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1710
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1710/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1710
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1710/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1710
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1710/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1710/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1710
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1710/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1710
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1710/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1710
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1710/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/62/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Loïc Molinari (loic.molinari) wrote :

All good!

review: Approve
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

Text conflict in src/ubuntumirclient/glcontext.cpp
Text conflict in src/ubuntumirclient/integration.cpp
2 conflicts encountered.

review: Needs Fixing
341. By Gerry Boland

Merge trunk, fix conflicts, give hints to branch predictor

Revision history for this message
Emanuele Antonio Faraone (emanueleant03) :
review: Approve
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:341
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/70/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/1839
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1865
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1803
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1803
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1803
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1794
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1794/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1794
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1794/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1794
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1794/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1794
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1794/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1794
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1794/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1794
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1794/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1794
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1794/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1794
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1794/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1794
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1794/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/70/rebuild

review: Approve (continuous-integration)
Revision history for this message
Emanuele Antonio Faraone (emanueleant03) wrote :

news?

Revision history for this message
Loïc Molinari (loic.molinari) wrote :

Couldn't we set workaround_brokenFBOReadBack right after context creation in qtubuntu? That would avoid, at each makeCurrent() call, fetching data from different places in memory (the context, the private context and the bss for platforms with non-broken FBO readback) with the potential cache misses implied.

review: Needs Information
Revision history for this message
Gerry Boland (gerboland) wrote :

> Couldn't we set workaround_brokenFBOReadBack right after context creation in
> qtubuntu? That would avoid, at each makeCurrent() call, fetching data from
> different places in memory (the context, the private context and the bss for
> platforms with non-broken FBO readback) with the potential cache misses
> implied.

That's not code I actually wrote in this branch, it is code that landed in trunk. That duplicates what the Android QPA plugin does . We trusted Qt guys knew what they were doing :)

Revision history for this message
Loïc Molinari (loic.molinari) wrote :

OK, then I guess it's something worth checking later :)

review: Approve
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:341
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/81/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2059
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2087
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1998
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1998
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1998
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1989
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1989/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1989
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1989/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1989
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1989/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1989
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1989/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1989
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1989/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1989
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1989/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1989
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1989/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1989
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1989/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1989
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1989/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/81/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'README'
2--- README 2015-12-09 13:30:15 +0000
3+++ README 2016-06-03 09:55:53 +0000
4@@ -23,13 +23,10 @@
5 $ qmlscene -platform ubuntumirclient Foo.qml
6 $ QT_QPA_PLATFORM=ubuntumirclient qmlscene Foo.qml
7
8- The QPA plugins expose the following environment variables:
9-
10- QTUBUNTU_SWAPINTERVAL: Specifies the required swap interval as an
11- integer. 1 by default.
12-
13- QTUBUNTU_MULTISAMPLE: Enables multisampling with using 4 samples
14- per fragment.
15+ This QPA plugin exposes the following environment variables:
16+
17+ QT_QPA_EGLFS_SWAPINTERVAL: Specifies the required swap interval as an
18+ integer. 1 by default.
19
20 QTUBUNTU_NO_THREADED_OPENGL: Disables QtQuick threaded OpenGL
21 rendering.
22@@ -51,6 +48,8 @@
23 * ubuntumirclient.swapBuffers - Messages related to surface buffer swapping.
24 * ubuntumirclient - For all other messages.
25
26+ The QT_QPA_EGLFS_DEBUG environment variable prints a little more information
27+ from Qt's internals.
28
29 4. Building
30 -----------
31
32=== modified file 'src/ubuntumirclient/glcontext.cpp'
33--- src/ubuntumirclient/glcontext.cpp 2016-05-27 13:05:20 +0000
34+++ src/ubuntumirclient/glcontext.cpp 2016-06-03 09:55:53 +0000
35@@ -25,58 +25,12 @@
36
37 namespace {
38
39-void printOpenGLESConfig() {
40- static bool once = true;
41- if (once) {
42- const char* string = (const char*) glGetString(GL_VENDOR);
43- qCDebug(ubuntumirclient, "OpenGL ES vendor: %s", string);
44- string = (const char*) glGetString(GL_RENDERER);
45- qCDebug(ubuntumirclient, "OpenGL ES renderer: %s", string);
46- string = (const char*) glGetString(GL_VERSION);
47- qCDebug(ubuntumirclient, "OpenGL ES version: %s", string);
48- string = (const char*) glGetString(GL_SHADING_LANGUAGE_VERSION);
49- qCDebug(ubuntumirclient, "OpenGL ES Shading Language version: %s", string);
50- string = (const char*) glGetString(GL_EXTENSIONS);
51- qCDebug(ubuntumirclient, "OpenGL ES extensions: %s", string);
52- once = false;
53- }
54-}
55-
56-void printEglConfig(EGLDisplay display, EGLConfig config) {
57+void printEglConfig(EGLDisplay display, EGLConfig config)
58+{
59 Q_ASSERT(display != EGL_NO_DISPLAY);
60 Q_ASSERT(config != nullptr);
61
62- static const struct { const EGLint attrib; const char* name; } kAttribs[] = {
63- { EGL_BUFFER_SIZE, "EGL_BUFFER_SIZE" },
64- { EGL_ALPHA_SIZE, "EGL_ALPHA_SIZE" },
65- { EGL_BLUE_SIZE, "EGL_BLUE_SIZE" },
66- { EGL_GREEN_SIZE, "EGL_GREEN_SIZE" },
67- { EGL_RED_SIZE, "EGL_RED_SIZE" },
68- { EGL_DEPTH_SIZE, "EGL_DEPTH_SIZE" },
69- { EGL_STENCIL_SIZE, "EGL_STENCIL_SIZE" },
70- { EGL_CONFIG_CAVEAT, "EGL_CONFIG_CAVEAT" },
71- { EGL_CONFIG_ID, "EGL_CONFIG_ID" },
72- { EGL_LEVEL, "EGL_LEVEL" },
73- { EGL_MAX_PBUFFER_HEIGHT, "EGL_MAX_PBUFFER_HEIGHT" },
74- { EGL_MAX_PBUFFER_PIXELS, "EGL_MAX_PBUFFER_PIXELS" },
75- { EGL_MAX_PBUFFER_WIDTH, "EGL_MAX_PBUFFER_WIDTH" },
76- { EGL_NATIVE_RENDERABLE, "EGL_NATIVE_RENDERABLE" },
77- { EGL_NATIVE_VISUAL_ID, "EGL_NATIVE_VISUAL_ID" },
78- { EGL_NATIVE_VISUAL_TYPE, "EGL_NATIVE_VISUAL_TYPE" },
79- { EGL_SAMPLES, "EGL_SAMPLES" },
80- { EGL_SAMPLE_BUFFERS, "EGL_SAMPLE_BUFFERS" },
81- { EGL_SURFACE_TYPE, "EGL_SURFACE_TYPE" },
82- { EGL_TRANSPARENT_TYPE, "EGL_TRANSPARENT_TYPE" },
83- { EGL_TRANSPARENT_BLUE_VALUE, "EGL_TRANSPARENT_BLUE_VALUE" },
84- { EGL_TRANSPARENT_GREEN_VALUE, "EGL_TRANSPARENT_GREEN_VALUE" },
85- { EGL_TRANSPARENT_RED_VALUE, "EGL_TRANSPARENT_RED_VALUE" },
86- { EGL_BIND_TO_TEXTURE_RGB, "EGL_BIND_TO_TEXTURE_RGB" },
87- { EGL_BIND_TO_TEXTURE_RGBA, "EGL_BIND_TO_TEXTURE_RGBA" },
88- { EGL_MIN_SWAP_INTERVAL, "EGL_MIN_SWAP_INTERVAL" },
89- { EGL_MAX_SWAP_INTERVAL, "EGL_MAX_SWAP_INTERVAL" },
90- { -1, NULL }
91- };
92- const char* string = eglQueryString(display, EGL_VENDOR);
93+ const char *string = eglQueryString(display, EGL_VENDOR);
94 qCDebug(ubuntumirclient, "EGL vendor: %s", string);
95
96 string = eglQueryString(display, EGL_VERSION);
97@@ -85,93 +39,19 @@
98 string = eglQueryString(display, EGL_EXTENSIONS);
99 qCDebug(ubuntumirclient, "EGL extensions: %s", string);
100
101- qCDebug(ubuntumirclient, "EGL configuration attibutes:");
102- for (int index = 0; kAttribs[index].attrib != -1; index++) {
103- EGLint value;
104- if (eglGetConfigAttrib(display, config, kAttribs[index].attrib, &value))
105- qCDebug(ubuntumirclient, " %s: %d", kAttribs[index].name, static_cast<int>(value));
106- }
107-}
108-
109-QString eglErrorToString(EGLint errorNumber)
110-{
111- #define EGL_ERROR_CASE(error) case error: return QString(#error);
112-
113- switch (errorNumber) {
114- EGL_ERROR_CASE(EGL_SUCCESS)
115- EGL_ERROR_CASE(EGL_NOT_INITIALIZED)
116- EGL_ERROR_CASE(EGL_BAD_ACCESS)
117- EGL_ERROR_CASE(EGL_BAD_ALLOC)
118- EGL_ERROR_CASE(EGL_BAD_ATTRIBUTE)
119- EGL_ERROR_CASE(EGL_BAD_CONTEXT)
120- EGL_ERROR_CASE(EGL_BAD_CONFIG)
121- EGL_ERROR_CASE(EGL_BAD_CURRENT_SURFACE)
122- EGL_ERROR_CASE(EGL_BAD_DISPLAY)
123- EGL_ERROR_CASE(EGL_BAD_SURFACE)
124- EGL_ERROR_CASE(EGL_BAD_MATCH)
125- EGL_ERROR_CASE(EGL_BAD_PARAMETER)
126- EGL_ERROR_CASE(EGL_BAD_NATIVE_PIXMAP)
127- EGL_ERROR_CASE(EGL_BAD_NATIVE_WINDOW)
128- EGL_ERROR_CASE(EGL_CONTEXT_LOST)
129- default:
130- return QString("?");
131- }
132-
133- #undef EGL_ERROR_CASE
134-}
135-
136-EGLenum api_in_use()
137-{
138- #ifdef QTUBUNTU_USE_OPENGL
139- return EGL_OPENGL_API;
140- #else
141- return EGL_OPENGL_ES_API;
142- #endif
143-}
144-
145-const int kSwapInterval = 1;
146-
147-int qGetEnvIntValue(const char *varName, bool *ok)
148-{
149- return qgetenv(varName).toInt(ok);
150+ qCDebug(ubuntumirclient, "EGL configuration attributes:");
151+ q_printEglConfig(display, config);
152 }
153
154 } // anonymous namespace
155
156-UbuntuOpenGLContext::UbuntuOpenGLContext(const QSurfaceFormat &surfaceFormat, UbuntuOpenGLContext *share,
157- EGLDisplay display, EGLConfig config)
158- : mSurfaceFormat(surfaceFormat)
159- , mEglDisplay(display)
160+UbuntuOpenGLContext::UbuntuOpenGLContext(const QSurfaceFormat &format, QPlatformOpenGLContext *share,
161+ EGLDisplay display)
162+ : QEGLPlatformContext(format, share, display, 0)
163 {
164- // Create an OpenGL ES 2 context.
165- QVector<EGLint> attribs;
166- attribs.append(EGL_CONTEXT_CLIENT_VERSION);
167- attribs.append(2);
168- attribs.append(EGL_NONE);
169- ASSERT(eglBindAPI(api_in_use()) == EGL_TRUE);
170-
171 if (ubuntumirclient().isDebugEnabled()) {
172- printEglConfig(mEglDisplay, config);
173+ printEglConfig(display, eglConfig());
174 }
175-
176- // Set vblank swap interval.
177- bool ok;
178- int swapInterval = qGetEnvIntValue("QTUBUNTU_SWAPINTERVAL", &ok);
179- if (!ok)
180- swapInterval = kSwapInterval;
181-
182- qCDebug(ubuntumirclient, "Setting swap interval to %d", swapInterval);
183- eglSwapInterval(mEglDisplay, swapInterval);
184-
185- mEglContext = eglCreateContext(mEglDisplay, config, share ? share->eglContext() : EGL_NO_CONTEXT,
186- attribs.constData());
187-
188- Q_ASSERT(mEglContext != EGL_NO_CONTEXT);
189-}
190-
191-UbuntuOpenGLContext::~UbuntuOpenGLContext()
192-{
193- ASSERT(eglDestroyContext(mEglDisplay, mEglContext) == EGL_TRUE);
194 }
195
196 static bool needsFBOReadBackWorkaround()
197@@ -179,7 +59,7 @@
198 static bool set = false;
199 static bool needsWorkaround = false;
200
201- if (!set) {
202+ if (Q_UNLIKELY(!set)) {
203 const char *rendererString = reinterpret_cast<const char *>(glGetString(GL_RENDERER));
204 needsWorkaround = qstrncmp(rendererString, "Mali-400", 8) == 0
205 || qstrncmp(rendererString, "Mali-T7", 7) == 0
206@@ -202,47 +82,32 @@
207 }
208 return offscreen->buffer()->bind();
209 } else {
210- EGLSurface eglSurface = static_cast<UbuntuWindow*>(surface)->eglSurface();
211- ASSERT(eglBindAPI(api_in_use()) == EGL_TRUE);
212-
213- EGLBoolean result = eglMakeCurrent(mEglDisplay, eglSurface, eglSurface, mEglContext);
214- if (result == EGL_FALSE) {
215- qCCritical(ubuntumirclient, "eglMakeCurrent() failed with %s",
216- qPrintable(eglErrorToString(eglGetError())));
217- return false;
218- }
219-
220- QOpenGLContextPrivate *ctx_d = QOpenGLContextPrivate::get(context());
221- if (!ctx_d->workaround_brokenFBOReadBack && needsFBOReadBackWorkaround()) {
222- ctx_d->workaround_brokenFBOReadBack = true;
223- }
224-
225- if (ubuntumirclient().isDebugEnabled()) {
226- printOpenGLESConfig();
227- }
228- return true;
229+ const bool ret = QEGLPlatformContext::makeCurrent(surface);
230+
231+ if (Q_LIKELY(ret)) {
232+ QOpenGLContextPrivate *ctx_d = QOpenGLContextPrivate::get(context());
233+ if (!ctx_d->workaround_brokenFBOReadBack && needsFBOReadBackWorkaround()) {
234+ ctx_d->workaround_brokenFBOReadBack = true;
235+ }
236+ }
237+
238+ return ret;
239 }
240 }
241
242-void UbuntuOpenGLContext::doneCurrent()
243+// Following method used internally in the base class QEGLPlatformContext to access
244+// the egl surface of a QPlatformSurface/UbuntuWindow
245+EGLSurface UbuntuOpenGLContext::eglSurfaceForPlatformSurface(QPlatformSurface *surface)
246 {
247- ASSERT(eglBindAPI(api_in_use()) == EGL_TRUE);
248- ASSERT(eglMakeCurrent(mEglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT) == EGL_TRUE);
249+ auto ubuntuWindow = static_cast<UbuntuWindow *>(surface);
250+ return ubuntuWindow->eglSurface();
251 }
252
253-void UbuntuOpenGLContext::swapBuffers(QPlatformSurface* surface)
254+void UbuntuOpenGLContext::swapBuffers(QPlatformSurface *surface)
255 {
256- UbuntuWindow *ubuntuWindow = static_cast<UbuntuWindow*>(surface);
257-
258- EGLSurface eglSurface = ubuntuWindow->eglSurface();
259- ASSERT(eglBindAPI(api_in_use()) == EGL_TRUE);
260- ASSERT(eglSwapBuffers(mEglDisplay, eglSurface) == EGL_TRUE);
261-
262+ QEGLPlatformContext::swapBuffers(surface);
263+
264+ // notify window on swap completion
265+ auto ubuntuWindow = static_cast<UbuntuWindow *>(surface);
266 ubuntuWindow->onSwapBuffersDone();
267 }
268-
269-void (*UbuntuOpenGLContext::getProcAddress(const QByteArray& procName)) ()
270-{
271- ASSERT(eglBindAPI(api_in_use()) == EGL_TRUE);
272- return eglGetProcAddress(procName.constData());
273-}
274
275=== modified file 'src/ubuntumirclient/glcontext.h'
276--- src/ubuntumirclient/glcontext.h 2016-04-28 14:09:54 +0000
277+++ src/ubuntumirclient/glcontext.h 2016-06-03 09:55:53 +0000
278@@ -18,30 +18,22 @@
279 #define UBUNTU_OPENGL_CONTEXT_H
280
281 #include <qpa/qplatformopenglcontext.h>
282+#include <private/qeglplatformcontext_p.h>
283
284 #include <EGL/egl.h>
285
286-class UbuntuOpenGLContext : public QPlatformOpenGLContext
287+class UbuntuOpenGLContext : public QEGLPlatformContext
288 {
289 public:
290- UbuntuOpenGLContext(const QSurfaceFormat &surfaceFormat, UbuntuOpenGLContext *share,
291- EGLDisplay display, EGLConfig config);
292- virtual ~UbuntuOpenGLContext();
293-
294- // QPlatformOpenGLContext methods.
295- QSurfaceFormat format() const override { return mSurfaceFormat; }
296- void swapBuffers(QPlatformSurface *surface) override;
297- bool makeCurrent(QPlatformSurface *surface) override;
298- void doneCurrent() override;
299- bool isValid() const override { return mEglContext != EGL_NO_CONTEXT; }
300- void (*getProcAddress(const QByteArray& procName)) () override;
301-
302- EGLContext eglContext() const { return mEglContext; }
303-
304-private:
305- const QSurfaceFormat mSurfaceFormat;
306- EGLContext mEglContext;
307- EGLDisplay mEglDisplay;
308+ UbuntuOpenGLContext(const QSurfaceFormat &format, QPlatformOpenGLContext *share,
309+ EGLDisplay display);
310+
311+ // QEGLPlatformContext methods.
312+ void swapBuffers(QPlatformSurface *surface) final;
313+ bool makeCurrent(QPlatformSurface *surface) final;
314+
315+protected:
316+ EGLSurface eglSurfaceForPlatformSurface(QPlatformSurface *surface) final;
317 };
318
319 #endif // UBUNTU_OPENGL_CONTEXT_H
320
321=== modified file 'src/ubuntumirclient/integration.cpp'
322--- src/ubuntumirclient/integration.cpp 2016-04-28 14:44:18 +0000
323+++ src/ubuntumirclient/integration.cpp 2016-06-03 09:55:53 +0000
324@@ -29,7 +29,6 @@
325
326 // Qt
327 #include <QGuiApplication>
328-#include <private/qguiapplication_p.h>
329 #include <qpa/qplatformnativeinterface.h>
330 #include <qpa/qplatforminputcontextfactory_p.h>
331 #include <qpa/qplatforminputcontext.h>
332@@ -90,31 +89,17 @@
333
334 mMirConnection = u_application_instance_get_mir_connection(mInstance);
335
336+ // Choose the default surface format suited to the Mir platform
337+ QSurfaceFormat defaultFormat;
338+ defaultFormat.setRedBufferSize(8);
339+ defaultFormat.setGreenBufferSize(8);
340+ defaultFormat.setBlueBufferSize(8);
341+ QSurfaceFormat::setDefaultFormat(defaultFormat);
342+
343 // Initialize EGL.
344- ASSERT(eglBindAPI(EGL_OPENGL_ES_API) == EGL_TRUE);
345-
346 mEglNativeDisplay = mir_connection_get_egl_native_display(mMirConnection);
347 ASSERT((mEglDisplay = eglGetDisplay(mEglNativeDisplay)) != EGL_NO_DISPLAY);
348 ASSERT(eglInitialize(mEglDisplay, nullptr, nullptr) == EGL_TRUE);
349-
350- // Configure EGL buffers format for all Windows.
351- mSurfaceFormat.setRedBufferSize(8);
352- mSurfaceFormat.setGreenBufferSize(8);
353- mSurfaceFormat.setBlueBufferSize(8);
354- mSurfaceFormat.setAlphaBufferSize(8);
355- mSurfaceFormat.setDepthBufferSize(24);
356- mSurfaceFormat.setStencilBufferSize(8);
357- if (!qEnvironmentVariableIsEmpty("QTUBUNTU_MULTISAMPLE")) {
358- mSurfaceFormat.setSamples(4);
359- qCDebug(ubuntumirclient, "setting MSAA to 4 samples");
360- }
361-#ifdef QTUBUNTU_USE_OPENGL
362- mSurfaceFormat.setRenderableType(QSurfaceFormat::OpenGL);
363-#else
364- mSurfaceFormat.setRenderableType(QSurfaceFormat::OpenGLES);
365-#endif
366-
367- mEglConfig = q_configFromGLFormat(mEglDisplay, mSurfaceFormat, true);
368 }
369
370 void UbuntuClientIntegration::initialize()
371@@ -197,7 +182,7 @@
372
373 QPlatformWindow* UbuntuClientIntegration::createPlatformWindow(QWindow* window)
374 {
375- return new UbuntuWindow(window, mClipboard, mInput, mNativeInterface, mEglDisplay, mEglConfig, mMirConnection);
376+ return new UbuntuWindow(window, mClipboard, mInput, mNativeInterface, mEglDisplay, mMirConnection);
377 }
378
379 bool UbuntuClientIntegration::hasCapability(QPlatformIntegration::Capability cap) const
380@@ -246,8 +231,25 @@
381 QPlatformOpenGLContext* UbuntuClientIntegration::createPlatformOpenGLContext(
382 QOpenGLContext* context)
383 {
384- return new UbuntuOpenGLContext(mSurfaceFormat, static_cast<UbuntuOpenGLContext*>(context->shareHandle()),
385- mEglDisplay, mEglConfig);
386+ QSurfaceFormat format(context->format());
387+
388+ auto platformContext = new UbuntuOpenGLContext(format, context->shareHandle(), mEglDisplay);
389+ if (!platformContext->isValid()) {
390+ // Older Intel Atom-based devices only support OpenGL 1.4 compatibility profile but by default
391+ // QML asks for at least OpenGL 2.0. The XCB GLX backend ignores this request and returns a
392+ // 1.4 context, but the XCB EGL backend tries to honour it, and fails. The 1.4 context appears to
393+ // have sufficient capabilities on MESA (i915) to render correctly however. So reduce the default
394+ // requested OpenGL version to 1.0 to ensure EGL will give us a working context (lp:1549455).
395+ static const bool isMesa = QString(eglQueryString(mEglDisplay, EGL_VENDOR)).contains(QStringLiteral("Mesa"));
396+ if (isMesa) {
397+ qCDebug(ubuntumirclient, "Attempting to choose OpenGL 1.4 context which may suit Mesa");
398+ format.setMajorVersion(1);
399+ format.setMinorVersion(4);
400+ delete platformContext;
401+ platformContext = new UbuntuOpenGLContext(format, context->shareHandle(), mEglDisplay);
402+ }
403+ }
404+ return platformContext;
405 }
406
407 QStringList UbuntuClientIntegration::themeNames() const
408
409=== modified file 'src/ubuntumirclient/integration.h'
410--- src/ubuntumirclient/integration.h 2016-04-28 14:09:54 +0000
411+++ src/ubuntumirclient/integration.h 2016-06-03 09:55:53 +0000
412@@ -66,9 +66,7 @@
413
414 // New methods.
415 MirConnection *mirConnection() const { return mMirConnection; }
416- QSurfaceFormat surfaceFormat() const { return mSurfaceFormat; }
417 EGLDisplay eglDisplay() const { return mEglDisplay; }
418- EGLConfig eglConfig() const { return mEglConfig; }
419 EGLNativeDisplayType eglNativeDisplay() const { return mEglNativeDisplay; }
420 UbuntuScreenObserver *screenObserver() const { return mScreenObserver.data(); }
421
422@@ -99,9 +97,7 @@
423
424 // EGL related
425 EGLDisplay mEglDisplay{EGL_NO_DISPLAY};
426- EGLConfig mEglConfig{nullptr};
427 EGLNativeDisplayType mEglNativeDisplay;
428- QSurfaceFormat mSurfaceFormat;
429 };
430
431 #endif // UBUNTU_CLIENT_INTEGRATION_H
432
433=== modified file 'src/ubuntumirclient/window.cpp'
434--- src/ubuntumirclient/window.cpp 2016-04-28 14:44:18 +0000
435+++ src/ubuntumirclient/window.cpp 2016-06-03 09:55:53 +0000
436@@ -29,6 +29,7 @@
437 #include <QMutexLocker>
438 #include <QSize>
439 #include <QtMath>
440+#include <private/qeglconvenience_p.h>
441
442 // Platform API
443 #include <ubuntu/application/instance.h>
444@@ -101,6 +102,24 @@
445 }
446 }
447
448+const char *mirPixelFormatToStr(MirPixelFormat pixelFormat)
449+{
450+ switch (pixelFormat) {
451+ case mir_pixel_format_invalid: return "invalid";
452+ case mir_pixel_format_abgr_8888: return "ABGR8888";
453+ case mir_pixel_format_xbgr_8888: return "XBGR8888";
454+ case mir_pixel_format_argb_8888: return "ARGB8888";
455+ case mir_pixel_format_xrgb_8888: return "XRGB8888";
456+ case mir_pixel_format_bgr_888: return "BGR888";
457+ case mir_pixel_format_rgb_888: return "RGB888";
458+ case mir_pixel_format_rgb_565: return "RGB565";
459+ case mir_pixel_format_rgba_5551: return "RGBA5551";
460+ case mir_pixel_format_rgba_4444: return "RGBA4444";
461+ case mir_pixel_formats:
462+ default: return "???";
463+ }
464+}
465+
466 MirSurfaceState qtWindowStateToMirSurfaceState(Qt::WindowState state)
467 {
468 switch (state) {
469@@ -124,14 +143,6 @@
470 return id++;
471 }
472
473-MirPixelFormat defaultPixelFormatFor(MirConnection *connection)
474-{
475- MirPixelFormat format;
476- unsigned int nformats;
477- mir_connection_get_available_surface_formats(connection, &format, 1, &nformats);
478- return format;
479-}
480-
481 UAUiWindowRole roleFor(QWindow *window)
482 {
483 QVariant roleVariant = window->property("role");
484@@ -151,12 +162,11 @@
485 return parent ? static_cast<UbuntuWindow *>(parent->handle()) : nullptr;
486 }
487
488-Spec makeSurfaceSpec(QWindow *window, UbuntuInput *input, MirConnection *connection)
489+Spec makeSurfaceSpec(QWindow *window, UbuntuInput *input, MirPixelFormat pixelFormat, MirConnection *connection)
490 {
491 const auto geom = window->geometry();
492 const int width = geom.width() > 0 ? geom.width() : 1;
493 const int height = geom.height() > 0 ? geom.height() : 1;
494- const auto pixelFormat = defaultPixelFormatFor(connection);
495
496 if (U_ON_SCREEN_KEYBOARD_ROLE == roleFor(window)) {
497 qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p) - creating input method surface (width=%d, height=%d", window, width, height);
498@@ -217,11 +227,11 @@
499 }
500 }
501
502-MirSurface *createMirSurface(QWindow *window, int mirOutputId, UbuntuInput *input,
503+MirSurface *createMirSurface(QWindow *window, int mirOutputId, UbuntuInput *input, MirPixelFormat pixelFormat,
504 MirConnection *connection, mir_surface_event_callback inputCallback,
505 void* inputContext)
506 {
507- auto spec = makeSurfaceSpec(window, input, connection);
508+ auto spec = makeSurfaceSpec(window, input, pixelFormat, connection);
509
510 // Install event handler as early as possible
511 mir_surface_spec_set_event_handler(spec.get(), inputCallback, inputContext);
512@@ -244,6 +254,18 @@
513 return surface;
514 }
515
516+MirPixelFormat disableAlphaBufferIfPossible(MirPixelFormat pixelFormat)
517+{
518+ switch(pixelFormat) {
519+ case mir_pixel_format_abgr_8888:
520+ return mir_pixel_format_xbgr_8888;
521+ case mir_pixel_format_argb_8888:
522+ return mir_pixel_format_xrgb_8888;
523+ default: // can do nothing, leave it alone
524+ return pixelFormat;
525+ }
526+}
527+
528 // FIXME - in order to work around https://bugs.launchpad.net/mir/+bug/1346633
529 // we need to guess the panel height (3GU)
530 int panelHeight()
531@@ -266,8 +288,7 @@
532 class UbuntuSurface
533 {
534 public:
535- UbuntuSurface(UbuntuWindow *platformWindow, EGLDisplay display, EGLConfig config, int mirOutputId,
536- UbuntuInput *input, MirConnection *connection)
537+ UbuntuSurface(UbuntuWindow *platformWindow, EGLDisplay display, UbuntuInput *input, MirConnection *connection)
538 : mWindow(platformWindow->window())
539 , mPlatformWindow(platformWindow)
540 , mInput(input)
541@@ -275,9 +296,44 @@
542 , mEglDisplay(display)
543 , mNeedsRepaint(false)
544 , mParented(mWindow->transientParent() || mWindow->parent())
545+ , mFormat(mWindow->requestedFormat())
546 , mShellChrome(mWindow->flags() & LowChromeWindowHint ? mir_shell_chrome_low : mir_shell_chrome_normal)
547 {
548- mMirSurface = createMirSurface(mWindow, mirOutputId, input, connection, surfaceEventCallback, this);
549+ // Have Qt choose most suitable EGLConfig for the requested surface format, and update format to reflect it
550+ EGLConfig config = q_configFromGLFormat(display, mFormat, true);
551+ if (config == 0) {
552+ // Older Intel Atom-based devices only support OpenGL 1.4 compatibility profile but by default
553+ // QML asks for at least OpenGL 2.0. The XCB GLX backend ignores this request and returns a
554+ // 1.4 context, but the XCB EGL backend tries to honour it, and fails. The 1.4 context appears to
555+ // have sufficient capabilities on MESA (i915) to render correctly however. So reduce the default
556+ // requested OpenGL version to 1.0 to ensure EGL will give us a working context (lp:1549455).
557+ static const bool isMesa = QString(eglQueryString(display, EGL_VENDOR)).contains(QStringLiteral("Mesa"));
558+ if (isMesa) {
559+ qCDebug(ubuntumirclient, "Attempting to choose OpenGL 1.4 context which may suit Mesa");
560+ mFormat.setMajorVersion(1);
561+ mFormat.setMinorVersion(4);
562+ config = q_configFromGLFormat(display, mFormat, true);
563+ }
564+ }
565+ if (config == 0) {
566+ qCritical() << "Qt failed to choose a suitable EGLConfig to suit the surface format" << mFormat;
567+ }
568+
569+ mFormat = q_glFormatFromConfig(display, config, mFormat);
570+
571+ // Have Mir decide the pixel format most suited to the chosen EGLConfig. This is the only way
572+ // Mir will know what EGLConfig has been chosen - it cannot deduce it from the buffers.
573+ auto pixelFormat = mir_connection_get_egl_pixel_format(connection, display, config);
574+ // But the chosen EGLConfig might have an alpha buffer enabled, even if not requested by the client.
575+ // If that's the case, try to edit the chosen pixel format in order to disable the alpha buffer.
576+ // This is an optimisation for the compositor, as it can avoid blending this surface.
577+ if (mWindow->requestedFormat().alphaBufferSize() < 0) {
578+ pixelFormat = disableAlphaBufferIfPossible(pixelFormat);
579+ }
580+
581+ const auto outputId = static_cast<UbuntuScreen *>(mWindow->screen()->handle())->mirOutputId();
582+
583+ mMirSurface = createMirSurface(mWindow, outputId, input, pixelFormat, connection, surfaceEventCallback, this);
584 mEglSurface = eglCreateWindowSurface(mEglDisplay, config, nativeWindowFor(mMirSurface), nullptr);
585
586 // Window manager can give us a final size different from what we asked for
587@@ -299,8 +355,11 @@
588 platformWindow->QPlatformWindow::setGeometry(geom);
589 QWindowSystemInterface::handleGeometryChange(mWindow, geom);
590
591- qCDebug(ubuntumirclient, "created surface at (%d, %d) with size (%d, %d), title '%s', role: '%d'\n",
592- geom.x(), geom.y(), geom.width(), geom.height(), mWindow->title().toUtf8().constData(), roleFor(mWindow));
593+ qCDebug(ubuntumirclient) << "Created surface with geometry:" << geom << "title:" << mWindow->title()
594+ << "role:" << roleFor(mWindow)
595+ << "\nRequested format:" << mWindow->requestedFormat()
596+ << "\nActual format:" << mFormat
597+ << "with associated Mir pixel format:" << mirPixelFormatToStr(pixelFormat);
598 }
599
600 ~UbuntuSurface()
601@@ -335,6 +394,8 @@
602 void setSurfaceParent(MirSurface*);
603 bool hasParent() const { return mParented; }
604
605+ QSurfaceFormat format() const { return mFormat; }
606+
607 private:
608 static void surfaceEventCallback(MirSurface* surface, const MirEvent *event, void* context);
609 void postEvent(const MirEvent *event);
610@@ -351,6 +412,7 @@
611 bool mNeedsRepaint;
612 bool mParented;
613 QSize mBufferSize;
614+ QSurfaceFormat mFormat;
615
616 QMutex mTargetSizeMutex;
617 QSize mTargetSize;
618@@ -509,8 +571,7 @@
619 }
620
621 UbuntuWindow::UbuntuWindow(QWindow *w, const QSharedPointer<UbuntuClipboard> &clipboard,
622- UbuntuInput *input, UbuntuNativeInterface *native, EGLDisplay eglDisplay,
623- EGLConfig eglConfig, MirConnection *mirConnection)
624+ UbuntuInput *input, UbuntuNativeInterface *native, EGLDisplay eglDisplay, MirConnection *mirConnection)
625 : QObject(nullptr)
626 , QPlatformWindow(w)
627 , mId(makeId())
628@@ -520,8 +581,7 @@
629 , mWindowVisible(false)
630 , mWindowExposed(true)
631 , mNativeInterface(native)
632- , mSurface(new UbuntuSurface{this, eglDisplay, eglConfig,
633- static_cast<UbuntuScreen*>(w->screen()->handle())->mirOutputId(), input, mirConnection})
634+ , mSurface(new UbuntuSurface{this, eglDisplay, input, mirConnection})
635 , mScale(1.0)
636 , mFormFactor(mir_form_factor_unknown)
637 {
638@@ -709,6 +769,11 @@
639 return mWindowVisible && mWindowExposed;
640 }
641
642+QSurfaceFormat UbuntuWindow::format() const
643+{
644+ return mSurface->format();
645+}
646+
647 void* UbuntuWindow::eglSurface() const
648 {
649 return mSurface->eglSurface();
650
651=== modified file 'src/ubuntumirclient/window.h'
652--- src/ubuntumirclient/window.h 2016-04-28 14:44:18 +0000
653+++ src/ubuntumirclient/window.h 2016-06-03 09:55:53 +0000
654@@ -40,7 +40,7 @@
655 Q_OBJECT
656 public:
657 UbuntuWindow(QWindow *w, const QSharedPointer<UbuntuClipboard> &clipboard,
658- UbuntuInput *input, UbuntuNativeInterface* native, EGLDisplay eglDisplay, EGLConfig eglConfig,
659+ UbuntuInput *input, UbuntuNativeInterface* native, EGLDisplay eglDisplay,
660 MirConnection *mirConnection);
661 virtual ~UbuntuWindow();
662
663@@ -54,6 +54,8 @@
664 void propagateSizeHints() override;
665 bool isExposed() const override;
666
667+ QSurfaceFormat format() const override;
668+
669 // Additional Window properties exposed by NativeInterface
670 MirFormFactor formFactor() const { return mFormFactor; }
671 float scale() const { return mScale; }

Subscribers

People subscribed via source and target branches