qtubuntu

Merge lp://staging/~albaguirre/qtubuntu/use-mir-rs-api into lp://staging/qtubuntu

use-mir-rs-api
Merge into trunk

Proposed by Alberto Aguirre on 2017-03-31

Status:	Needs review
Proposed branch:	lp://staging/~albaguirre/qtubuntu/use-mir-rs-api
Merge into:	lp://staging/qtubuntu
Diff against target:	592 lines (+154/-188) 7 files modified debian/control (+3/-4) src/ubuntumirclient/qmirclientcursor.cpp (+68/-25) src/ubuntumirclient/qmirclientinput.cpp (+0/-4) src/ubuntumirclient/qmirclientintegration.cpp (+1/-2) src/ubuntumirclient/qmirclientintegration.h (+1/-2) src/ubuntumirclient/qmirclientwindow.cpp (+80/-150) src/ubuntumirclient/ubuntumirclient.pro (+1/-1)
To merge this branch:	bzr merge lp://staging/~albaguirre/qtubuntu/use-mir-rs-api
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Unity8 CI Bot	continuous-integration		Needs Fixing on 2017-05-27
Gerry Boland (community)			Needs Information on 2017-05-22
Alan Griffiths		2017-03-31	Needs Information on 2017-05-19
Review via email: mp+321627@code.staging.launchpad.net

Commit message

Avoid use of deprecated mir EGL related apis.

With the new mesa update, the EGLNativeDisplayType is MirConnection * and the EGLNativeWindowType is a MirRenderSurface *.

Ignore deprecation warnings on use of MirRenderSurface - its "deprecated" because it'll be renamed in mir 1.0.

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-03-31:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2017-05-19:

This would be nicely simplified by lp:~alan-griffiths/mir/deprecation-macros plus -DMIR_DEPRECATE_RENDERSURFACES=0

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2017-05-19:

Looks mostly sensible, but I need to get more familiar with qtubuntu to be sure.

Can we aim to land lp:~alan-griffiths/mir/deprecation-macros and use -DMIR_DEPRECATE_RENDERSURFACES=0 - or building against trunk Mir an issue?

review: Needs Information

Revision history for this message

Gerry Boland (gerboland) wrote on 2017-05-22:

I have a question about how pixel formats work with the newer Mir API. I see you've removed code where Qt could set a desired pixel format on the mir surface - is this feature impossible to replicate?

This was done because sometimes the EGL config chosen may have an alpha buffer enabled, even if Qt doesn't need it. We used the mir pixel format to inform the Mir compositor that blending is not required in that case - a nice optimisation.

review: Needs Information

Revision history for this message

Gerry Boland (gerboland) wrote on 2017-05-22:

I agree with Alan that using his deprecation bits would be nice, but I'd be ok with landing this in the current state if we need this to land first.

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2017-05-22:

Mir currently has "Window" and "RenderSurface" but not "surface". I assume you mean "RenderSurface"?

> This was done because sometimes the EGL config chosen may have an alpha buffer
> enabled, even if Qt doesn't need it. We used the mir pixel format to inform
> the Mir compositor that blending is not required in that case - a nice
> optimisation.

I don't see that in the RS code, but I'm still learning my way around it.

Revision history for this message

Gerry Boland (gerboland) wrote on 2017-05-22:

And the obvious question: can we jump to using non-deprecated APIs in one go?

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2017-05-22:

> And the obvious question: can we jump to using non-deprecated APIs in one go?

It depends.

The reason for the current situation being that we can't introduce the intended "surface" APIs until we delete the current ones.

The RS APIs are a temporary workaround to enable the "surface" functionality without a name clash.

If we decide not to rename MirRenderSurface => MirSurface in 1.0 we can just remove the deprecation of render surface.

Revision history for this message

Gerry Boland (gerboland) wrote on 2017-05-22:

+ // Assume that the buffer size matches the surface size at creation time
I'd rather not make that assumption. Does the new API loose this functionality?

Revision history for this message

