974335 - Refactor Qt Widget Backend implementation

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Qt, defect)

Description

[Oleg Romashin (:romaxa)]

Qt widget backend become non-compilable in recent mozilla-central, so I decided to make it work again, also cleanup implementation, drop Qt4 support, and switch it completely to QWindow based implementation which is available in current Qt5 core base  library.

Also it will remove dependency on Qt5Widget, which is kind of deprecated.

Comment 1

[Oleg Romashin (:romaxa)]

Attachment: Configure and make file changes

Comment 2

[Oleg Romashin (:romaxa)]

Attachment: Remove Qt includes and dependency from plugins

First of all it does not compile due to unified sources, at second we don't care that much about plugins now

Comment 3

[Oleg Romashin (:romaxa)]

Attachment: GLX/EGL Provider fixes for Qt backend

Mainly build fixes, and adaptation to new Qt backend implementation

Comment 4

[Oleg Romashin (:romaxa)]

Attachment: Shared Widget changes

Build fix, drop Qt4 dependency and QtWidgets library. Switch world to QWindow

Comment 5

[Oleg Romashin (:romaxa)]

Attachment: Qt Build only changes, Drop Qt4, QtWidget dependency, switch to QWindow

Not sure if this need to be reviewed, it is not part of default build...
But if Doug could look at it overall, would be nice

Comment 6

[Oleg Romashin (:romaxa)]

Here is try build with all patches applied, looks good so far:
https://tbpl.mozilla.org/?tree=Try&rev=def59a5b0a7d

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

If this is really all the Qt-specific changes that you need to make under gfx/gl, then I'm very pleased, this is a lot better than the old code which had to do contortions to reuse Qt's existing GL context.

Comment 8

[Oleg Romashin (:romaxa)]

Attachment: Make Qt backend build same skia files as GTK backend

Comment 9

[George Wright (:gw280) (needinfo me!)]

Comment on attachment 8378454 [details] [diff] [review]
Make Qt backend build same skia files as GTK backend

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

you need to modify generate_mozbuild.py now rather than moz.build directly, as moz.build is autogenerated.

Comment 10

[Oleg Romashin (:romaxa)]

> as moz.build is autogenerated.

Probably moz.build should say that in the beginning of the file

Comment 11

[Oleg Romashin (:romaxa)]

Attachment: Make Qt backend build same skia files as GTK backend

Comment 12

[Mike Hommey [:glandium]]

Comment on attachment 8378322 [details] [diff] [review]
Configure and make file changes

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

::: configure.in
@@ +4430,5 @@
>      if test -z "$QTDIR"; then
>          case $QT_VERSION in
>          5.*)
>              AC_MSG_RESULT("Using qt5: $QT_VERSION")
> +            PKG_CHECK_MODULES(MOZ_QT, Qt5Gui Qt5Network Qt5Core Qt5Quick, ,

modules="Qt5Gui Qt5Network Qt5Core Qt5Quick"
if test -n "$NS_PRINTING"; then
    modules="$modules Qt5PrintSupport"
fi
PKG_CHECK_MODULES(MOZ_QT, $modules,
...

@@ +4468,3 @@
>              MOZ_QT_CFLAGS="$MOZ_QT_CFLAGS -I$QTDIR/include/QtGui/$QT_VERSION/QtGui"
> +            if test "$NS_PRINTING"; then
> +                MOZ_QT_LIBS="$MOZ_QT_LIBS -lQt5Widgets -lQt5PrintSupport"

You're putting Qt5Widgets here, but not int the pkg-config version.

::: dom/src/geolocation/Makefile.in
@@ +8,3 @@
>  CXXFLAGS += $(MOZ_QT_CFLAGS)
>  endif
> +

trailing newline

Comment 13

[Oleg Romashin (:romaxa)]

Attachment: Configure and make file changes.

> > +            if test "$NS_PRINTING"; then
> > +                MOZ_QT_LIBS="$MOZ_QT_LIBS -lQt5Widgets -lQt5PrintSupport"
> 
> You're putting Qt5Widgets here, but not int the pkg-config version.

main requirement here is Qt5PrintSupport, Qt5PrintSupport.pc does  pull Qt5Widgets dependency automatically, that is why I'm not pointing that dependency there.

In theory would be nice to get Printing without Qt5Widgets dependency... but that probably long term plan.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8378324 [details] [diff] [review]
Remove Qt includes and dependency from plugins

I'm not a good person for reviewing this. Although, it looks fine.

Josh, are you managing around here?

Comment 15

[Doug Turner (:dougt)]

Comment on attachment 8378332 [details] [diff] [review]
Shared Widget changes

lgtm

Comment 16

[Doug Turner (:dougt)]

Comment on attachment 8378335 [details] [diff] [review]
Qt Build only changes, Drop Qt4, QtWidget dependency, switch to QWindow

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

::: dom/plugins/test/testplugin/nptest_qt.cpp
@@ +33,4 @@
>  #include "nptest_platform.h"
>  #include "npapi.h"
>  
>  struct _PlatformData {

Is this struct needed?

@@ +61,2 @@
>    printf("NPERR_INCOMPATIBLE_VERSION_ERROR\n");
>    return NPERR_INCOMPATIBLE_VERSION_ERROR;

does this plugin just never load now?

::: dom/system/unix/nsHapticFeedback.cpp
@@ -1,1 @@
>  /* -*- Mode: c++; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-

this file is QT only now.  Having it in dom/system/unix is probalby wrong.

Please rename dom/system/unix/nsHapticFeedback.cpp to dom/system/unix/QTHapticFeedback.cpp.

You many need to create a stub nsHapticFeedback.cpp.

Comment 17

[Oleg Romashin (:romaxa)]

Attachment: [NPDB] Qt Build only changes, Drop Qt4, QtWidget dependency, switch to QWindow

Comment 18

[Oleg Romashin (:romaxa)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/9b14f95eac89
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/9531922178e3
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/269675d98f5d
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4f4574e6f08e
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/acab9b9fc4da
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/c5953f75569b

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9b14f95eac89
https://hg.mozilla.org/mozilla-central/rev/9531922178e3
https://hg.mozilla.org/mozilla-central/rev/269675d98f5d
https://hg.mozilla.org/mozilla-central/rev/4f4574e6f08e
https://hg.mozilla.org/mozilla-central/rev/acab9b9fc4da
https://hg.mozilla.org/mozilla-central/rev/c5953f75569b