1205372 - Dragging windows between different content scale monitor causes glitchy adjustments

Status: NEW

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

Description

[Benoit Girard (:BenWa)]

+++ 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.

Comment 1

[Benoit Girard (:BenWa)]

AFAIK this is a long standing bug.

Comment 2

[Steven Michaud [:smichaud] (Retired)]

> 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.

Comment 3

[Benoit Girard (:BenWa)]

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.

Comment 4

[Benoit Girard (:BenWa)]

Attachment: 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.

Comment 5

[Benoit Girard (:BenWa)]

Attachment: patch (proper qref)

Comment 6

[Markus Stange [: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.

Comment 7

[Markus Stange [:mstange]]

Updating mBounds is definitely a good idea either way.

Comment 8

[Benoit Girard (:BenWa)]

(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.

Comment 9

[Benoit Girard (:BenWa)]

(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.

Comment 10

[Markus Stange [:mstange]]

(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.

Comment 11

[Tracy Walker [:tracy]]

This is still occurring with latest Nightly build. But less often than I used to see it.