Closed Bug 532828 Opened 10 years ago Closed 6 years ago

Losing OS window focus causes a total repaint

Categories

(Core :: Web Painting, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: zwol, Assigned: mstange)

References

Details

Attachments

(1 file)

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!
Bug number dyslexia strikes again.  I meant to refer to bug 532721.
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.
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?"
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.
Depends on: 564991
Attached patch v1Splinter Review
Is nsDisplayThemedBackground::ComputeInvalidationRegion a good place for handling this?
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Attachment #810544 - Flags: review?(roc)
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.
Attachment #810544 - Flags: review?(roc) → review+
(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
https://hg.mozilla.org/mozilla-central/rev/732798c42e48
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Depends on: 941559
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.