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
11 months 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
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.