Closed
Bug 993004
Opened 10 years ago
Closed 10 years ago
[B2G][Dialer][Callbar]The callbar is invisible when viewed in landscape orientation
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
People
(Reporter: JMercado, Assigned: nical)
References
Details
(Keywords: regression)
Attachments
(3 files, 1 obsolete file)
Description: When viewed in landscape orientation, the callbar is invisible. This does not prevent the user from interacting with it to return to the dialer. Repro Steps: 1) Update a buri to BuildID: 20140407000203 2) Make a call 3) Open an application that can be viewed in landscape orientation (such as the browser) 4) View the callbar in landscape orientation Actual: The callbar is invisible. Expected: The callbar can be seen and provides information about the call in progress. 1.4 Environmental Variables: Device: buri 1.4 MOZ BuildID: 20140407000203 Gaia: 86de7fcce674ef6196d68e7e23552d219a3d72db Gecko: 6e028297be14 Version: 30.0a2 Firmware Version: v1.2-device.cfg Notes: Repro frequency: 100% See attached: screenshot, logcat
Reporter | ||
Comment 1•10 years ago
|
||
Reporter | ||
Comment 2•10 years ago
|
||
This issue does not occur on the 1.3 Buri MOZ RIL 1.3 Environmental Variables: Device: buri 1.3 MOZ BuildID: 20140401164001 Gaia: c5cd3a11e91339163b351d50769eaca0067da860 Gecko: 5045a67b47ed Version: 28.0 Firmware Version: v1.2-device.cfg The callbar appears when viewed in landscape mode.
Reporter | ||
Updated•10 years ago
|
status-b2g-v1.3:
--- → unaffected
Updated•10 years ago
|
blocking-b2g: --- → 1.4?
Component: Gaia::Dialer → Gaia::System::Window Mgmt
Keywords: regression,
regressionwindow-wanted
Updated•10 years ago
|
QA Contact: jzimbrick
Comment 3•10 years ago
|
||
Mozilla-Inbound Regression Window: Last Working Environmental Variables: Device: Buri BuildID: 20140311145711 Gaia: 3005269d4dcabcc7d27eaf72bda44a969873af8c Gecko: aab0b6f3e345 Version: 30.0a1 Base Image: V1.2-device.cfg First Broken Environmental Variables: Device: Buri BuildID: 20140311152010 Gaia: 3005269d4dcabcc7d27eaf72bda44a969873af8c Gecko: 499a25915134 Version: 30.0a1 Base Image: V1.2-device.cfg The Gaia ID is the same on both builds, indicating a Gecko issue. Gecko Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=aab0b6f3e345&tochange=499a25915134
Keywords: regressionwindow-wanted
Comment 4•10 years ago
|
||
Tiling regression.
Blocks: 981794
Component: Gaia::System::Window Mgmt → Graphics: Layers
Product: Firefox OS → Core
Version: unspecified → 30 Branch
Comment 6•10 years ago
|
||
Hi, Milan, can you help to find someone to check this issue? Thank you very much.
Flags: needinfo?(milan)
Comment 7•10 years ago
|
||
This happens on both landscape orientations?
Comment 8•10 years ago
|
||
:nical, see my question in https://bugzilla.mozilla.org/show_bug.cgi?id=981794#c7
Assignee: nobody → nical.bugzilla
Flags: needinfo?(milan) → needinfo?(nical.bugzilla)
Assignee | ||
Comment 9•10 years ago
|
||
The comment Milan mentioned is:
> I have a feeling this isn't quite right, and that either 90 or 270 case should
> be (aClip.y, aClip.width-aClip.x, aClip.height, aClip.width)
I wouldn't be surprised, it's worth a shot.
Is this bar supposed to be a tiled layer, though?
Flags: needinfo?(nical.bugzilla)
Comment 10•10 years ago
|
||
Not sure - let me play with it a bit, you can catch up tomorrow.
Comment 11•10 years ago
|
||
(In reply to Milan Sreckovic [:milan] from comment #10) > Not sure - let me play with it a bit, you can catch up tomorrow. Or not. Having trouble finding a phone where both accelerometer work and it will take one of the sims I have.
Comment 12•10 years ago
|
||
Milan, is there anything we can help here? Thanks.
Assignee | ||
Comment 13•10 years ago
|
||
I can confirm that this bugs is due to wrong clipping of the tiles here: http://dxr.mozilla.org/mozilla-central/source/gfx/layers/composite/TiledContentHost.cpp#349 Looking into it.
Assignee | ||
Comment 14•10 years ago
|
||
Ok, so I have it working now: the proper computation is: case ROTATION_0: result = aClip; break; case ROTATION_90: result = gfx::Rect(aClip.y, destSize.width - aClip.x - aClip.width, aClip.height, aClip.width); break; case ROTATION_270: result = gfx::Rect(destSize.height - aClip.y - aClip.height, aClip.x, aClip.height, aClip.width); break; case ROTATION_180: result = gfx::Rect(destSize.width - aClip.x - aClip.width, destSize.height - aClip.y - aClip.height, aClip.width, aClip.height); break; In addition, in order to properly support rendering to intermediate surfaces, we must add the offset to the intermediate surface before doing this computation (currently we are doing it after). So this also fixes a bug where typing a long string into an input field in landscape mode makes the text disappear. I originally thought zoom levels would need to be part of the equation but actually they don't since the clip space isn't transformed by panning and zooming. I am still looking into the proper place to get destSize (which is basically the screen resolution. I originally though that it had to be the size of the destination surface (the screen OR the intermediate surface) but it appears that it always works with the screen size even when rendering into an intermediate surface. Plus we don't currently have access to the size of render targets (they always return (0, 0) and print warnings about us not being supposed to ask for the size. All compositor implems have an mSize member that I could expose to the base class, but some implementation don't actually seem to keep it up to date (for example in the basic compositor it looks like an unused member).
Assignee | ||
Comment 15•10 years ago
|
||
Also, worth noting, it looks like the compositor receives the screen orientation before the content process repaints content, which causes us to start using the clipping coordinates of the next orientation before we actually display the next orientation (for half a second or so). there must be a better place to update the compositor's mScreenRotation so that it is in sync with the content.
Assignee | ||
Comment 16•10 years ago
|
||
Attachment #8411808 -
Flags: review?(bas)
Comment 17•10 years ago
|
||
(In reply to Nicolas Silva [:nical] from comment #14) > Ok, so I have it working now: the proper computation is: > > case ROTATION_0: > result = aClip; > break; > case ROTATION_90: > result = gfx::Rect(aClip.y, > destSize.width - aClip.x - aClip.width, > aClip.height, aClip.width); > break; > case ROTATION_270: > result = gfx::Rect(destSize.height - aClip.y - aClip.height, > aClip.x, > aClip.height, aClip.width); > break; > case ROTATION_180: > result = gfx::Rect(destSize.width - aClip.x - aClip.width, > destSize.height - aClip.y - aClip.height, > aClip.width, aClip.height); > break; Hey, Botond, looks like our whiteboard computation was right :)
Assignee | ||
Comment 18•10 years ago
|
||
woops, the previous patch was empty. Botond will be a better reviewer for this, since he already had to wrap his head around the problem.
Attachment #8411808 -
Attachment is obsolete: true
Attachment #8411808 -
Flags: review?(bas)
Attachment #8411846 -
Flags: review?(botond)
Assignee | ||
Comment 19•10 years ago
|
||
(In reply to Nicolas Silva [:nical] from comment #15) > Also, worth noting, it looks like the compositor receives the screen > orientation before the content process repaints content, which causes us to > start using the clipping coordinates of the next orientation before we > actually display the next orientation (for half a second or so). there must > be a better place to update the compositor's mScreenRotation so that it is > in sync with the content. I filed bug 1000916 as a followup for this.
Comment 20•10 years ago
|
||
I may not have a correct visual of what is going on, and that may mean others don't either, so I would maybe draw some pictures and attach them. In the meantime, I'll just comment based on my mental image :) (In reply to Nicolas Silva [:nical] from comment #14) > Ok, so I have it working now: the proper computation is: > > case ROTATION_0: > result = aClip; > break; > case ROTATION_90: > result = gfx::Rect(aClip.y, > destSize.width - aClip.x - aClip.width, Double check if this is not: destSize.height - aClip.x - aClip.width instead? > aClip.height, aClip.width); > break; > case ROTATION_270: > result = gfx::Rect(destSize.height - aClip.y - aClip.height, Double check if this is not: destSize.width - aClip.y - aClip.height instead? > aClip.x, > aClip.height, aClip.width); > break; > case ROTATION_180: > result = gfx::Rect(destSize.width - aClip.x - aClip.width, > destSize.height - aClip.y - aClip.height, > aClip.width, aClip.height); > break;
Assignee | ||
Comment 21•10 years ago
|
||
(In reply to Milan Sreckovic [:milan] from comment #20) > I may not have a correct visual of what is going on, and that may mean > others don't either, so I would maybe draw some pictures and attach them. > In the meantime, I'll just comment based on my mental image :) I'll attach some pictures but in the mean time, I already checked what you suggest. The reason it makes sens that the computation substracts/adds widths to Xs and heights to Ys is that we are going from clip-space to layer space. in clip-space, the x and y axis correspond to the ones of the destination buffer, which means that width is along X and y is along Y. If you were going the other way around (from layer space to clip space) you would indeed have to substract/add X axis to heights and Y axis to width
Assignee | ||
Comment 22•10 years ago
|
||
(In reply to Nicolas Silva [:nical] from comment #21) > destination buffer, which means that width is along X and y is along Y. ...and height is along Y, sorry
Assignee | ||
Comment 23•10 years ago
|
||
hope this helps: https://dl.dropboxusercontent.com/u/7742672/IMG_20140424_175711.jpg
Comment 24•10 years ago
|
||
Comment on attachment 8411846 [details] [diff] [review] Fix the clipping transformation when the screen is rotated Review of attachment 8411846 [details] [diff] [review]: ----------------------------------------------------------------- After some IRC and email discussion with Milan and Nicolas I am convinced that this is correct.
Attachment #8411846 -
Flags: review?(botond) → review+
Assignee | ||
Comment 25•10 years ago
|
||
try push https://tbpl.mozilla.org/?tree=Try&rev=ce64c50cedc7
Comment 26•10 years ago
|
||
Comment on attachment 8411846 [details] [diff] [review] Fix the clipping transformation when the screen is rotated Review of attachment 8411846 [details] [diff] [review]: ----------------------------------------------------------------- It should be noted I suspect you're getting the -client size- of the widget here rather than the Widget size. Which on mobile platforms would usually be the same, but on desktop would not. But that's just a naming issue since for your purposes the client size, I believe, would be the correct one.
Assignee | ||
Comment 27•10 years ago
|
||
Fixed the error caused by silly enum sentinel values not being taken in he switch statement and landed. https://hg.mozilla.org/integration/mozilla-inbound/rev/3d116167774f
Comment 28•10 years ago
|
||
sorry nicolas has to backout this checking since it caused bustages like https://tbpl.mozilla.org/php/getParsedLog.php?id=38471754&tree=Mozilla-Inbound
Assignee | ||
Comment 29•10 years ago
|
||
fixed and relanded https://hg.mozilla.org/integration/mozilla-inbound/rev/a14c6bf99d13
Assignee | ||
Comment 30•10 years ago
|
||
also missed a #include of gfx2DGlue.h https://hg.mozilla.org/integration/mozilla-inbound/rev/863ba80b665f on Windows
Comment 31•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a14c6bf99d13 https://hg.mozilla.org/mozilla-central/rev/863ba80b665f
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Comment 32•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/c2f1cc4116e4
status-b2g-v2.0:
--- → fixed
status-firefox29:
--- → wontfix
status-firefox30:
--- → fixed
status-firefox31:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•