Gerry Boland (gerboland) wrote on 2017-05-22:

+bool UbuntuSurface::needsRepaint() const
+{
+ return mNeedsRepaint;

This can totally go away AFAICS, since this changes the buffer "ownership" drastically. Before when we got a surface resize event, we had to keep swapping buffers until we got one sized at the new size - so we had to keep force repainting until that happened. Now since Qt owns the buffer, it can resize immediately.

Removing all the needsRepaint stuff also makes resizing smoother IMO in my tests here.

Revision history for this message

Gerry Boland (gerboland) wrote on 2017-05-22:

overall looks ok, I just
- miss the pixel format optimisation
- would ultimately like to remove any deprecated api use (but not necessarily in this MP)

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2017-05-24:

> I have a question about how pixel formats work with the newer Mir API. I see
> you've removed code where Qt could set a desired pixel format on the mir
> surface - is this feature impossible to replicate?
>
> This was done because sometimes the EGL config chosen may have an alpha buffer
> enabled, even if Qt doesn't need it. We used the mir pixel format to inform
> the Mir compositor that blending is not required in that case - a nice
> optimisation.

The pixel format for EGL rendering is now set through the chosen EGLConfig. The new mir mesa path will actually obey the pixel format selected.

If you want to use mir_pixel_format enums, you can specify a EGL_NATIVE_VISUAL_ID in the attribs list during eglChooseConfig and it'll be respected.

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2017-05-24:

> + // Assume that the buffer size matches the surface size at creation time
> I'd rather not make that assumption. Does the new API loose this
> functionality?

If the size chosen by the server is changed, it now gets delivered as a resize event as long as the event handler was registered through the window spec. This means that the same resizing logic is applied.

I tested this slightly by using the fullscreen window manager option in mir's demo server.

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-05-24:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2017-05-24:

> overall looks ok, I just
> - miss the pixel format optimisation
I'll look into that, I should just be able to use EGL_NATIVE_VISUAL_ID.

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2017-05-24:

> Looks mostly sensible, but I need to get more familiar with qtubuntu to be
> sure.
>
> Can we aim to land lp:~alan-griffiths/mir/deprecation-macros and use
> -DMIR_DEPRECATE_RENDERSURFACES=0 - or building against trunk Mir an issue?

Shouldn't be an issue on a local machine, but qtubuntu's CI would be.

lp://staging/~albaguirre/qtubuntu/use-mir-rs-api updated on 2017-05-27

397. By Alberto Aguirre on 2017-05-27: Cleanup #pragmas and unneeded repaints when resizing.

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-05-27:

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

review: Needs Fixing (continuous-integration)

Unmerged revisions

397. By Alberto Aguirre on 2017-05-27

Cleanup #pragmas and unneeded repaints when resizing.

396. By Alberto Aguirre on 2017-04-06

Avoid using mir_window_get_parameters, it will be deprecated

395. By Alberto Aguirre on 2017-04-05

Remove mir_event_type_key and mir_event_type_motion as they are deprecated.

394. By Alberto Aguirre on 2017-04-05

Use MirRenderSurface for pixmap cursors.

Creating buffer streams directly will be deprecated in mir in the near future, instead use render surfaces.

393. By Alberto Aguirre on 2017-04-05

Update resizing now that client can drive the buffer size through mir_render_surface_set_size.

392. By Alberto Aguirre on 2017-03-31

Avoid use of deprecated mir EGL related apis.

With the new mesa update, the EGLNativeDisplayType is MirConnection and the EGLNativeWindowType is a MirRenderSurface.
Ignore deprecation warnings on use of MirRenderSurface - its "deprecated" because it'll be renamed in mir 1.0.

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

to all changes:

Alberto Aguirre

Daniel d'Andrada

Ubuntu Phablet Team

qtubuntu

Merge lp://staging/~albaguirre/qtubuntu/use-mir-rs-api into lp://staging/qtubuntu

Commit message

Description of the change

Unmerged revisions

Preview Diff

Subscribers