Closed Bug 498579 Opened 15 years ago Closed 4 years ago

Possible issues with double-painting

Categories

(Core :: Widget: Cocoa, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: bzbarsky, Unassigned)

References

Details

(Keywords: perf)

Running the testcase in bug 497788, I see us spending a lot of time (30% or so) in painting code.  The interesting thing s that we seem to enter it through two different callstacks, each accounting for about half the painting time.  When I put printfs into nsViewManager::Refresh and the testcase code, I see us often doing refresh twice per single testcase iteration.

The relevant stacks for getting to __CFRunLoopDoObservers:

start
_start
main
XRE_main
nsAppStartup::Run()
nsAppShell::Run()
-[NSApplication run]
-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
_DPSNextEvent
BlockUntilNextEventMatchingListInMode
ReceiveNextEventCommon
RunCurrentEventLoopInMode
CFRunLoopRunInMode
CFRunLoopRunSpecific
nsAppShell::ProcessGeckoEvents(void*)
nsBaseAppShell::NativeEventCallback()
NS_ProcessPendingEvents_P(nsIThread*, unsigned int)
nsThread::ProcessNextEvent(int, int*)
nsAppShell::OnProcessNextEvent(nsIThreadInternal*, int, unsigned int)
nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, int, unsigned int)
nsAppShell::ProcessNextNativeEvent(int)
-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
_DPSNextEvent
BlockUntilNextEventMatchingListInMode
ReceiveNextEventCommon
RunCurrentEventLoopInMode
CFRunLoopRunInMode
CFRunLoopRunSpecific
__CFRunLoopDoObservers

and 

start
_start
main
XRE_main
nsAppStartup::Run()
nsAppShell::Run()
-[NSApplication run]
-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
_DPSNextEvent
BlockUntilNextEventMatchingListInMode
ReceiveNextEventCommon
RunCurrentEventLoopInMode
CFRunLoopRunInMode
CFRunLoopRunSpecific
__CFRunLoopDoObservers

and from there the stack to nsViewManager::Refresh is identical:

_handleWindowNeedsDisplay
-[NSWindow displayIfNeeded]
-[NSView displayIfNeeded]
-[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:]
-[NSThemeFrame _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]
-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]
-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]
-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]
-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]
-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]
-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]
-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]
-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]
-[NSView _drawRect:clip:]
-[ChildView drawRect:]
nsChildView::DispatchWindowEvent(nsGUIEvent&)
nsChildView::DispatchEvent(nsGUIEvent*, nsEventStatus&)
HandleEvent(nsGUIEvent*)
nsViewManager::DispatchEvent(nsGUIEvent*, nsEventStatus*)
nsViewManager::Refresh(nsView*, nsIRenderingContext*, nsIRegion*, unsigned int)
Depends on: 516732
I'm not too worried about the different callstacks, as noted in bug 516732 comment 7 / 8, but this is intriguing:

(In reply to comment #0)
> When I
> put printfs into nsViewManager::Refresh and the testcase code, I see us often
> doing refresh twice per single testcase iteration.

Could you test this again with the patch for bug 517804 included?

(In reply to Markus Stange [:mstange] from comment #1)

...
Could you test this again with the patch for bug 517804 included?

Shouldn't this issue be gone?

Flags: needinfo?(spohl.mozilla.bugs)

This question is better directed at :bzbarsky.

Flags: needinfo?(spohl.mozilla.bugs) → needinfo?(bzbarsky)

I don't know, sorry. In a current build nsViewManager::Refresh seems to not be called at all, pretty much, in this testcase. And in general the drawing and invalidation codepaths have changed a lot since 2009.

So it's entirely possible this is fixed now; I just don't know how to check.

Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(bzbarsky)
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.