1189565 - [e10s] APZ seriously messes up mouse coordinates on secondary display in e10s mode on OS X with Retina Mac

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Steven Michaud [:smichaud] (Retired)]

As of the patch for bug 1157746, when a Firefox window created on one display is moved to another, the locations effected by mouse events are seriously offset from the actual mouse cursor's location, in e10s mode.

1) Run a Firefox m-c nightly dated 2015-07-23 or later on a system with a secondary display attached.  Make sure "enable multi-process nightly" is selected.
2) Drag the Firefox window from the main display to the secondary display.
3) Notice that the effect of mouse-moves and mouse-clicks is offset up and to the left of the actual mouse cursor location.

The displacement seems to grow larger as you move the mouse cursor lower and to the right.  Only Gecko chrome and content are effected.  The mouse works correctly to hover or click the close, minimize and zoom buttons, to drag the window by its titlebar, and to resize the window by its edges or "grab region" (to the lower right).

The bug disappears when you resize the window.  It reappears when you drag it from one display to the other.

Turning HiDPI on or off makes no difference.  The bug only happens with e10s on (not with it off).

The workaround is to set layers.async-pan-zoom.enabled to false in about:config.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

I'm not able to reproduce this on my OS X laptop with a secondary display attached. There must be more factors involved here.

(In reply to Steven Michaud [:smichaud] from comment #0)
> 3) Notice that the effect of mouse-moves and mouse-clicks is offset up and
> to the left of the actual mouse cursor location.
> The displacement seems to grow larger as you move the mouse cursor lower and
> to the right.

This sounds like it's off by a scaling factor of some sort with the origin anchored at the top-left of the window. Do you know if your two displays have different densities or would result in a different nsIWidget::GetDefaultScale()? You said it doesn't make a difference with HiDPI which surprises me.

Comment 2

[Steven Michaud [:smichaud] (Retired)]

Kats, what version of OS X were you testing with?  And where was your secondary display "arranged"?

I tested on OS X 10.7.5, on a Retina MacBook Pro, with a non-Retina (and larger) secondary display arranged to the left.  Later I'll try on different versions of OS X, with different "arrangements", to see if that makes a difference.

I tested with a clean profile.

To test without HiDPI mode, I right-clicked on the distro and chose Get Info : Open in Low Resolution.

Looking at the source code, it seems that nsChildView/nsCocoaWindow::GetDefaultScale() always returns 1.0 for non-HiDPI displays and 2.0 for HiDPI ones (in HiDPI mode).  With a clean profile, that is.  I doubt nsIWidget::GetDefaultScale() is involved here -- unless it's being misused somewhere in APZ code.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

