Dragging windows between different content scale monitor causes glitchy adjustments

NEW
Unassigned

Status

()

Core
Widget: Cocoa
P3
normal
2 years ago
22 days ago

People

(Reporter: BenWa, Unassigned)

Tracking

Trunk
All
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox40 affected, firefox41 affected, firefox42 affected, firefox43 affected)

Details

(Whiteboard: tpi:+)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
+++ This bug was initially created as a clone of Bug #1189565 +++

STR:
1) Get a dual monitor setup with a x1 and x2 content scale monitor.
2) Open the browser on one monitor, drag it to the other.

This causes several glitchy adjustment frames as the window eventually fully assumes the new content scale.

NOTE: This bug is specifically scoping out the garbage frame we get during the transition which is likely another bug.
(Reporter)

Updated

2 years ago
Component: Widget → Widget: Cocoa
(Reporter)

Updated

2 years ago
See Also: → bug 1189565
(Reporter)

Comment 1

2 years ago
AFAIK this is a long standing bug.
status-firefox40: --- → affected
status-firefox41: --- → affected
status-firefox42: --- → affected
status-firefox43: --- → affected
> AFAIK this is a long standing bug.

Yes, it certainly is.  I was going to work on it at bug 962528, if I had the time.  But I'm going to retire in two weeks, so I never will.
See Also: → bug 962528
(Reporter)

Comment 3

2 years ago
Thanks for the help Steven, your patch was a great step in the right direction! I appreciate the smooth hand-off.

It looks like this patch will only help in the non-e10s case and/or will just fix an issue with the chrome transparency content scale. I think this bug might be of a very very narrow scope.

In any case the widget doesn't handle changing it's size properly and fixing this is a great first step.
(Reporter)

Comment 4

2 years ago
Created attachment 8661961 [details] [diff] [review]
patch

This patch fixes the transparency in the chrome being wrong (top left hand side) and should help the widget handle its size better.

I'm not sure if the changes need to be applied to nsCocoaWindow or not? To fix the bug it seems that the nsChildView changes are sufficient.

Also the ReportSizeEvent doesn't trigger a reflow in nsViewManager.cpp like I'd expect. It seems like we think the sizes are the same and skip the reflow. Forcing fixes the problem. nscoord should be different with a different content scale so perhaps I found the wrong spot I'm not sure.

I could use a bit of feedback here.
Attachment #8661961 - Flags: feedback?(mstange)
(Reporter)

Comment 5

2 years ago
Created attachment 8661962 [details] [diff] [review]
patch (proper qref)
Attachment #8661961 - Attachment is obsolete: true
Attachment #8661961 - Flags: feedback?(mstange)
Attachment #8661962 - Flags: feedback?(mstange)
(In reply to Benoit Girard (:BenWa) from comment #4)
> nscoord should be different with a different content scale

The window size in device pixels changes, but the size in CSS pixels (and thus in app units (nscoords)) stays the same.

Do you know why the reflow is required?

Updating the vibrant regions happens in nsChildView::UpdateVibrancy, which is called through nsChildView::UpdateThemeGeometries at the end of painting. So in theory, a paint should be all that's needed... It would be good to find out why it's not.
Updating mBounds is definitely a good idea either way.
(Reporter)

Comment 8

2 years ago
(In reply to Markus Stange [:mstange] from comment #6)
> (In reply to Benoit Girard (:BenWa) from comment #4)
> > nscoord should be different with a different content scale
> 
> The window size in device pixels changes, but the size in CSS pixels (and
> thus in app units (nscoords)) stays the same.
> 
> Do you know why the reflow is required?
> 
> Updating the vibrant regions happens in nsChildView::UpdateVibrancy, which
> is called through nsChildView::UpdateThemeGeometries at the end of painting.
> So in theory, a paint should be all that's needed... It would be good to
> find out why it's not.

My theory is that:
1) We reflow with the right size but with the wrong content scale (before it's been updated) and we don't reflow because we only compare the size and see it hasn't changed
2) We reflow with the wrong size with the wrong content scale but both mistakes cancel out and we think we've reflowed.

I'm not sure I have the right spot is where this should occurs. The ReportSizeEvent is reach there so I'd expect it to be a reasonable guess.

If we have no idea I can keep looking.
(Reporter)

Comment 9

2 years ago
(In reply to Benoit Girard (:BenWa) from comment #8)
> I'm not sure I have the right spot is where this should occurs. The
> ReportSizeEvent is reach there so I'd expect it to be a reasonable guess.
> 

Sorry for the broken english:
I'm not sure I have the right spot where this should be occurring. ReportSizeEvent calls that function so I'd expect it to be a reasonable guess.
(In reply to Benoit Girard (:BenWa) from comment #8)
> My theory is that:
> 1) We reflow with the right size but with the wrong content scale (before
> it's been updated) and we don't reflow because we only compare the size and
> see it hasn't changed
> 2) We reflow with the wrong size with the wrong content scale but both
> mistakes cancel out and we think we've reflowed.

Sounds plausible. Can you find out how nsViewManager computes the app unit size? Ideally, at the point when it does that, it should have both the new devPixel size and the new content scale at the same time, and both should cancel out to keep the size in app units the same, so that no reflow is needed.
(Reporter)

Updated

2 years ago
Depends on: 1210180
Attachment #8661962 - Flags: feedback?(mstange)
This is still occurring with latest Nightly build. But less often than I used to see it.
Priority: -- → P3
Whiteboard: tpi:+
Version: 42 Branch → Trunk
You need to log in before you can comment on or make changes to this bug.