Closed Bug 962528 Opened 10 years ago Closed 6 years ago

[hidpi] Ugly double redraw & rescale moving from lo-dpi to hi-dpi window

Categories

(Core :: Widget: Cocoa, defect, P2)

Product:
Core
Component:
Widget: Cocoa
Platform:
x86
macOS
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla63
Tracking Flags:
Tracking Status
firefox30 - ---
firefox63 --- fixed

People

(Reporter: gfritzsche, Assigned: smichaud)

References

(Blocks 1 open bug)

Details

(Whiteboard: tpi:+)

Attachments

(4 files)

Window dragged from non-retina to retina display
1.71 MB, image/png
Details
Window dragged from retina to non-retina
469.92 KB, image/png
Details
Screenshot from Apple's rdar://problem/21037762
7.12 MB, image/png
Details
Tentative patch
1.43 KB, patch
mstange
: review+
Details | Diff | Splinter Review 
I've got two active displays:
(1) Retina MBP display
(2) 2560x1440 Dell

Moving between displays results in highly visible rescaling and redrawing.
Especially moving from (2) to (1) (and ergo from lo- to hi-dpi) apparently:
* first redraws with the lo-dpi, resulting in very small text etc.
* then changes to hi-dpi and redraws again for that.

Moving from (1) to (2) also flickers but it's faster so it's hard to tell if it's the same issue.
Chrome also momentarily shows garbage as you move between displays, but I agree that Firefox's "garbage" is more visible.  Both are temporary, though.

Safari somehow manages not to show any garbage (at least in my tests, on OS X 10.7.5).

This is worth attention at some point, but I don't think it's particularly important.
As a side note, behavior of windows moving from one screen to another changed a lot since 10.9. They can't any more overlap 2 screens, which, since Apple implemented variable resolution displays, makes things much nicer from a program standpoint.

From what I could see, a window will only display on the screen the mouse pointer is on when dragging it, and is rendered with that screen DPI. While dragging, the rest of the window (which intersects other screens) is semi-transparent but rendered with the pointer's screen DPI (then up or down-scaled as needed on other screens), and when the drag finishes, those regions of the window disappear. Which leaves us with a unique DPI density for the whole window.

I don't know though at which level all this happens, or if my comment is of any help about this bug :-)
I filled a similar bug. The difference between us+Chrome is we use the OpenGL view and that Safari is using the native CoreAnimation layer to composite.

I suspect the garbage is a OS bug that doesn't handle the DPI switch correctly.

