Closed
Bug 995411
Opened 10 years ago
Closed 10 years ago
Move APZ code into gfx/layers/apz
Categories
(Core :: Panning and Zooming, defect)
Core
Panning and Zooming
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: kats, Assigned: kats)
References
Details
Attachments
(2 files, 2 obsolete files)
9.24 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
18.98 KB,
patch
|
botond
:
review+
|
Details | Diff | Splinter Review |
I'd like to move the following files into gfx/layers/apz: gfx/layers/composite/APZCTreeManager.{cpp,h} gfx/layers/ipc/AsyncPanZoomController.{cpp,h} gfx/layers/ipc/GestureEventListener.{cpp,h} gfx/layers/ipc/Axis.{cpp,h} gfx/layers/ipc/GeckoContentController.h and maybe also these: widget/xpwidgets/APZCCallbackHelper.{cpp,h} widget/xpwidgets/ActiveElementManager.{cpp,h} (coming in bug 976605) I'm undecided as of yet whether or not we want to have a directory structure within gfx/layers/apz. It might make sense to keep the widget pieces separate from the rest (and there will probably be more widget pieces as time goes on). Additionally some of the structs shoved into FrameMetrics.h (ScrollableLayerGuid and ZoomConstraints) should be moved into gfx/layers/apz as well.
Comment 1•10 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #0) > I'm undecided as of yet whether or not we want to have a directory structure > within gfx/layers/apz. It might make sense to keep the widget pieces > separate from the rest (and there will probably be more widget pieces as > time goes on). I think it makes sense to separate the classes on the parent side (APZC[TM], GEL) from the classes on the child side (APZCCallbackHelper and such) into different directories.
Assignee | ||
Comment 2•10 years ago
|
||
This moves files but breaks the build. (Split patch for easier review, but will land them squashed together)
Assignee: nobody → bugmail.mozilla
Attachment #8406866 -
Flags: review?(jmuizelaar)
Attachment #8406866 -
Flags: review?(botond)
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8406867 -
Flags: review?(jmuizelaar)
Attachment #8406867 -
Flags: review?(botond)
Comment 4•10 years ago
|
||
Comment on attachment 8406866 [details] [diff] [review] Part 1 - Move files into gfx/layers/apz Review of attachment 8406866 [details] [diff] [review]: ----------------------------------------------------------------- This seems fine in principle. ::: gfx/layers/moz.build @@ +102,5 @@ > + 'apz/src/AsyncPanZoomController.h', > + 'apz/src/Axis.h', > + 'apz/src/GestureEventListener.h', > + 'apz/src/TaskThrottler.h', > + 'apz/util/APZCCallbackHelper.h', Does all of this stuff need to be exported?
Attachment #8406866 -
Flags: review?(jmuizelaar) → review+
Updated•10 years ago
|
Attachment #8406867 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Jeff Muizelaar [:jrmuizel] from comment #4) > Comment on attachment 8406866 [details] [diff] [review] > Part 1 - Move files into gfx/layers/apz > > Review of attachment 8406866 [details] [diff] [review]: > ----------------------------------------------------------------- > > This seems fine in principle. > > ::: gfx/layers/moz.build > @@ +102,5 @@ > > + 'apz/src/AsyncPanZoomController.h', > > + 'apz/src/Axis.h', > > + 'apz/src/GestureEventListener.h', > > + 'apz/src/TaskThrottler.h', > > + 'apz/util/APZCCallbackHelper.h', > > Does all of this stuff need to be exported? At the moment, yes, because the exported AsyncPanZoomController.h includes them, and there is code that includes that exported AsyncPanZoomController.h. I can probably do some follow-up work so that we can encapsulate more of this code and not export it.
Assignee | ||
Comment 6•10 years ago
|
||
Unfortunately TBPL is not too happy with my changes. Will need to investigate. https://tbpl.mozilla.org/?tree=Try&rev=66eedd2dcd58
Comment 7•10 years ago
|
||
Comment on attachment 8406866 [details] [diff] [review] Part 1 - Move files into gfx/layers/apz Review of attachment 8406866 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/moz.build @@ +101,5 @@ > + 'apz/src/APZCTreeManager.h', > + 'apz/src/AsyncPanZoomController.h', > + 'apz/src/Axis.h', > + 'apz/src/GestureEventListener.h', > + 'apz/src/TaskThrottler.h', Please add a comment saying that the exporting of things from 'src' is temporary until we extract an interface from APZCTreeManager and put it into 'public' (as discussed on IRC).
Attachment #8406866 -
Flags: review?(botond) → review+
Comment 8•10 years ago
|
||
Comment on attachment 8406866 [details] [diff] [review] Part 1 - Move files into gfx/layers/apz Review of attachment 8406866 [details] [diff] [review]: ----------------------------------------------------------------- Also you'll probably want to move ActiveElementManager.{h,cpp}, which has just landed (on m-i), as well.
Updated•10 years ago
|
Attachment #8406867 -
Flags: review?(botond) → review+
Assignee | ||
Comment 9•10 years ago
|
||
Also updated to move widget/xpwidget/ActiveElementManager, carrying r+ since fundamentally nothing has changed.
Attachment #8406866 -
Attachment is obsolete: true
Attachment #8407740 -
Flags: review+
Assignee | ||
Comment 10•10 years ago
|
||
Updated to include some comments in the moz.build file, also updating namespaces of the things moved over from widget/xpwidget, and fixing various build errors. Try push at https://tbpl.mozilla.org/?tree=Try&rev=4f4e1fb087de and I'll squash these two parts together for landing.
Attachment #8406867 -
Attachment is obsolete: true
Attachment #8407741 -
Flags: review?(botond)
Comment 11•10 years ago
|
||
Comment on attachment 8407741 [details] [diff] [review] Part 2 - Fix up includes and namespaces (v2) Review of attachment 8407741 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/gonk/ParentProcessController.cpp @@ +25,5 @@ > NS_IMETHOD Run() { > MOZ_ASSERT(NS_IsMainThread()); > nsCOMPtr<nsIContent> content = nsLayoutUtils::FindContentFor(mFrameMetrics.GetScrollId()); > if (content) { > + mozilla::layers::APZCCallbackHelper::UpdateSubFrame(content, mFrameMetrics); layers:: would be sufficient (I don't care much though)
Attachment #8407741 -
Flags: review?(botond) → review+
Assignee | ||
Comment 12•10 years ago
|
||
green try |
New try push with more unified build fixes (see latest two dep bugs added): https://tbpl.mozilla.org/?tree=Try&rev=93783f4703b1
Assignee | ||
Comment 13•10 years ago
|
||
landing |
Squashed and landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/f9963d292e66
Assignee | ||
Comment 14•10 years ago
|
||
landing |
Also https://hg.mozilla.org/integration/mozilla-inbound/rev/953430fb3c69 to update the clobber file which I neglected to do.
Comment 15•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f9963d292e66 https://hg.mozilla.org/mozilla-central/rev/953430fb3c69
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•