532828 - Losing OS window focus causes a total repaint

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[Zack Weinberg (:zwol)]

We appear to repaint the entire content area when it loses OS-level input focus.  This is easiest demonstrated if you have a test case which tickles some other repaint bug -- right now, bug 523271 will do:

 - Load up the test case from that bug.
 - Click where it says "click here".
 - Note the improperly redrawn gradient in the background.
 - Select a different OS-level window, without obscuring the window with the test case
 - Note how the gradient has now been redrawn properly.

We should not have to do this: at most, certain sub-areas may have to be repainted (e.g. a visible text selection is often supposed to change color when the window it's in loses focus).  Not only is it inefficient, but it makes it harder to take screen shots of "didn't repaint everything we should" bugs!

Comment 1

[Zack Weinberg (:zwol)]

Bug number dyslexia strikes again.  I meant to refer to bug 532721.

Comment 2

[Markus Stange [:mstange]]

What platforms have you tested this on?
On Mac this is intentional. We change the appearance of native controls like <select> boxes, scrollbars and toolbars when the window becomes inactive, so we need some way to invalidate them. Tracking them individually seems hard.

Comment 3

[Zack Weinberg (:zwol)]

I've only tested on Linux.  I believe all three primary platforms need appearance changes to native controls and visible text selections, but I don't see any huge difficulty in tracking them individually - it would just be a frame tree walk, asking each one "do you need to repaint for window (in)activation?"

Comment 4

[Markus Stange [:mstange]]

Sounds good. At the moment this isn't possible on Mac because we can't turn off the repaint which is forced by the OS, but retained layers will solve that. In fact, part 10 in bug 564991 explicitly adds a whole-window repaint.

Comment 5

[Markus Stange [:mstange]]

Attachment: v1

Is nsDisplayThemedBackground::ComputeInvalidationRegion a good place for handling this?

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 810544 [details] [diff] [review]
v1

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

Nice! DLBI makes this easy.

::: layout/base/nsDisplayList.cpp
@@ +2364,5 @@
> +{
> +  nsPresContext* presContext = mFrame->PresContext();
> +  nsIPresShell* presShell = presContext->GetPresShell();
> +  nsIDocument* doc = presShell ? presShell->GetDocument() : nullptr;
> +  return doc && !doc->GetDocumentState().HasState(NS_DOCUMENT_STATE_WINDOW_INACTIVE); 

Just use mFrame->GetContent()->OwnerDoc(). No possibility of any nulls.

Comment 7

[Markus Stange [:mstange]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #6)
> Comment on attachment 810544 [details] [diff] [review]
> v1
> 
> Review of attachment 810544 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Nice! DLBI makes this easy.

Indeed!

> ::: layout/base/nsDisplayList.cpp
> @@ +2364,5 @@
> > +{
> > +  nsPresContext* presContext = mFrame->PresContext();
> > +  nsIPresShell* presShell = presContext->GetPresShell();
> > +  nsIDocument* doc = presShell ? presShell->GetDocument() : nullptr;
> > +  return doc && !doc->GetDocumentState().HasState(NS_DOCUMENT_STATE_WINDOW_INACTIVE); 
> 
> Just use mFrame->GetContent()->OwnerDoc(). No possibility of any nulls.

Nice, thanks.

https://hg.mozilla.org/integration/mozilla-inbound/rev/732798c42e48

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/732798c42e48