Closed Bug 431843 Opened 15 years ago Closed 15 years ago

Panning - smooth movement

Categories

(Firefox for Android Graveyard :: Panning/Zooming, enhancement, P1)

Product:
Firefox for Android Graveyard
Component:
Panning/Zooming
Platform:
Other
All
Type:
enhancement

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
fennec1.0m4

People

(Reporter: christian.bugzilla, Assigned: Gavin)

References

Details

Attachments

(1 file, 1 obsolete file)

updated to comments
51.49 KB, patch
Details | Diff | Splinter Review
Assignee: nobody → gavin.sharp
Priority: -- → P1
Target Milestone: --- → Fennec M4
Flags: blocking-fennec1.0+
Attached patch patch (obsolete) — — Splinter Review 
This patch has two main parts:
1) removal of the deckpage/tab support (r?mfinkle)
2) implementation of canvas-based pan/zoom based on stuart's PoC (r?stuart)

Stuart's original proof of concept used a hardcoded width/height canvas/browser, but I've switched to using a flex-based approach, to support different windows sizes, and resizes. This required an ugly hack to let the canvas know what it's real size is, and using a deck that will flex instead of just two stacks, but it doesn't seem to negatively affect performance, so I figured we can revisit later. margins are used to move the canvas instead of left/right to avoid triggering absolute positioning layout, which seems to be slower.
Attachment #326007 - Flags: review?(pavlov)
Attachment #326007 - Flags: review?(mark.finkle)
Comment on attachment 326007 [details] [diff] [review]
patch

>diff -r 2231628c6a5d app/mobile.js

>+pref("browser.ui.cursor", true);

Ignore this change, of course...
Comment on attachment 326007 [details] [diff] [review]
patch

>diff -r 2231628c6a5d app/mobile.js

>-pref("browser.ui.cursor", false);
>+pref("browser.ui.cursor", true);

Do you want to enable the cursor on the device? or is this just from your PC setup?

>diff -r 2231628c6a5d chrome/content/browser.js

>-function ProgressController(aBrowser) {

>+
>+    // FIXME: until we can get proper canvas repainting hooked up, update the canvas every 300ms
>+    //var tabbrowser = this._tabbrowser;
>+    //setTimeout(function () {
>+    //  tabbrowser.updateCanvasState();
>+    //}, 300);

Yeah, this is short term, but deckbrowser might have a better place for it. No need to let this "detail" leak from the deckbrowser impl.

>   },
> 
>   onStateChange : function(aWebProgress, aRequest, aStateFlags, aStatus) {
>@@ -355,13 +340,14 @@
>             LocationBar.update(TOOLBARSTATE_LOADING);
>           if (HUDBar)
>             HUDBar.update(TOOLBARSTATE_LOADING);
>+          this._tabbrowser.updateCanvasState();

Ok, now I see the issue - we have to leak other things too. We should eventually move this progresscontroller into deckbrowser.


>diff -r 2231628c6a5d chrome/content/deckbrowser.xml

Wow, the way that diff was intermingled was crazy - but it looked ok to me


There are some <command>s you could remove too, right?
Attachment #326007 - Flags: review?(mark.finkle) → review+
(In reply to comment #3)
> Do you want to enable the cursor on the device? or is this just from your PC
> setup?

Yeah, this is just a local change that snuck in.

> Ok, now I see the issue - we have to leak other things too. We should
> eventually move this progresscontroller into deckbrowser.

Could do - not sure it's worth duplicating progresscontrollers just to separate deckbrowser and the rest of the fennec code, though.

> There are some <command>s you could remove too, right?

Could be, will look into that.

Also need to investigate a problem romaxa pointed out with panning on hs.fi (constraint code definitely needs some tweaking).
Comment on attachment 326007 [details] [diff] [review]
patch

this looks mostly right -- i'll do a more full review of this code when i get back from vacation
Attachment #326007 - Flags: review?(pavlov) → review+
- Fixes hs.fi, by using the maximum scrollable width of <html> or <body> (same as the existing ZoomController code)
- Removes unneeded <command>s
Attachment #326007 - Attachment is obsolete: true
Pushed to mobile-browser:
http://hg.mozilla.org/mobile-browser/index.cgi/rev/d31f553a503a

Going to call this fixed, still work to be done in followups though.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
this has been in for a while and verified with beta3
Status: RESOLVED → VERIFIED
Component: General → Panning/Zooming
Depends on: 546352
You need to log in before you can comment on or make changes to this bug.