[Merge] lp:~gerboland/qtubuntu/add--more-surface-type-support into lp:qtubuntu
Nick Dedekind
nick.dedekind at canonical.com
Wed Nov 2 10:44:24 UTC 2016
Review: Needs Fixing
Couple of comments.
Diff comments:
> === modified file 'src/ubuntumirclient/input.cpp'
> --- src/ubuntumirclient/input.cpp 2016-09-21 10:25:33 +0000
> +++ src/ubuntumirclient/input.cpp 2016-10-26 11:43:06 +0000
> @@ -501,6 +501,8 @@
> const auto localPoint = QPointF(mir_pointer_event_axis_value(pev, mir_pointer_axis_x),
> mir_pointer_event_axis_value(pev, mir_pointer_axis_y));
>
> + mLastInputWindow = platformWindow;
The rest of the events relating to mLastInputWindow only set on down event (key down/touch action down). Perhaps only on mir_pointer_action_button_down? I don't think we want it to change if we release over a different window than we pressed (if that can actually happen).
case mir_pointer_action_button_down:
mLastInputWindow = platformWindow;
case mir_pointer_action_button_up:
case mir_pointer_action_motion:
{
> +
> switch (action) {
> case mir_pointer_action_button_up:
> case mir_pointer_action_button_down:
>
> === modified file 'src/ubuntumirclient/window.cpp'
> --- src/ubuntumirclient/window.cpp 2016-10-26 11:43:06 +0000
> +++ src/ubuntumirclient/window.cpp 2016-10-26 11:43:06 +0000
> @@ -165,51 +204,93 @@
> return parent ? static_cast<UbuntuWindow *>(parent->handle()) : nullptr;
> }
>
> -Spec makeSurfaceSpec(QWindow *window, UbuntuInput *input, MirPixelFormat pixelFormat, MirConnection *connection)
> -{
> - const auto geom = window->geometry();
> - const int width = geom.width() > 0 ? geom.width() : 1;
> - const int height = geom.height() > 0 ? geom.height() : 1;
> +bool requiresParent(const MirSurfaceType type)
> +{
> + switch (type) {
> + case mir_surface_type_dialog: //FIXME - not quite what the specification dictates, but is what Mir's api dictates
> + case mir_surface_type_utility:
> + case mir_surface_type_gloss:
> + case mir_surface_type_menu:
> + case mir_surface_type_satellite:
> + case mir_surface_type_tip:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +bool requiresParent(const Qt::WindowType type)
> +{
> + return requiresParent(qtWindowTypeToMirSurfaceType(type));
> +}
> +
> +bool isMovable(const Qt::WindowType type)
> +{
> + auto mirType = qtWindowTypeToMirSurfaceType(type);
> + switch (mirType) {
> + case mir_surface_type_menu:
> + case mir_surface_type_tip:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +Spec makeSurfaceSpec(QWindow *window, MirPixelFormat pixelFormat, UbuntuWindow *parentWindowHandle,
> + MirConnection *connection)
> +{
> + const auto geometry = window->geometry();
> + const int width = geometry.width() > 0 ? geometry.width() : 1;
> + const int height = geometry.height() > 0 ? geometry.height() : 1;
> + auto type = qtWindowTypeToMirSurfaceType(window->type());
>
> if (U_ON_SCREEN_KEYBOARD_ROLE == roleFor(window)) {
> - qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p) - creating input method surface (width=%d, height=%d", window, width, height);
> - return Spec{mir_connection_create_spec_for_input_method(connection, width, height, pixelFormat)};
> - }
> -
> - const Qt::WindowType type = window->type();
> - if (type == Qt::Popup) {
> - auto parent = transientParentFor(window);
> - if (parent == nullptr) {
> - //NOTE: We cannot have a parentless popup -
> - //try using the last surface to receive input as that will most likely be
> - //the one that caused this popup to be created
> - parent = input->lastInputWindow();
> - }
> - if (parent) {
> - auto pos = geom.topLeft();
> - pos -= parent->geometry().topLeft();
> - MirRectangle location{pos.x(), pos.y(), 0, 0};
> - qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p) - creating menu surface(width:%d, height:%d)", window, width, height);
> - return Spec{mir_connection_create_spec_for_menu(
> - connection, width, height, pixelFormat, parent->mirSurface(),
> - &location, mir_edge_attachment_any)};
> - } else {
> - qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p) - cannot create a menu without a parent!", window);
> - }
> - } else if (type == Qt::Dialog) {
> - auto parent = transientParentFor(window);
> - if (parent) {
> - // Modal dialog
> - qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p) - creating modal dialog (width=%d, height=%d", window, width, height);
> - return Spec{mir_connection_create_spec_for_modal_dialog(connection, width, height, pixelFormat, parent->mirSurface())};
> - } else {
> - // TODO: do Qt parentless dialogs have the same semantics as mir?
> - qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p) - creating parentless dialog (width=%d, height=%d)", window, width, height);
> - return Spec{mir_connection_create_spec_for_dialog(connection, width, height, pixelFormat)};
> - }
> - }
> - qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p) - creating normal surface(type=0x%x, width=%d, height=%d)", window, type, width, height);
> - return Spec{mir_connection_create_spec_for_normal_surface(connection, width, height, pixelFormat)};
> + type = mir_surface_type_inputmethod;
> + }
> +
> + MirRectangle location{geometry.x(), geometry.y(), 0, 0};
> + MirSurface *parent = nullptr;
> + if (parentWindowHandle) {
> + parent = parentWindowHandle->mirSurface();
> + // Qt uses absolute positioning, but Mir positions surfaces relative to parent.
> + location.top -= parentWindowHandle->geometry().top();
> + location.left -= parentWindowHandle->geometry().left();
> + }
> +
> + Spec spec;
> +
> + switch (type) {
> + case mir_surface_type_menu:
> + spec = Spec{mir_connection_create_spec_for_menu(connection, width, height, pixelFormat, parent,
> + &location, mir_edge_attachment_any)};
> + break;
> + case mir_surface_type_dialog:
> + spec = Spec{mir_connection_create_spec_for_modal_dialog(connection, width, height, pixelFormat, parent)};
> + break;
> + case mir_surface_type_utility:
> + spec = Spec{mir_connection_create_spec_for_dialog(connection, width, height, pixelFormat)};
> + break;
> + case mir_surface_type_tip:
> +#if MIR_CLIENT_VERSION < MIR_VERSION_NUMBER(3, 4, 0)
Have we got any release targets where we're not publishing new versions of qtubuntu without new versions of mirclient? Can't just bump req in debian/control?
> + spec = Spec{mir_connection_create_spec_for_tooltip(connection, width, height, pixelFormat, parent,
> + &location)};
> +#else
> + spec = Spec{mir_connection_create_spec_for_tip(connection, width, height, pixelFormat, parent,
> + &location, mir_edge_attachment_any)};
> +#endif
> + break;
> + case mir_surface_type_inputmethod:
> + spec = Spec{mir_connection_create_spec_for_input_method(connection, width, height, pixelFormat)};
> + break;
> + default:
> + spec = Spec{mir_connection_create_spec_for_normal_surface(connection, width, height, pixelFormat)};
> + break;
> + }
> +
> + qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p): %s spec (type=0x%x, position=(%d, %d)px, size=(%dx%d)px)",
> + window, mirSurfaceTypeToStr(type), window->type(), location.left, location.top, width, height);
> +
> + return std::move(spec);
> }
>
> void setSizingConstraints(MirSurfaceSpec *spec, const QSize& minSize, const QSize& maxSize, const QSize& increment)
> @@ -759,13 +859,17 @@
> void UbuntuWindow::setGeometry(const QRect &rect)
> {
> QMutexLocker lock(&mMutex);
> +
> + if (window()->windowState() == Qt::WindowFullScreen || window()->windowState() == Qt::WindowMaximized) {
> + qCDebug(ubuntumirclient, "setGeometry(window=%p) - not resizing, window is maximized or fullscreen", window());
> + return;
> + }
> +
> qCDebug(ubuntumirclient, "setGeometry (window=%p, position=(%d, %d)dp, size=(%dx%d)dp)",
> window(), rect.x(), rect.y(), rect.width(), rect.height());
> -
> - //NOTE: mir surfaces cannot be moved by the client so ignore the topLeft coordinates
> - const auto newSize = rect.size();
> -
> - mSurface->resize(newSize);
> + QPlatformWindow::setGeometry(rect); // Immediately update internal geometry so Qt believes position updated
I thought this was removed so that we didn't update the qt geometry until mir swapped the buffers
> +
> + mSurface->updateGeometry(rect);
> // Note: don't call handleGeometryChange here, wait to see what Mir replies with.
> }
>
--
https://code.launchpad.net/~gerboland/qtubuntu/add--more-surface-type-support/+merge/308367
Your team Ubuntu Phablet Team is subscribed to branch lp:qtubuntu.
More information about the Ubuntu-reviews
mailing list