Closed Bug 1154125 Opened 5 years ago Closed 4 years ago

When screen resolution change, window isn't repainted properly

Categories

(Core :: Layout, defect)

39 Branch
x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox49 - fixed
firefox50 --- fixed

People

(Reporter: jya, Assigned: jfkthame)

References

Details

Attachments

(2 files, 1 obsolete file)

This has happened a few times now that I got a new screen that can take a while to start. As a result, returning from sleep the mac sees one screen and a couple of seconds later the 2nd screen becomes active and the resolutions will be adjusted.

When that happens, often the various windows aren't redrawn and its content isn't resized properly

Screen capture attached.
Note that resizing the window is enough to put everything back in order.

Have now reproduced the problem on two different machines.
I see this too, FF 38.0.1, Mac OS X 10.10.3, late-2013 MacBook Pro with external Samsung 24" HDMI monitor.

IMO this happens consistently when the screen resolution changes, which it seems to do whenever screen blanking kicks in on the laptop.  I haven't tested this systematically but whenever I come back to my monitor and its screen is dark, I can expect all windows to have been resized, and the FF window to be improperly painted as per this bug report.

IMO resizing the FF window does not actually fix *all* the problems.  Occasionally, utility windows will appear in very strange places, for example, the dropdown window from the address bar with suggested completions shows up elsewhere on the screen.  (No screenshot available, and not really reproducible, but it's happened to me several times.)  I don't have that problem when working on the laptop screen.
Not sure if this is graphics or widget, though I'd guess at widget. Jean-Yves, are you still seeing this?
Component: General → Untriaged
Flags: needinfo?(jyavenard)
Product: Firefox → Core
Haven't tested FF44 yet, but I observed the same problem in 43 and in 45 DevEd as I reported before.  Ditto the problem with the utility windows ending up all over the geography.  Mac OS X 10.11 now, same hardware.
Markus, do you have any idea why this would happen and/or whether this is more of a Cocoa or a Gfx issue? :-)
Flags: needinfo?(jyavenard) → needinfo?(mstange)
yes, I still see it, not as often as before (but I think this is more to do with OS 10.11.3 resuming better).

When my mac comes out of sleep, and one of the screen took too long to resume, so all windows are moved to one screen. content in firefox is all screwed up until I resize the window)

I'll make a screen capture next time it happens. Oh I see there's already one here, it's exactly like that except that now more often than not, the shrunk content appear in the top right instead of the top left
Flags: needinfo?(mstange)
My guess is that this bug is somewhere in the code that syncs up the resolution and tab size between the parent and the content process. Not sure what component that would be, but I think it's neither gfx nor widget.
(In reply to Markus Stange [:mstange] from comment #7)
> My guess is that this bug is somewhere in the code that syncs up the
> resolution and tab size between the parent and the content process. Not sure
> what component that would be, but I think it's neither gfx nor widget.

Right. I should have looked at the screenshot - reading comment #0 I thought this affected the outer window.

Mike, do you have any ideas where this window size syncing code in e10s lives? :-)

(Jean-Yves/Lars, I'm assuming this goes away if you turn off e10s?)
Flags: needinfo?(mconley)
No.. I have e10s off on my system
OK, then I don't know where the bug is. We've had similar bugs in e10s a few times, see bug 1125325 and bug 1111957.
This is independent of e10s, I've been running with e10s disabled.
CC'ing :jfkthame who might have an idea - this bug predates his recent work on window sizing, but his patches may have fixed it in Nightly.
Clearing needinfo, since it sounds like this isn't e10s.

I'm actually hearing several different bugs within this bug:

1) Web content is not being sized properly
2) Popups like the AwesomeBar dropdown, are misplaced after a wake-up 

It sounds like this bug is mostly about (1). We should probably file a separate one for (2) - :lth, do you think you could do that, mentioning what you've seen?
Flags: needinfo?(mconley) → needinfo?(lhansen)
mconley, i will do that next week when i'm back in oslo and have the monitor available.  (leaving ni? open)
I can no longer repro on Aurora, so this may be fixed.  Will get back to the matter if it comes up again.
Flags: needinfo?(lhansen)
I've had the problem occurring 4 times in the past 2 days, always coming back from sleep mode.

I hadn't happened for a while, and it's back again. Running 46 developer edition, e10s on 10.11.3
I have it basically every time I plug/unplug my external monitors.