I tested on 10.9.5, with the secondary display "above" my primary display. The right edges of the secondary and primary displays are vertically aligned, and the secondary display is wider (so it's left edge extends beyond the left edge of the primary display).

Comment 4

[Steven Michaud [:smichaud] (Retired)]

I also see the bug on OS X 10.10.4.  On both 10.7.5 and 10.10.4, I see it (with exactly the same symptoms) regardless of where the secondary display is "arranged" -- left, right, above or below.

None of the edges are aligned -- I'll check if that makes a difference.  I'll also test on 10.9.5.

Comment 5

[Steven Michaud [:smichaud] (Retired)]

Arranging my secondary display like yours (above and with the right edges aligned) makes no difference -- I still see the bug.  I also see the bug (with exactly the same symptoms) on OS X 10.9.5..

My secondary display has an unusual horizontal to vertical ratio:  4 to 3, like in traditional CRTs (though my display isn't a CRT) -- not the "cinema" ratio that's almost universal these days (and which frankly I hate).  My Retina MacBook Pro's display, of course, has the "cinema" ratio.

My secondary display is an NEC LCD2190UXp.

Are you in an office, Kats?  If so could you look around with a 4 by 3 secondary monitor to test with?

Comment 6

[Kartikaya Gupta (email:[e-mail])]

(In reply to Steven Michaud [:smichaud] from comment #5)
> Are you in an office, Kats?  If so could you look around with a 4 by 3
> secondary monitor to test with?

I am not, unfortunately. My secondary display is 1920x1200.

Comment 7

[Steven Michaud [:smichaud] (Retired)]

> My secondary display is 1920x1200.

Mine is 1600x1200.

I'm not in an office, either.  And I have no "cinema"-style external monitor to test with.

Comment 8

[Steven Michaud [:smichaud] (Retired)]

I wonder if the bug here is that our APZ code is using the "wrong" display's coordinate system in e10s mode.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Attachment: Logging patch

If you have some time, could you apply this patch, reproduce the problem and save the console output? It would help if you moved the window to a known coordinate position like (0,0) on your secondary display. It would also be good to use a page like http://people.mozilla.org/~kgupta/grid.html and try clicking on one of the links after the window-move so that we know the approximate coordinates of the click in content CSS pixels at least.

Comment 10

[Steven Michaud [:smichaud] (Retired)]

The logs from your patch are *much* too noisy to be useful.  How about limiting them to key presses?  And each log "entry" should have a timestamp, to make it possible to edit out some of the noise.

Also you probably want to log the "native" mouse coordinates (those from the native key-down event) along with the other information.

As you'd expect, I can't click directly on any of your test page's links.  But I did manage to find the correct offsets (to click on) for the first two in the top row.

Comment 11

[Steven Michaud [:smichaud] (Retired)]

> How about limiting them to key presses?

Oops, that makes no sense.  How about limiting them to mouse clicks?

Comment 12

[Kartikaya Gupta (email:[e-mail])]

In a case like this where we have no idea what's going wrong I find it's better to have too much logging and wade through it than to target the logging and hope we guessed the right spots to log. As long as you only click on the test page twice (once while it's on the primary display, and once after moving it to the secondary display) I shouldn't have any trouble finding the relevant parts in the log spew.

Comment 13

[Kartikaya Gupta (email:[e-mail])]

(Also if you *really* want more targetted logging I can write a patch that does that but it'll have to wait until next week at least. Using the existing logging stuff we have is cheap and easy.)

Comment 14

[Steven Michaud [:smichaud] (Retired)]

Whatever I do is going to have to wait until next week, anyway -- I'm too busy with other stuff.  By the way, I *can't* do exactly as you say (click just twice on the test page).  I also need to click on it (on the titlebar) to drag it from one screen to the other.

For now this bug doesn't seem to be urgent.  I'm the only one we know is effected, and I have an easy workaround (set layers.async-pan-zoom.enabled to false).

Comment 15

[Tracy Walker [:tracy]]

I saw this today right after updating Nightly.  Browser was already on second screen when it restarted for the update.  But cursor focus was strangely offset on the about:newtab page.  Opened a new tab and it was fine.

Comment 16

[Steven Michaud [:smichaud] (Retired)]

Tracy, please tell us as much as you can about your secondary monitor, and the computer you're connecting it to.

Comment 17

[Tracy Walker [:tracy]]

The External is an HP 25xi connected to laptop via HDMI. It's a 10.9.5 MacBook Pro with retina display.

Here's the Graphics/Display data. 

NVIDIA GeForce GT 750M:
  Chipset Model:	NVIDIA GeForce GT 750M
  Type:	GPU
  Bus:	PCIe
  PCIe Lane Width:	x8
  VRAM (Total):	2048 MB
  Vendor:	NVIDIA (0x10de)
  Device ID:	0x0fe9
  Revision ID:	0x00a2
  ROM Revision:	3776
  gMux Version:	4.0.8 [3.2.8]
  Displays:
Color LCD:
  Display Type:	Retina LCD
  Resolution:	2880 x 1800
  Retina:	Yes
  Pixel Depth:	32-Bit Color (ARGB8888)
  Mirror:	Off
  Online:	Yes
  Built-In:	Yes
HP 25xi:
  Resolution:	1920 x 1080 @ 60Hz (1080p)
  Pixel Depth:	32-Bit Color (ARGB8888)
  Display Serial Number:	3CM31103BM  
  Main Display:	Yes
  Mirror:	Off
  Online:	Yes
  Rotation:	Supported
  Television:	Yes

Comment 18

[Steven Michaud [:smichaud] (Retired)]

Thanks!  Does it make a difference if you use some other type of connection than HDMI?

Also tell us if turning off e10s or APZ makes the problem go away (as it does for me).  To turn off APZ, set layers.async-pan-zoom.enabled to false in about:config.

Comment 19

[Steven Michaud [:smichaud] (Retired)]

Finally, also let us know if your problem starts with the 2015-07-23 mozilla-central nightly (as it does for me).  For that you'll need to test with a clean profile.

Comment 20

[Steven Michaud [:smichaud] (Retired)]

Kats, do you have an HDMI connector to play with?  And what kind of connection do you currently have to your external monitor?

For the record, my external monitor is connected via a Mini DisplayPort (and an adapter) to a DVI connector.

Comment 21

[Kartikaya Gupta (email:[e-mail])]

I'm connected via Thunderbolt on my laptop to a Displayport on the external monitor. I don't have an HDMI connector lying around unfortunately.

Comment 22

[Tracy Walker [:tracy]]

Yes, turning off APZ made the issue go away immediately.  I'll have to dig up cables and adapters to check other connection to the monitor.

Comment 23

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #20)

> For the record, my external monitor is connected via a Mini
> DisplayPort (and an adapter) to a DVI connector.

Actually this is partly wrong.  What I called a Mini DisplayPort is
actually a Thunderbolt port.

So my external monitor is connected via a Thunderbolt port (and an
adapter) to a DVI connector.

Comment 24

[Steven Michaud [:smichaud] (Retired)]

I just tried connecting two other, older (non-Retina) MacBook Pros via the same adapter and DVI connector -- one with a Thunderbolt port, one with a genuine Mini DisplayPort.  I *didn't* see the bug with either one.

So I now expect we'll only see this bug with Retina MacBook Pros -- even though the bug still happens if you turn off HiDPI support (by right-clicking on the distro and choosing Get Info : Open in Low Resolution).

However the kind of connector *might* still make a difference with Retina MacBook Pros.

Comment 25

[Steven Michaud [:smichaud] (Retired)]

Kats, you haven't yet told us whether your MacBook Pro is a Retina MacBook Pro.

Comment 26

[Kartikaya Gupta (email:[e-mail])]

I have a non-retina macbook air.

Comment 27

[Steven Michaud [:smichaud] (Retired)]

So, getting perhaps a little ahead of the evidence, it looks like this bug happens with *every* Retina MacBook Pro, no matter how it's connected (or otherwise set up).  That makes this a much more serious (and common) bug than I first thought.

I'll try to track it down myself.  Though I'm unfamiliar with APZ code, it sounds like it's missing notifications (of display configuration changes) that do get sent to HiDPI code.  That should make it easier to spot the problem.

Comment 28

[Steven Michaud [:smichaud] (Retired)]

[Tracking Requested - why for this release]:

The bug is much more common than I first thought, and therefore more serious.

Comment 29

[Kartikaya Gupta (email:[e-mail])]

Note that APZ is enabled #ifdef nightly, so it's not really on the 42 train and it may not make sense to track it as such.

Comment 30

[Steven Michaud [:smichaud] (Retired)]

Fair enough.  We should still "track e10s", though.

Comment 31

[Steven Michaud [:smichaud] (Retired)]

The plot thickens, but I haven't yet managed to figure this bug out.

By trial and error I've found that the following patch stops this bug from happening:

diff -U 10 -r src.old/layout/base/nsDisplayList.cpp src/layout/base/nsDisplayList.cpp
--- src.old/layout/base/nsDisplayList.cpp	2015-07-31 16:10:17.000000000 -0500
+++ src/layout/base/nsDisplayList.cpp	2015-08-06 18:46:55.000000000 -0500
@@ -1299,22 +1299,22 @@
   }
   return result;
 }
 
 bool
 nsDisplayListBuilder::IsBuildingLayerEventRegions()
 {
   if (mMode == PAINTING) {
     // Note: this is the only place that gets to query LayoutEventRegionsEnabled
     // 'directly' - other code should call this function.
-    return gfxPrefs::LayoutEventRegionsEnabledDoNotUseDirectly() ||
-           mAsyncPanZoomEnabled;
+    return gfxPrefs::LayoutEventRegionsEnabledDoNotUseDirectly() /*||
+           mAsyncPanZoomEnabled*/;
   }
   return false;
 }
 
 void nsDisplayListSet::MoveTo(const nsDisplayListSet& aDestination) const
 {
   aDestination.BorderBackground()->AppendToTop(BorderBackground());
   aDestination.BlockBorderBackgrounds()->AppendToTop(BlockBorderBackgrounds());
   aDestination.Floats()->AppendToTop(Floats());
   aDestination.Content()->AppendToTop(Content());

And without that change, so does the following:

diff -U 10 -r src.old/layout/generic/nsFrame.cpp src/layout/generic/nsFrame.cpp
--- src.old/layout/generic/nsFrame.cpp	2015-07-27 10:56:09.000000000 -0500
+++ src/layout/generic/nsFrame.cpp	2015-08-06 18:49:33.000000000 -0500
@@ -2039,26 +2039,28 @@
     }
 
     MarkAbsoluteFramesForDisplayList(aBuilder, dirtyRect);
 
     // Preserve3DChildren() also guarantees that applyAbsPosClipping and usingSVGEffects are false
     // We only modify the preserve-3d rect if we are the top of a preserve-3d heirarchy
     if (Preserves3DChildren()) {
       aBuilder->MarkPreserve3DFramesForDisplayList(this, aDirtyRect);
     }
 
+#if (0)
     if (aBuilder->IsBuildingLayerEventRegions()) {
       nsDisplayLayerEventRegions* eventRegions =
         new (aBuilder) nsDisplayLayerEventRegions(aBuilder, this);
       aBuilder->SetLayerEventRegions(eventRegions);
       set.BorderBackground()->AppendNewToTop(eventRegions);
     }
+#endif
     aBuilder->AdjustWindowDraggingRegion(this);
     BuildDisplayList(aBuilder, dirtyRect, set);
   }
 
   if (aBuilder->IsBackgroundOnly()) {
     set.BlockBorderBackgrounds()->DeleteAll();
     set.Floats()->DeleteAll();
     set.Content()->DeleteAll();
     set.PositionedDescendants()->DeleteAll();
     set.Outlines()->DeleteAll();

Comment 33

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #31)

Actually, what I said in comment #31 isn't quite right.  The patches I listed there don't completely get rid of this bug.  You also have to add the following patch:

diff -U 10 -r src.old/layout/ipc/RenderFrameParent.cpp src/layout/ipc/RenderFrameParent.cpp
--- src.old/layout/ipc/RenderFrameParent.cpp	2015-07-27 10:56:09.000000000 -0500
+++ src/layout/ipc/RenderFrameParent.cpp	2015-08-07 18:28:24.000000000 -0500
@@ -313,24 +313,26 @@
 
   if (XRE_IsParentProcess()) {
     // Our remote frame will push layers updates to the compositor,
     // and we'll keep an indirect reference to that tree.
     *aId = mLayersId = CompositorParent::AllocateLayerTreeId();
     if (lm && lm->GetBackendType() == LayersBackend::LAYERS_CLIENT) {
       ClientLayerManager *clientManager =
         static_cast<ClientLayerManager*>(lm.get());
       clientManager->GetRemoteRenderer()->SendNotifyChildCreated(mLayersId);
     }
+#if (0)
     if (mAsyncPanZoomEnabled) {
       mContentController = new RemoteContentController(this);
       CompositorParent::SetControllerForLayerTree(mLayersId, mContentController);
     }
+#endif
   } else if (XRE_IsContentProcess()) {
     ContentChild::GetSingleton()->SendAllocateLayerTreeId(aId);
     mLayersId = *aId;
     CompositorChild::Get()->SendNotifyChildCreated(mLayersId);
   }
   *aSuccess = true;
 }
 
 APZCTreeManager*
 RenderFrameParent::GetApzcTreeManager()

I still haven't figured out this bug.  And for the moment I'm out of time -- I'm on vacation all of next week.

Comment 34

[Steven Michaud [:smichaud] (Retired)]

Actually, the following patch plus the one in comment #33 also "fixes" this bug:

diff -U 10 -r src.old/layout/generic/nsFrame.cpp src/layout/generic/nsFrame.cpp
--- src.old/layout/generic/nsFrame.cpp	2015-07-27 10:56:09.000000000 -0500
+++ src/layout/generic/nsFrame.cpp	2015-08-07 18:38:37.000000000 -0500
@@ -2039,21 +2039,24 @@
     }
 
     MarkAbsoluteFramesForDisplayList(aBuilder, dirtyRect);
 
     // Preserve3DChildren() also guarantees that applyAbsPosClipping and usingSVGEffects are false
     // We only modify the preserve-3d rect if we are the top of a preserve-3d heirarchy
     if (Preserves3DChildren()) {
       aBuilder->MarkPreserve3DFramesForDisplayList(this, aDirtyRect);
     }
 
-    if (aBuilder->IsBuildingLayerEventRegions()) {
+    ViewportFrame *viewportFrame = do_QueryFrame(this);
+    nsBoxFrame *boxFrame = do_QueryFrame(this);
+    bool isContentProcess = XRE_IsContentProcess();
+    if (!viewportFrame && !boxFrame && aBuilder->IsBuildingLayerEventRegions()) {
       nsDisplayLayerEventRegions* eventRegions =
         new (aBuilder) nsDisplayLayerEventRegions(aBuilder, this);
       aBuilder->SetLayerEventRegions(eventRegions);
       set.BorderBackground()->AppendNewToTop(eventRegions);
     }
     aBuilder->AdjustWindowDraggingRegion(this);
     BuildDisplayList(aBuilder, dirtyRect, set);
   }
 
   if (aBuilder->IsBackgroundOnly()) {

Comment 35

[Steven Michaud [:smichaud] (Retired)]

I'm getting closer to figuring this out.  I've now found that the following patch "fixes" this bug all by itself:

diff -U 10 -r src.old/gfx/layers/apz/src/APZCTreeManager.cpp src/gfx/layers/apz/src/APZCTreeManager.cpp
--- src.old/gfx/layers/apz/src/APZCTreeManager.cpp	2015-07-27 10:56:08.000000000 -0500
+++ src/gfx/layers/apz/src/APZCTreeManager.cpp	2015-08-18 18:05:55.000000000 -0500
@@ -403,20 +410,21 @@
         apzc = node->GetApzc();
         break;
       }
     }
 
     // The APZC we get off the layer may have been destroyed previously if the
     // layer was inactive or omitted from the layer tree for whatever reason
     // from a layers update. If it later comes back it will have a reference to
     // a destroyed APZC and so we need to throw that out and make a new one.
     bool newApzc = (apzc == nullptr || apzc->IsDestroyed());
