Closed
Bug 1043644
Opened 11 years ago
Closed 11 years ago
Things go wonky after zooming in
Categories
(Core Graveyard :: Widget: WinRT, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla34
People
(Reporter: kats, Assigned: kats)
References
Details
Attachments
(2 files, 5 obsolete files)
5.37 KB,
patch
|
tnikkel
:
review+
|
Details | Diff | Splinter Review |
10.45 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
Latest metro project branch build, if you go to CNN.com or any other website and zoom in, you won't be able to zoom out or pan horizontally.
Looks like the composition bounds get larger as you zoom in, and that screws things up.
Comment 2•11 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1)
> Regression from bug 1000423.
I guess the claim in the comments that "mTransformScale is 1, module CSS transforms", is not true on Metro, after all.
Comment 3•11 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #2)
> "mTransformScale is 1, module CSS transforms"
modulo*
Assignee | ||
Comment 4•11 years ago
|
||
So in this case there's only one APZC, and for it resolution == cumulative resolution. I'm quite reluctant to change this code again in production. Jim, is there a policy on landing metro-only fixes #ifdef'd onto the metro tree?
Flags: needinfo?(jmathies)
![]() |
||
Comment 5•11 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)
> So in this case there's only one APZC, and for it resolution == cumulative
> resolution. I'm quite reluctant to change this code again in production.
> Jim, is there a policy on landing metro-only fixes #ifdef'd onto the metro
> tree?
Feel free to land metro only fixes in the metro repo. In the off chance we do bring the metro browser back, I'd suggest it be best to land stuff you would be comfortable having mass-merged back into mc.
Flags: needinfo?(jmathies)
Assignee | ||
Comment 6•11 years ago
|
||
Assignee | ||
Comment 7•11 years ago
|
||
Assignee | ||
Comment 8•11 years ago
|
||
tn, I'm happy to land option #2 directly on the metro tree. Let me know if option #1 is better and worth going into m-c (like if it fixes other things).
Flags: needinfo?(tnikkel)
Comment 9•11 years ago
|
||
Comment on attachment 8462109 [details] [diff] [review]
Patch (option #1)
Things that we can do after this (leaving this here just so I remember, not because you have to do it): get rid of viewbounds altogether, it's the same as frame bounds, it was my mistake to think it would be different. Also we should be able to get rid of the first android ifdef, and maybe the second one (have to test).
Attachment #8462109 -
Flags: review+
Flags: needinfo?(tnikkel)
Comment 10•11 years ago
|
||
Comment on attachment 8462109 [details] [diff] [review]
Patch (option #1)
Just as a reminder to myself so I can find it later: this patch does bug 985992, comment 2.
Comment 11•11 years ago
|
||
Comment on attachment 8462109 [details] [diff] [review]
Patch (option #1)
I forgot that you need to make the same change in two other places: nsLayoutUtils::CalculateRootCompositionSize and nsLayoutUtils::CalculateCompositionSizeForFrame
Comment 12•11 years ago
|
||
I did the first cleanup item. I'll leave the other two for now.
viewbounds are the same as framebounds, so we don't need them. If we are a root scroll frame we'll use widget bounds or content viewer bounds.
Assignee | ||
Comment 13•11 years ago
|
||
Updated to also modify the two functions in nsLayoutUtils.cpp
Assignee: nobody → bugmail.mozilla
Attachment #8462109 -
Attachment is obsolete: true
Attachment #8462111 -
Attachment is obsolete: true
Attachment #8463362 -
Flags: review?(tnikkel)
Assignee | ||
Comment 14•11 years ago
|
||
tn, I left you as the author on this patch.
Attachment #8462348 -
Attachment is obsolete: true
Attachment #8463363 -
Flags: review+
Comment 15•11 years ago
|
||
Comment on attachment 8463362 [details] [diff] [review]
Part 1 - Try using the contentviewer bounds
Now that I read this with fresh eyes I think that we should be incorporating the resolution up to the parent before considering the content viewer bounds as layer pixels. (It'll be 1 right now, but maybe not in the future.)
You can do it as a separate patch on top of the first two if you'd like.
Assignee | ||
Comment 16•11 years ago
|
||
(In reply to Timothy Nikkel (:tn) from comment #15)
> Now that I read this with fresh eyes I think that we should be incorporating
> the resolution up to the parent before considering the content viewer bounds
> as layer pixels. (It'll be 1 right now, but maybe not in the future.)
Not sure which resolution you're talking about here.
Btw, try push from the first two parts is at https://tbpl.mozilla.org/?tree=Try&rev=00615a8ab506 - I fixed the build failures locally by moving frameSize into the ifdef.
Comment 17•11 years ago
|
||
I think that the content viewer bounds type is layout device pixels in the parent layer. So to convert them to layer pixels (where we do that) we need to multiply them by the cumulative resolution of the parent. Or am I thinking about it wrong?
Assignee | ||
Comment 18•11 years ago
|
||
That makes sense. I changed GetContentViewerBounds to take a LayoutDeviceIntRect to make it more clear and propagated some conversions.
Attachment #8463362 -
Attachment is obsolete: true
Attachment #8463362 -
Flags: review?(tnikkel)
Attachment #8464089 -
Flags: review?(tnikkel)
Assignee | ||
Comment 19•11 years ago
|
||
Part 2 rebased
Attachment #8463363 -
Attachment is obsolete: true
Attachment #8464091 -
Flags: review+
Assignee | ||
Comment 20•11 years ago
|
||
New try push:
https://tbpl.mozilla.org/?tree=Try&rev=408ef940ff99
Comment 21•11 years ago
|
||
Comment on attachment 8464089 [details] [diff] [review]
Part 1 - Try using the contentviewer bounds
>@@ -6643,7 +6671,13 @@ nsLayoutUtils::CalculateRootCompositionS
> }
> #endif
> } else {
>- rootCompositionSize = viewSize;
>+ LayoutDeviceIntRect contentBounds;
>+ if (nsLayoutUtils::GetContentViewerBounds(rootPresContext, contentBounds)) {
>+ gfxSize res = rootPresContext->PresShell()->GetCumulativeResolution();
>+ rootCompositionSize = contentBounds.Size() * LayoutDeviceToLayerScale(res.width, res.height);
>+ } else {
>+ rootCompositionSize = viewSize;
>+ }
> }
> }
> }
This should use the cumulative resolution of the parent of rootPresContext, if there is one, or just 1.0 if not.
Attachment #8464089 -
Flags: review?(tnikkel) → review+
Assignee | ||
Comment 22•11 years ago
|
||
Comment 24•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fdde5b95b064
https://hg.mozilla.org/mozilla-central/rev/0f1b0d6b870c
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Updated•6 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•