46.0a2, Mac 10.11.3, *no* e10s.
Component: Untriaged → Layout
Happens to me everytime I close my MacBook and re-open (with external monitors) or if my laptop goes to sleep.  For me this is THE bug.  I'm on 10.11.2 w/ 46.01
This is weird. I was having trouble reproducing it, and suspected it might be more prevalent on 10.11 (I'm running 10.9.5 locally); but now I have a Nightly session where it reproduces pretty consistently. But only in one of the several windows that I have open; and I can't see any reason why this one window should be affected, but the others seem immune.

And of course my attempts to reproduce with a debug build have so far been fruitless....
OK, I may have found something. It seems that in some cases, when there's a resolution change (e.g. due to plugging in an external lo-dpi monitor with mirroring enabled, so that the laptop's retina screen switches into a lo-dpi scaled mode, or when reversing this operation) we may end up calling nsViewManager::SetWindowDimensions with an incorrect size from nsDocumentViewer::SetBoundsWithFlags.

This happens if the presContext has not yet handled the resolution-change notification, with the result that it gets a stale AppUnitsPerDevPixel value, which it uses to convert the device pixel window size to appUnits.

The size from SetWindowDimensions eventually becomes the size used to reflow the root presContext, and so that reflow occurs with a size that is 2x (or 0.5x, depending which way the resolution change was happening) that it should be.

In my testing, at least, this behavior has been somewhat erratic/unpredictable, but easier to reproduce in a session with a number of windows and tabs open. It makes sense that it'd be timing-dependent, because when the screen resolution changes globally, there are a lot of widgets that receive change notifications, and the exact order these are received/handled, and how long that takes, is probably not deterministic.

A simple fix of checking for a DPI change before looking up AppUnitsPerDevPixel in SetBoundsWithFlags seems to prevent the problem, at least in my testing so far.

I've pushed a try build at https://treeherder.mozilla.org/#/jobs?repo=try&revision=b527f3ef4f43; once that's available, I'd appreciate anyone who is regularly seeing this problem giving it a try and reporting whether it helps.
This appears to prevent the problem in my testing. Let's see if others can confirm whether it fixes things for them, too.
Attachment #8766315 - Flags: review?(mstange)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Hey mhoye - I seem to recall you experiencing this bug a lot. Do you still see it, and if so, would you be willing to test jfkthame's build to see if the behaviour changes?
Flags: needinfo?(mhoye)
That try build fixed the issue for me.
I can't reproduce the issue with jfkthame's build, but the specific thing I can't reproduce is getting the child window to open on the other display, not the scaling issue that's presented when it does.
Flags: needinfo?(mhoye)
Comment on attachment 8766315 [details] [diff] [review]
Ensure the context has up-to-date device pixel DPI scaling before using it in calling nsViewManager::SetWindowDimensions

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

Seems reasonable. It's too bad that writing a test for this appears to be mostly impossible...
Attachment #8766315 - Flags: review?(mstange) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/068fa73fa918388dc076d78f257fbe8dc87b332f
Bug 1154125 - Ensure the context has up-to-date device pixel DPI scaling before using it in calling nsViewManager::SetWindowDimensions. r=mstange
By which I of course mean "Linux, Mac, and Windows" and "test_img_mutations.html and test_picture_mutations.html".
If we update the device-context resolution here, we also need to make sure the prescontext is informed so that it will notify all its children; I believe this should fix the mochitest failures above. Pushed a try run to check: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0a76ad83e34
Attachment #8766744 - Flags: review?(mstange)
Attachment #8766315 - Attachment is obsolete: true
Attachment #8766744 - Flags: review?(mstange) → review+
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/480bd162d285
Ensure the context has up-to-date device pixel DPI scaling before using it in calling nsViewManager::SetWindowDimensions. r=mstange
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/480bd162d285
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Duplicate of this bug: 1281528
See Also: → 1284718
What are the uplift chances for this patch / these patches?
Flags: needinfo?(jfkthame)
Duplicate of this bug: 1284718
Comment on attachment 8766744 [details] [diff] [review]
Ensure the context has up-to-date device pixel DPI scaling before using it in calling nsViewManager::SetWindowDimensions

Approval Request Comment
[Feature/regressing bug #]: hi-dpi support (but not a recent regression)

[User impact if declined]: occasionally broken display after screen configuration changes (e.g. attaching/detaching a monitor)

[Describe test coverage new/current, TreeHerder]: manually tested (see also the duplicate bugs where reporters confirm fixed in latest nightlies)

[Risks and why]: minimal, just adds a resolution-change check to ensure correct dpi is used when resizing view

[String/UUID change made/needed]: none
Flags: needinfo?(jfkthame)
Attachment #8766744 - Flags: approval-mozilla-aurora?
Duplicate of this bug: 1167670
Duplicate of this bug: 1279470
[Tracking Requested - why for this release]: Widely reported bug--let's uplift to 49 per comment 35
Comment on attachment 8766744 [details] [diff] [review]
Ensure the context has up-to-date device pixel DPI scaling before using it in calling nsViewManager::SetWindowDimensions

Fix a widely reported bug, taking it.
Attachment #8766744 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Duplicate of this bug: 1257071
You need to log in before you can comment on or make changes to this bug.