+    newApzc = true;
     if (newApzc) {
       apzc = MakeAPZCInstance(aLayersId, state->mController);
       apzc->SetCompositorParent(aState.mCompositor);
       if (state->mCrossProcessParent != nullptr) {
         apzc->ShareFrameMetricsAcrossProcesses();
       }
       MOZ_ASSERT(node == nullptr);
       node = new HitTestingTreeNode(apzc, true, aLayersId);
     } else {
       // If we are re-using a node for this layer clear the tree pointers

Comment 36

[Tracy Walker [:tracy]]

(In reply to Steven Michaud [:smichaud] from comment #28)
> [Tracking Requested - why for this release]:
> 
> The bug is much more common than I first thought, and therefore more serious.

Yes, I've experienced this more often than just after update. It has been happening on wake from sleep too.

The simple work around is to resize the affected Window.  Fortunately, the Window edge grab is not offset.

Steven, sorry I haven't got back to check alternate cabling methods.  Do you still need that from me?  Anything else I can do to help here?

Comment 37

[Steven Michaud [:smichaud] (Retired)]

> Anything else I can do to help here?

I should shortly have a fix for this bug.  You can test it :-)

Comment 38

[Steven Michaud [:smichaud] (Retired)]

And don't bother with the cabling -- I no longer think it's relevant.

Comment 39

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix

Even after the big clue in comment #35, it still took me a while to finish figuring this bug out.  Finally, though, I noticed that nsView::WindowResized() was never getting called in the main process when dragging a window from an HiDPI screen to a non-HiDPI screen (or vice versa) -- only (somehow) in the content process (in e10s mode).  So when e10s is on, there's a mismatch between the window size information in the main/chrome process and the content process.  Hence this bug.

Once you think of it this way, the fix is obvious.  Moving a window between an HiDPI screen and a non-HiDPI screen causes its size to change in device pixels (though not in display pixels).  So all along we should have been calling ReportSizeEvent() from nsChildView::BackingScaleFactorChanged() and nsCocoaWindow::BackingScaleFactorChanged().  But somehow this didn't cause any (clearly visible) trouble until APZ got turned on.

I've started a set of tryserver builds:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e9bfca45fbc5

Comment 40

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #0)

> Turning HiDPI on or off makes no difference.

I still can't explain why this would happen.  But in any case it's no longer true.  Testing with today's mozilla-central nightly (still on OS X 10.7.5), this bug doesn't happen when it's set to "open in low resolution".

Comment 41

[Kartikaya Gupta (email:[e-mail])]

Nice, thanks for tracking this down! The fix looks sane to me. (Adjusting flags since APZ is only enabled #ifdef nightly, and so right now this bug should be affecting 43 and not 42).

Comment 42

[Steven Michaud [:smichaud] (Retired)]

Tracy, please test with this build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-e9bfca45fbc5/try-macosx64/firefox-43.0a1.en-US.mac.dmg

Comment 43

[Tracy Walker [:tracy]]

(In reply to Steven Michaud [:smichaud] from comment #42)
> Tracy, please test with this build:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-
> e9bfca45fbc5/try-macosx64/firefox-43.0a1.en-US.mac.dmg

The cases I could reproduce with latest Nightly are not reproducible with the try build.

Comment 44

[Markus Stange [:mstange]]

Comment on attachment 8650144 [details] [diff] [review]
Fix

Review of attachment 8650144 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

Comment 45

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2e731191c692

Comment 46

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2e731191c692

Comment 47

[Tracy Walker [:tracy]]

I think landing this has regressed other position and sizing issues.  With about:home loaded, if I move the window from external monitor to the retina display.  I get a home page about 1/3rd expected size plus random images flickering in the rest of the window content area.  These images can be anything from what is in a browser on another desktop to screen saver images.  (maybe cached in the video driver?)

Also, moving browser window back and forth between displays now gets a much slower resizing of content before settling on final correct size.  The double resizing was happening prior to this landing, but it was much quicker than now.

Comment 48

[Steven Michaud [:smichaud] (Retired)]

Tracy, do you have hardware acceleration off?  Or any other non-default settings?

And what version of OS X are you testing on?

I didn't see any such problems testing on OS X 10.7.5.

Comment 49

[Steven Michaud [:smichaud] (Retired)]

Also, of course, please test with a clean profile.

Comment 50

[Steven Michaud [:smichaud] (Retired)]

And finally, please look for a regression range for your problems.  They may not have started when my patch landed.

Comment 51

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #50)

Also test if you see these problems with my tryserver build from comment #42.

Comment 52

[Tracy Walker [:tracy]]

I had a random Nightly from the 18th available. It displayed the flickering of cached images when browser moved to retina display.  So I'll NI myself to investigate each of the things I mentioned above and file individual bugs on them.

Steven, how do I check on hardware acceleration settings?

Comment 53

[Steven Michaud [:smichaud] (Retired)]

Preferences : Advanced : General : Use hardware acceleration when available

> I had a random Nightly from the 18th available.

This bug's patch landed on the trunk only as of today's mozilla-central nightly.  See comment #46.

Comment 54

[Tracy Walker [:tracy]]

(In reply to Steven Michaud [:smichaud] from comment #53)
> Preferences : Advanced : General : Use hardware acceleration when available
> 
> > I had a random Nightly from the 18th available.
> 
> This bug's patch landed on the trunk only as of today's mozilla-central
> nightly.  See comment #46.

Yes, that's why I'll investigate the issues separately.  As at least the odd flickering one is not caused by your check-in here.

Comment 56

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

While investigating bug 1194462, I've run into the sizing issues that Tracy is talking about in comment 47, using the latest Firefox 43 Nightly. Mozregression pointed to this bug as the cause. I've filed bug 1201053 for these issues.

Comment 57

[Steven Michaud [:smichaud] (Retired)]

I've asked KWierso (the current sheriff) to back this out on m-c.

After playing around for a couple of hours, I still haven't found any leads on bug 1201053.

Comment 58

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out
https://hg.mozilla.org/mozilla-central/rev/a6786bf8d71d

Comment 59

[Steven Michaud [:smichaud] (Retired)]

I'm probably not going to be able to fix this bug before I retire at the end of this month.

Comment 60

[Steven Michaud [:smichaud] (Retired)]

To repeat what I said at bug 1201053 comment #5:

It's pretty clear my patch for this bug was correct, at least in a general way -- we do need to send Gecko a resize event when the backing scale factor changes.  But it's also clear we can't do that without making other changes -- possibly a lot of them, very likely in cross-platform code.

Comment 61

[Tracy Walker [:tracy]]

For long term workaround, per comment #14, (set layers.async-pan-zoom.enabled to false). 

Short term workaround is to resize the affected window.

Comment 62

[Liz Henry (:lizzard) (relman/hg->git project)]

Since we now have a few duplicates of this bug, I think at least a few people are affected. 

Tracking this for 43 since it's a regression.  We should probably find another owner for this bug. 

Steven, I didn't realize you're retiring! We'll miss you (especially the mac users)  :)

Comment 63

[Benoit Girard (:BenWa)]

Attachment: Hacky working patch (now why does it work?)

So since resizing manually works I decided to try and see if calling Resize() works. I add +1 to make sure we never ignore this resize call.

This fixes all the issues that I have with the exception of one garbage frame, which is probably in unrelated code.

Now to fully understand why this works and work backwards to a clean fix.

If anyone can weight-in, please do.

Comment 64

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8660941 [details] [diff] [review]
Hacky working patch (now why does it work?)

Does this bring back bug 1201053?

Comment 65

[Benoit Girard (:BenWa)]

No, adjusting mBounds.width/height was required to fix this.

Comment 66

[Benoit Girard (:BenWa)]

Alright it turns out that we also have to fix this form APZ:

http://hg.mozilla.org/mozilla-central/annotate/5d61714cf5c3/gfx/layers/apz/src/AsyncPanZoomController.cpp#l2847

Basically what happens is when change the content scale but APZ doesn't require another paint. I think this code is trying to properly adjust an event when there's a resolution change in-flight.

I can fix this either by resizing, or using APZ such that it dispatches a new FM/paint request.

What should happen here to trigger APZ to update this FM/paint request to the new zoom without having the user interaction trigger it.

Comment 67

[Benoit Girard (:BenWa)]

Attachment: patch

Asking kats to review this since botond help me write this patch. Removing needinfo since we talked in person.

This patch here is sufficient to fully fix this bug AFAIK. However there's some useful bits from smichaud's patch + my additions that help fix painting issue when switching content scale. I'll move the patch and continue working on that part in a different bug since it's not related to this bug and I don't want to conflate them.

Comment 68

[Benoit Girard (:BenWa)]

e10s and APZ are currently tied together. I'd like to track e10s instead of 43 just to make sure this doesn't slide through the cracks. The fix is pretty much ready anyways.

Comment 69

[Benoit Girard (:BenWa)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd90e4d0d7b2

Comment 70

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8661928 [details] [diff] [review]
patch

Review of attachment 8661928 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, thanks!

Just for the record on IRC we discussed that another potential fix may be to call mLastDispatchedPaintMetrics.SetDevPixelsPerCSSPixel(aLayerMetrics.GetDevPixelsPerCSSPixel()); inside the block at http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp?rev=5d4380c90c05#2957. This patch is probably a better fix though.

Comment 71

[Benoit Girard (:BenWa)]

I filed bug 1205372 to follow up with smichaud patch which addresses a different worthwhile problem. I cleared the CC' list when cloning because the issue is fairly different.

I imagine it's likely a dupe but I couldn't find a similar bug.

Comment 72

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c83c2741197af887a19179a24f1ed1ba6f25a661
Bug 1189565 - Only factor in the async zoom change. r=kats

Comment 73

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c83c2741197a