Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Move QGL includes into isolated wrapper class, and fix qgl.h and GLDefs.h conflicts

RESOLVED FIXED in mozilla17

Status

Core Graveyard
Widget: Qt
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: romaxa, Assigned: romaxa)

Tracking

unspecified
mozilla17
x86
Linux

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Hacks with gl defines and includes order does not work very well everywhere.
in order to avoid include problems, I think it is better just move all QGL includes out of mozilla includes scope.
(Assignee)

Comment 1

5 years ago
Created attachment 648227 [details] [diff] [review]
WIP, need to test on different build env
Assignee: nobody → romaxa
Status: NEW → ASSIGNED
(Assignee)

Comment 2

5 years ago
Created attachment 648233 [details] [diff] [review]
Fix QGL GLDefs includes conflict by moving them into isolated class

Harmattan,Ubuntu-arm works fine
Attachment #648227 - Attachment is obsolete: true
(Assignee)

Comment 3

5 years ago
Created attachment 648395 [details] [diff] [review]
Fix QGL GLDefs includes conflict by moving them into isolated class

NPODB, but not one liner,  I guess it make sense to double check it...
Tested on desktop, maemo, ubuntu-arm linux
Attachment #648233 - Attachment is obsolete: true
Attachment #648395 - Flags: review?(doug.turner)

Comment 4

5 years ago
Comment on attachment 648395 [details] [diff] [review]
Fix QGL GLDefs includes conflict by moving them into isolated class

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

::: widget/qt/faststartupqt/Makefile.in
@@ +22,5 @@
>  	$(NULL)
>  
> +LOCAL_INCLUDES += -I$(topsrcdir)/xpcom/build \
> +	-I$(topsrcdir)/widget/qt \
> +	$(NULL)

Can you align all of these?

LOCAL_INCLUDES += -I$(topsrcdir)/xpcom/build \
                  -I$(topsrcdir)/widget/qt \
                  $(NULL)

@@ +27,3 @@
>  
> +GARBAGE += moziqwidget.h nsQAppInstance.h nsQAppInstance.cpp mozqglwidgetwrapper.h mozqglwidgetwrapper.cpp
> +export:: $(topsrcdir)/widget/qt/moziqwidget.h $(topsrcdir)/toolkit/xre/nsQAppInstance.h $(topsrcdir)/toolkit/xre/nsQAppInstance.cpp $(topsrcdir)/widget/qt/mozqglwidgetwrapper.h $(topsrcdir)/widget/qt/mozqglwidgetwrapper.cpp

This line is getting pretty long, can you create multiple lines?

::: widget/qt/mozqglwidgetwrapper.h
@@ +1,2 @@
> +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
> +/* vim: set ts=4 et sw=4 tw=80: */

Vi line?  you don't use vim.  remove this.
Attachment #648395 - Flags: review?(doug.turner) → review+
(Assignee)

Comment 5

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/da1a2c7ecb4d
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 6

5 years ago
ups, not yet
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/da1a2c7ecb4d
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/84c6d08bef5b
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.