703155 - Port Mozilla Qt to Qt5

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Qt, defect)

Description

[Oleg Romashin (:romaxa)]

Qt 5 has some classes reorganization and don't have anymore Q_WS_X11 and QX11Info.

So in order to compile it on Qt5 we need to tweak existing non-X11 configuration.

Comment 1

[Oleg Romashin (:romaxa)]

Attachment: WIP1

Comment 2

[Oleg Romashin (:romaxa)]

Attachment: Qt5 building and basic working port

Ok, here is the patch which does:
1) Fix includes virtual path in order to keep it compiling with Qt4 and Qt5
2) Get rid of QX11Info as much as possible, and replace with pure X11 implementation (including cached X display in QtPlatform shared for Qt port instance)
3) Qt specific old input method implementation disabled because API has changed. need followup with new implementation

Comment 3

[Oleg Romashin (:romaxa)]

Patch is changing Qt related functionality across different mozilla components.
Should I set multiple reviewers, or it is enough to have just one Qt reviewer here?

Comment 4

[Karl Tomlinson (:karlt)]

Comment on attachment 596924 [details] [diff] [review]
Qt5 building and basic working port

You can have my r+ on the dom/plugins changes.

I didn't really review gfxQtPlatform but it looks like there needs to be a better solution for getting the Display than the XOpenDisplay.  I assume Qt will often be using a different Display.  Both Qt and Gecko need to use the same connection in order avoid race conditions across the different connections.  There is also a limit (about 128 iirc) on the number of connections to a server, so these should not be consumed unnecessarily.

Comment 5

[Oleg Romashin (:romaxa)]

Attachment: Qt5 port

Here is the version with more strict display/screen requests from qt5 xcb/xlib platform interface.

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 598763 [details] [diff] [review]
Qt5 port

Review of attachment 598763 [details] [diff] [review]:
-----------------------------------------------------------------

r+ for the build parts. I really can't review the Qt parts.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 598763 [details] [diff] [review]
Qt5 port

The new screen accessors look fine to me. Looks like you have maintained support for older Qt versions too.

>diff --git a/gfx/cairo/cairo/src/cairo-qt-surface.cpp b/gfx/cairo/cairo/src/cairo-qt-surface.cpp
> #include <QtGui/QPaintDevice>
> #include <QtGui/QImage>
> #include <QtGui/QPixmap>
> #include <QtGui/QBrush>
> #include <QtGui/QPen>
>-#include <QtGui/QWidget>
>-#include <QtGui/QX11Info>
>+#include <QWidget>
> #include <QtCore/QVarLengthArray>

nit: Do you need to drop the "QtGui" folder? Should the others be changed too? Could you keep them all the same?

>diff --git a/widget/qt/nsWindow.cpp b/widget/qt/nsWindow.cpp

>-#include <QtGui/QApplication>
>-#include <QtGui/QDesktopWidget>
>+#include <QApplication>
>+#include <QDesktopWidget>
> #include <QtGui/QCursor>
>-#include <QtGui/QIcon>
>-#include <QtGui/QX11Info>
>-#include <QtGui/QGraphicsScene>
>-#include <QtGui/QGraphicsView>
>-#include <QtGui/QGraphicsSceneContextMenuEvent>
>-#include <QtGui/QGraphicsSceneDragDropEvent>
>-#include <QtGui/QGraphicsSceneMouseEvent>
>-#include <QtGui/QGraphicsSceneHoverEvent>
>-#include <QtGui/QGraphicsSceneWheelEvent>
>-#include <QtGui/QGraphicsSceneResizeEvent>
>-#include <QtGui/QStyleOptionGraphicsItem>
>+#include <QIcon>
>+#include <QGraphicsScene>
>+#include <QGraphicsView>
>+#include <QGraphicsSceneContextMenuEvent>
>+#include <QGraphicsSceneDragDropEvent>
>+#include <QGraphicsSceneMouseEvent>
>+#include <QGraphicsSceneHoverEvent>
>+#include <QGraphicsSceneWheelEvent>
>+#include <QGraphicsSceneResizeEvent>
>+#include <QStyleOptionGraphicsItem>
> #include <QPaintEngine>
>+#include <QMimeData>

You seem to be removing the folders in other files, especially here.

r+ since the code changes seem fine. Think about whether you want to drop the folder from other cairo-qt-surface.cpp includes

Comment 8

[Oleg Romashin (:romaxa)]

I'm dropping QtGui prefix, only for includes which has changed it's location after Qt5 components reorg.

Actually Cairo changes are not supposed to be here, it was pushed recently to upstream and handled in separate bug 726461... I'll remove it from this patch.

Comment 9

[Oleg Romashin (:romaxa)]

Attachment: Qt5 port

Refreshed to latest trunk, and tested on try, N9, beagle board, et.c ;)

Comment 10

[Doug Turner (:dougt)]

Comment on attachment 608592 [details] [diff] [review]
Qt5 port

Review of attachment 608592 [details] [diff] [review]:
-----------------------------------------------------------------

make sure you run this on Try.  No need for bustages.  Thanks Oleg!

Comment 11

[Oleg Romashin (:romaxa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/82fe9c59db85

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/82fe9c59db85