We'd have to look for a work around to maybe composite quickly (or even syncrounsly) when we drag from the screen.
(In reply to comment #2)

The new functionality on Mavericks is nice, but you still see garbage in both Firefox and Chrome while moving a window from one monitor to another.

You don't see as *much* garbage during the transition (at least in Firefox), but I also see this on OS X 10.8.5.  This OS-specific difference suggests that this really is (ultimately) an OS bug.  Though we may be able to learn ways to work around it (or them).
In my testing, none of the native Cocoa apps that I have on my system (including third party apps such as Limechat, VLC, Skype and Guitar Pro) are affected by this bug.

The Rdio client which I think is an app embedding WebView has a completely different behavior where the window snaps immediately from one display to the other without the fading behavior as it is being dragged.
It would be interesting to try with main thread compositing. I dont know if we supported retina and main thread compositing at the same time.
(In reply to comment #5)

Firefox isn't really a "native Cocoa app" :-)

We do a lot of stuff on our own that most "native Cocoa apps" let the OS do for them.  For example none of our UI objects are native objects, and we do our own drawing.
(In reply to comment #7)
> (In reply to comment #5)
> 
> Firefox isn't really a "native Cocoa app" :-)
> 
> We do a lot of stuff on our own that most "native Cocoa apps" let the OS do for
> them.  For example none of our UI objects are native objects, and we do our own
> drawing.

Yes *I* understand that.  I was trying to demo what a normal user would see if they compared the behavior of a few apps with regards to this.
(In reply to comment #8)

Fair enough.

If someone wants to work on this, please feel free.  I didn't mean to put people off that by what I said in comment #1.  But I don't think *I'll* be working on this anytime soon.
I'm noticing substantially worse corruption Nightly since roughly the middle of last week; on OS X 10.8.5 my window doesn't get redrawn as I drag between retina to non-retina screens - see attached screenshots.

I'll look for a regression range.
Note that the actual UI hit points are correctly scaled for the new screen; if you mouse over where the tab *should* be displayed, the border as it is wrongly displayed highlights.

The display corrects itself with any full redraw (e.g. resizing the window).
I think we should aim to fix it for this release or sooner. This type of glitching is below our quality bar.
tracking-firefox30: --- → ?
> This type of glitching is below our quality bar.

I disagree about the state of the glitching before the latest developments.

Irving, please find the regression range in mozilla-central nightlies.  And if possible find exactly which patch triggered the problem.
This regression didn't reproduce under a clean profile, so I ended up having to bisect under my messy default profile. The change it identified is:

The first bad revision is:
changeset:   168253:08b4b4b4aef3
user:        Bill McCloskey <wmccloskey@mozilla.com>
date:        Tue Feb 11 09:01:08 2014 -0800
summary:     Bug 960783 - Support "new out-of-process window" menu item in nightly (r=felipe,bsmedberg,trevor,ted)
(In reply to comment #16)

So is it fair to say that you only see the especially bad glitching when electrolysis is on?

If so, then this should definitely *not* block FF 30.
The extra bad glitching from Comment 10 is definitely triggered by something in my profile; I did a 'Reset Firefox' and the cleaned up profile isn't failing.

I did not deliberately turn on e10s - the profile that reproduces the problem has the default settings for browser.tabs.remote* and I had never used 'New e10s Window'.

(that said, when I do use 'New e10s Window', and drag that window from regular to HiDPI, the content of the e10s window doesn't change resolution so it looks tiny on the HiDPI screen)

I'll try to isolate what in my old profile was causing my glitch.
Found it: the glitching profile has layers.acceleration.disabled = true (not sure why I had changed that...)

Reproduced with a clean profile; set layers.acceleration.disabled to true and restarted, the glitch shows up. Changed back to 'false', restarted, the normal (slightly glitchy) behaviour returns.
Thanks, Irving.  I'll try to repro later.

Benoit, do you have any idea why this setting should trigger worse glitching, but only after the patch for bug 960783 landed?
Flags: needinfo?(bgirard)
This e10s case would be a very different code path. My guess: It's doesn't trigger a repaint+recomposite properly.
Flags: needinfo?(bgirard)
I've reproduced this bug on OS X 10.7.5, with today's mozilla-central nightly.  I tested with a clean profile, in which I'd unchecked Preferences : Advanced : General : Use hardware acceleration when available.

These new glitches are *permanent*, not temporary like the ones originally reported.  This wasn't noted clearly enough above.  So yes, these should never get into a release.

Though these happen with what's supposedly a non-e10s window, I'm convinced that e10s code is involved here -- the bug doesn't happen if you set both browser.tabs.remote and browser.tabs.remote.autostart to false.  I'll try to find exactly what code this is.

Side note:  Is there any way in the UI to distinguish an e10s window from a non-e10s window?  I haven't been able to find one.
> These new glitches are *permanent*, not temporary like the ones originally reported.

They don't disappear until you explicitly make the window redraw -- for example by resizing it.
(In reply to Steven Michaud from comment #22)
> These new glitches are *permanent*, not temporary like the ones originally
> reported.

Then we should spin this off to a new bug since this isn't the original problem. I imagine this investigation will make the problem go from permanent to temporary so it doesn't match this original description.

> Side note:  Is there any way in the UI to distinguish an e10s window from a
> non-e10s window?  I haven't been able to find one.

When displaying web content (non chrome page) the title in the tab will be unlined to indicate the tab is being filled by a content process.
> Then we should spin this off to a new bug since this isn't the original problem.

I'll do that.
underlined*
I've opened bug 974616.
Lets go ahead and track this for now
At this stage, with no sign of significant user input about seeing this glitch (and generally I expect we don't see this much with average users) this is not a tracking-worthy issue but we can consider a low-risk uplift when something is available.
tracking-firefox30: + → -
(In reply to Benoit Girard (:BenWa) from comment #3)
> I filled a similar bug. The difference between us+Chrome is we use the
> OpenGL view and that Safari is using the native CoreAnimation layer to
> composite.
> 
> I suspect the garbage is a OS bug that doesn't handle the DPI switch
> correctly.
> 
> We'd have to look for a work around to maybe composite quickly (or even
> syncrounsly) when we drag from the screen.

Chrome has now fixed this. We should too.
> Chrome has now fixed this. We should too.

I agree that Chrome's behavior is now like Safari's.

But do you have any idea *how* Chrome has fixed this?
No but since they are also using a OpenGL view we should be able to fix it the same way.

If we're lucky maybe we can find how it was fix on their tracker and/or change log.
I'll look.  I've noticed that Chromium r197479 (from late May) also has the new behavior, which will help.  (Sometimes changes are made to Chrome that don't end up in Chromium.)

I've been working on bug 984200, which may be difficult/impossible to fix without also fixing this bug.  At least for the time being, I'll try working on both at once.
Assignee: nobody → smichaud
Still seeing this behavior. Just a ping to check that someone is still working on it.
I haven't been working on it, but with luck I will be in the not too distant future.

This will be a major undertaking, involving all kinds of reverse engineering (of Safari, Chrome and OS X itself).  I'm currently the only Mozilla developer really qualified to do the work.  But I'm planning to retire sometime later this year, and I don't want to leave Mozilla without anyone who can do this kind of stuff.

So I hope to work with another Mozilla developer on this (I have someone in mind), as a way to pass the torch.
Hi, I would file another bug but my issue seems to be an edge case of this one.

To repo on 40.0a2 (2015-05-25), OSX 10.10.3:
1. Open two tabs on a lowdpi/hidpi screen.
2. Drag one tab from it's parent window to another screen so that a new window is spawned.
3. The webpage content should be drawn in the parent dpi.

Couldn't repro on stable. Works both ways. Sreenshots:
http://grab.by/Hyf4
http://grab.by/Hyfa
I can reproduce this on nightly, plugging in Mac Book Pro Retina to iMac, and going to nytimes.com.
Apple contacted us with this problem, tracked in rdar://problem/21037762, but I don't have a lot of other details.
(In reply to comment #38)

This bug is about temporary redrawing artifacts.  Apple's bug seems to be about a "permanent" one (that only an explicit action, like resizing the window, can make disappear).  If Apple ever provides us with more detail, that will need to go into a new, separate bug.

Without more detail there's nothing we can do ... other than eventually try to fix this bug.
See Also: → 1205372
Priority: -- → P2
Whiteboard: tpi:+
Attached patch Tentative patch — — Splinter Review 
I got lucky today and captured this broken behavior in a profile: https://perfht.ml/2KuIsIz

What I see happening in this profile:

- first there's a MozUpdateWindowPos event, likely triggered by the move of the window.

- then we have some relevant code with this stack, starting a runnable to notify the prescontext of the resolution change:
nsPresContext::UIResolutionChanged()
nsChildView::BackingScaleFactorChanged()
-[ChildView viewDidChangeBackingProperties]
_NSViewHierarchyDidChangeBackingProperties
-[NSWindow _postWindowDidChangeBackingPropertiesAndDisplayWindowForPreviousBackingScaleFactor:previousColorSpace:]
__67-[NSWindow _updateSettingsSendingScreenChangeNotificationIfNeeded:]_block_invoke
NSPerformVisuallyAtomicChange
-[NSWindow _updateSettingsSendingScreenChangeNotificationIfNeeded:]
-[NSWindow _setFrame:updateBorderViewSize:]
-[NSWindow _setFrame:]
-[NSWindow _windowMovedToRect:]
-[NSWindow _windowMoved:]

- then we have a 16ms Msg_FlushRendering message, which (I think) means we paint synchronously.

- then there's a refresh driver tick, with a long 22ms restyle (due to an invalidation caused by nsPresContext::UIResolutionChangedInternalScale, which is the runnable started earlier, that runs after we have painted once, but before the refresh driver tick).

- then we paint again with an Msg_FlushRendering message of 115ms.

I think to fix this bug, the thing we need to do is ensure nsPresContext::UIResolutionChangedInternalScale runs before we paint. The attached tentative patch does it by calling nsPresContext::UIResolutionChangedSync instead of nsPresContext::UIResolutionChanged to avoid starting a runnable.

In my testing the attached patch fixes the bug for the parent process (ie. the tab bar and navigation toolbar and never painted at the wrong size), but we still have a similar problem on web pages (there's no problem when the browser displays a page loaded in the parent process, eg. about:performance). I would say this is an e10s related problem and could be bug 1156339.

I don't know why this was initially done with a runnable, maybe there was a reason and my patch would cause regressions, that's the reason why I called it a 'tentative patch'.
Attachment #8988529 - Flags: review?(mstange)
Comment on attachment 8988529 [details] [diff] [review]
Tentative patch

It looks like the asynchronicity was part of the initial implementation of this in bug 794038 comment 19. I think you're right and this call should be synchronous instead. It looks like a similar problem was encountered in bug 1125325 (just search for "async"...).
Attachment #8988529 - Flags: review?(mstange) → review+
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/701591b9d48f
Update the UI resolution synchronously to avoid painting the UI once at the wrong resolution when moving a window across screens of different resolutions, r=mstange.
https://hg.mozilla.org/mozilla-central/rev/701591b9d48f
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: