Closed
Bug 1227706
Opened 8 years ago
Closed 8 years ago
Make each nsWindow/GeckoView manage their own compositor/GLController
Categories
(Core Graveyard :: Widget: Android, defect)
Tracking
(firefox46 fixed)
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: jchen, Assigned: jchen)
References
Details
Attachments
(7 files)
8.07 KB,
patch
|
snorp
:
review+
|
Details | Diff | Splinter Review |
25.57 KB,
patch
|
snorp
:
review+
|
Details | Diff | Splinter Review |
16.79 KB,
patch
|
snorp
:
review+
|
Details | Diff | Splinter Review |
21.71 KB,
patch
|
snorp
:
review+
|
Details | Diff | Splinter Review |
15.41 KB,
patch
|
snorp
:
review+
|
Details | Diff | Splinter Review |
8.96 KB,
patch
|
snorp
:
review+
|
Details | Diff | Splinter Review |
13.86 KB,
patch
|
snorp
:
review+
|
Details | Diff | Splinter Review |
Each nsWindow should have its own compositor, and each GeckoView should manage its own GLController instance, because GLController is responsible for creating/pausing/resuming the compositor.
Assignee | ||
Comment 1•8 years ago
|
||
GLController will no longer be a singleton, but we should keep initializing EGL only once, so this patch makes EGL initialization static.
Attachment #8698188 -
Flags: review?(snorp)
Assignee | ||
Comment 2•8 years ago
|
||
GeckoViewSupport better reflects the purpose of the class and will match the GLControllerSupport class that another patch is adding. This patch also changes the way GeckoViewSupport is constructed in order to be more encapsulating.
Attachment #8698189 -
Flags: review?(snorp)
Assignee | ||
Comment 3•8 years ago
|
||
One nsWindow will have one corresponding GLController, and using native GLController methods instead of GeckoEvents lets us control the compositor for each nsWindow separately.
Attachment #8698190 -
Flags: review?(snorp)
Assignee | ||
Comment 4•8 years ago
|
||
GLController instances are associated with a particular nsWindow, rather than a particular View. Therefore, we need to let GeckoView manage GLController instances, as part of GeckoView's handling of saving and restoring states.
Attachment #8698191 -
Flags: review?(snorp)
Assignee | ||
Comment 5•8 years ago
|
||
This patch adds native method implementations in GLControllerSupport to manage compositor creation/pause/resume.
Attachment #8698192 -
Flags: review?(snorp)
Assignee | ||
Comment 6•8 years ago
|
||
Now that we properly support individual compositors for nsWindows, we should get rid of the static singletons that held the compositor objects.
Attachment #8698193 -
Flags: review?(snorp)
Assignee | ||
Comment 7•8 years ago
|
||
Remove GLController calls and events in GeckoAppShell and GeckoEvent that were made obsolete by the new native calls.
Attachment #8698194 -
Flags: review?(snorp)
Attachment #8698188 -
Flags: review?(snorp) → review+
Attachment #8698189 -
Flags: review?(snorp) → review+
Attachment #8698190 -
Flags: review?(snorp) → review+
Attachment #8698191 -
Flags: review?(snorp) → review+
Comment on attachment 8698192 [details] [diff] [review] Implement nsWindow::GLControllerSupport (v1) Review of attachment 8698192 [details] [diff] [review]: ----------------------------------------------------------------- This seems ok, but please test very thoroughly. This stuff is fragile. ::: widget/android/nsWindow.cpp @@ +448,5 @@ > + template<class Functor> > + static void OnNativeCall(Functor&& aCall) > + { > + if (aCall.IsTarget(&GLControllerSupport::CreateCompositor) || > + aCall.IsTarget(&GLControllerSupport::PauseCompositor)) { Fix indentation
Attachment #8698192 -
Flags: review?(snorp) → review+
Comment on attachment 8698193 [details] [diff] [review] Get rid of compositor singletons in nsWindow (v1) Review of attachment 8698193 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/android/nsWindow.cpp @@ +3018,5 @@ > return sWidgetPaintsBackground; > } > > bool > nsWindow::NeedsPaint() I don't even think this really gets used anymore...
Attachment #8698193 -
Flags: review?(snorp) → review+
Attachment #8698194 -
Flags: review?(snorp) → review+
Comment 10•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/cdae21cb55aa https://hg.mozilla.org/integration/mozilla-inbound/rev/fca23dbf60cc https://hg.mozilla.org/integration/mozilla-inbound/rev/1ebeb391981b https://hg.mozilla.org/integration/mozilla-inbound/rev/094dca979346 https://hg.mozilla.org/integration/mozilla-inbound/rev/23411cbf19f1 https://hg.mozilla.org/integration/mozilla-inbound/rev/2ec824e55926 https://hg.mozilla.org/integration/mozilla-inbound/rev/425588347628
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/cdae21cb55aa https://hg.mozilla.org/mozilla-central/rev/fca23dbf60cc https://hg.mozilla.org/mozilla-central/rev/1ebeb391981b https://hg.mozilla.org/mozilla-central/rev/094dca979346 https://hg.mozilla.org/mozilla-central/rev/23411cbf19f1 https://hg.mozilla.org/mozilla-central/rev/2ec824e55926 https://hg.mozilla.org/mozilla-central/rev/425588347628
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•