Closed
Bug 701830
Opened 13 years ago
Closed 13 years ago
Rescaling at new resolution after zoom does not occur
Categories
(Firefox for Android Graveyard :: General, defect, P1)
Tracking
(firefox11 fixed, fennec11+)
VERIFIED
FIXED
People
(Reporter: nhirata, Assigned: pcwalton)
References
Details
(Whiteboard: [testday-20111111])
Attachments
(1 obsolete file)
1. go to the preferences
2. select reformat text on zoom
3. go to a website with some text and zoom
Expected: no pixelation on the text
Actual: text does not get reformatted on zoom
Note:
1. known issue I believe, didn't see a bug on it.
Reporter | ||
Updated•13 years ago
|
Whiteboard: [testday-20111111]
Updated•13 years ago
|
Assignee: nobody → pwalton
Summary: Reformat text on zoom does not occur → Re-rendering of text at new resolution after zoom does not occur
Updated•13 years ago
|
Depends on: native_droid_zooming
Assignee | ||
Comment 2•13 years ago
|
||
This isn't just text, it's any content. Working on this now.
Assignee | ||
Updated•13 years ago
|
Summary: Re-rendering of text at new resolution after zoom does not occur → Rescaling at new resolution after zoom does not occur
Comment 4•13 years ago
|
||
Comment on attachment 573993 [details] [diff] [review]
WIP patch.
>@@ -945,16 +946,18 @@ abstract public class GeckoApp
> tab.updateTitle(title);
> loadFavicon(tab);
>
> mMainHandler.post(new Runnable() {
> public void run() {
> if (Tabs.getInstance().isSelectedTab(tab))
> mBrowserToolbar.setTitle(tab.getDisplayTitle());
> onTabsChanged();
>+
>+ mSoftwareLayerClient.geckoLoadedNewContent();
> }
> });
> }
I'm not sure this is the best place for this call. It's ok for now, but we might need something a little earlier, when gecko first starts loading the new content. This will only get triggered after the page is loaded.
>+ private PanZoomPosition mGeckoPanZoomPosition;
>+ /* The pan/zoom position that Gecko is currently displaying. If null, we don't know. */
>+ private PanZoomPosition mPendingPanZoomPosition;
>+ /*
>+ * The pan/zoom position that we're telling Gecko to display. If this is non-null, Gecko is
>+ * currently busy panning or zooming to this location.
>+ */
>+ private PanZoomPosition mQueuedPanZoomPosition;
>+ /*
>+ * The pan/zoom position we're waiting to send to Gecko. It will be sent once
>+ * Gecko finishes panning to mPendingPanZoomPosition.
>+ */
>+ private boolean mDrawPending;
Could you place the comments for the fields above the field, rather than below? This is inconsistent and much more confusing :/
> public void endDrawing(int x, int y, int width, int height) {
>+ if (mGeckoPanZoomPosition == null) {
>+ /*
>+ * Ignore the draw. We don't know where Gecko is currently panned or zoomed to, so we
>+ * can't safely draw.
>+ */
>+ mDrawPending = true;
>+ return;
>+ }
>+
>+ Log.e("Fennec", "### Drawing at " + mGeckoPanZoomPosition.toJSON());
>+
> LayerController controller = getLayerController();
>- //controller.unzoom(); /* FIXME */
>+ float zoomFactor = controller.getZoomFactor();
>+ controller.unzoom();
> controller.notifyViewOfGeometryChange();
>
>- mViewportController.setVisibleRect(mGeckoVisibleRect);
>-
>- if (mGeckoVisibleRect != null) {
>- RectF layerRect = mViewportController.untransformVisibleRect(mGeckoVisibleRect,
>- getPageSize());
>- mTileLayer.origin = new PointF(layerRect.left, layerRect.top);
>- }
>+ //mViewportMetrics.setPosition(mGeckoPanZoomPosition);
>+ mTileLayer.origin = mGeckoPanZoomPosition.untransformPoint();
>
> repaint(new Rect(x, y, x + width, y + height));
Don't really need a separate method for repaint(); can be inlined.
>+
>+ mDrawPending = false;
>+
>+ mPendingPanZoomPosition = null;
>+ if (mQueuedPanZoomPosition != null) {
>+ //PanZoomPosition position = mQueuedPanZoomPosition.scale(1.0f / zoomFactor);
>+ mQueuedPanZoomPosition = null;
>+ //sendPanZoomPositionToGecko(position);
>+ } else if (mDrawPending) {
>+ mDrawPending = false;
>+ GeckoAppShell.scheduleRedraw();
>+ }
> }
I assume the commented-out lines here will be uncommented eventually.
Also, how can mDrawPending be true here? It was set to false just a few lines above.
> /**
> * Returns true if this controller is fine with performing a redraw operation or false if it
> * would prefer that the action didn't take place.
> */
> public boolean getRedrawHint() {
>- return aboutToCheckerboard();
>+ return aboutToCheckerboard() ||
>+ (getZoomFactor() != 1.0f && mPanZoomController.getRedrawHint());
> }
I don't understand why zoom factor is checked here.
>+ public boolean getRedrawHint() {
>+ switch (mState) {
>+ case NOTHING:
>+ case TOUCHING:
>+ case PANNING_HOLD:
>+ return true;
>+ case FLING:
>+ case PANNING:
>+ case PINCHING:
>+ return false;
>+ }
>+
>+ throw new RuntimeException("Unknown state!");
>+ }
>+
I'd prefer a log+graceful handling here rather than an exception.
>+
>+ /** Returns a new zoom position, with the offset and zoom scaled by the given factor. */
>+ public PanZoomPosition scale(float factor) {
>+ return new PanZoomPosition(new PointF(x * factor, y * factor), zoom * factor);
>+ }
indent should be 4 spaces.
Also something I noticed was that in a bunch of places you do == comparisons on floats. I'm not sure that's a good idea, I'd prefer to see Math.abs(x-y) < epsilon to be robust against floating-point rounding errors.
Attachment #573993 -
Flags: feedback?(kgupta) → feedback+
Updated•13 years ago
|
Priority: -- → P1
Comment 5•13 years ago
|
||
FYI: This is currently being considered a blocker for releasing the Native UI nightly to Test Drivers.
Assignee | ||
Updated•13 years ago
|
Attachment #573993 -
Attachment is obsolete: true
Assignee | ||
Comment 6•13 years ago
|
||
Patches for this are in bug 703141.
Assignee | ||
Comment 7•13 years ago
|
||
Fixed with the landing of bug 703141.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 8•13 years ago
|
||
Verified fixed on: Mozilla /5.0 (Android;Linux armv7l;rv:11.0a1) Gecko/20111124 Firefox/11.0a1 Fennec/11.0a1
Device: LG Optimus 2X (Android 2.2)
Status: RESOLVED → VERIFIED
Updated•13 years ago
|
tracking-fennec: --- → 11+
Updated•13 years ago
|
status-firefox11:
--- → fixed
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•