Closed Bug 1043644 Opened 7 years ago Closed 7 years ago

Things go wonky after zooming in

Categories

(Core Graveyard :: Widget: WinRT, defect)

All
Windows 8.1
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla34

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(2 files, 5 obsolete files)

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.
Regression from bug 1000423.
Blocks: 1000423
(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.
(In reply to Botond Ballo [:botond] from comment #2)
> "mTransformScale is 1, module CSS transforms"

modulo*
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)
(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)
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 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 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 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
Attached patch stop using view bounds (obsolete) — Splinter Review
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.
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)
tn, I left you as the author on this patch.
Attachment #8462348 - Attachment is obsolete: true
Attachment #8463363 - Flags: review+
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.
(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.
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?
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)
Part 2 rebased
Attachment #8463363 - Attachment is obsolete: true
Attachment #8464091 - Flags: review+
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+
Duplicate of this bug: 985992
https://hg.mozilla.org/mozilla-central/rev/fdde5b95b064
https://hg.mozilla.org/mozilla-central/rev/0f1b0d6b870c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.