diff --git a/docs/performance/bestpractices.md b/docs/performance/bestpractices.md index 26279171bc65..a64bc7d8b766 100644 --- a/docs/performance/bestpractices.md +++ b/docs/performance/bestpractices.md @@ -395,7 +395,7 @@ the dimensions of the window without risking a synchronous reflow. Returns the window's scroll offsets without taking the chance of causing a sync reflow. -### Writing tests to ensure you don’t add more unintentional reflow +### Writing tests to ensure you don’t add more unintentional reflow The interface [nsIReflowObserver](https://dxr.mozilla.org/mozilla-central/source/docshell/base/nsIReflowObserver.idl) @@ -409,7 +409,7 @@ while those actions occur. You should add tests like this for your feature if you happen to be touching the DOM. -## Detecting over-painting with paint flashing +## Detecting over-painting Painting is, in general, cheaper than both style calculation and layout calculation; still, the more you can avoid, the better. Generally @@ -417,61 +417,17 @@ speaking, the larger an area that needs to be repainted, the longer it takes. Similarly, the more things that need to be repainted, the longer it takes. -Our graphics team has added a handy feature to help you detect when and -where paints are occurring. This feature is called “paint flashing,” and -it can be activated for both web content and the browser chrome. Paint -flashing tints each region being painted with a randomly selected color -so that it’s more easy to see what on the screen is being painted. +If a profile says a lot of time is spent in painting or display-list building, +and you're unsure why, consider talking to our always helpful graphics team in +the [gfx room](https://chat.mozilla.org/#/room/%23gfx:mozilla.org) on +[Matrix](https://wiki.mozilla.org/Matrix), and they can probably advise you. -- You can activate paint flashing for browser chrome by setting - *nglayout.debug.paint_flashing_chrome* to *true*. +Note that a significant number of the graphics team members are in the US +Eastern Time zone (UTC-5 or UTC-4 during Daylight Saving Time), so let that +information guide your timing when you ask questions in the +[gfx room](https://chat.mozilla.org/#/room/%23gfx:mozilla.org). -- You can activate paint flashing for web content by setting - *nglayout.debug.paint_flashing* to *true*. - -After enabling these, exercise your function and see what’s painting. -See a lot of flashing / colors? That means a lot of painting is going -on. The worst case is called **over-painting**. This is when you draw -multiple times over the same space. Unless transparency is involved, all -but the last painting will be overwritten, becoming unnecessary. If you -can find ways to avoid doing this, you can save substantial time. - -Keep in mind that painting occurs on the main thread. Remember, too, -that the goal is to have as little happen on the main thread as -possible. That means that finding and removing (when possible) -over-painting is a good place to start reducing your burden on the main -thread, which will in turn improve performance. - -Perhaps you’re animating something that requires a repaint? For example, -transitioning the *background-color* of a DOM node from red to blue -will result in a repaint for every frame of the animation, and paint -flashing will reveal that. Consider using a different animation that can -be accelerated by the GPU. These GPU-accelerated animations occur off of -the main thread, and have a much higher probability of running at 60 FPS -(see the section below called [Use the compositor for animations](#use-the-compositor-for-animations) -for further details). - -Perhaps you’re touching some DOM nodes in such a way that unexpected -repaints are occurring in an area that don’t need it. Best to -investigate and try to remove those as best you can. Sometimes, our -graphics layer invalidates regions in ways that might not be clear to -you, and a section outside of the thing that just repainted will also -repaint. Sometimes this can be addressed by ensuring that the thing -changing is on its own layer (though this comes at a memory cost). You -can put something on its own layer by setting its *z-index*, or by -setting the *will-change* on the node, though this should be used -sparingly. - -If you’re unsure why something is repainting, consider talking to our -always helpful graphics team in the [gfx room](https://chat.mozilla.org/#/room/%23gfx:mozilla.org) on -[Matrix](https://wiki.mozilla.org/Matrix), and they can probably -advise you. Note that a significant number of the graphics team members -are in the US Eastern Time zone (UTC-5 or UTC-4 during Daylight Saving -Time), so let that information guide your timing when you ask questions -in the [gfx room](https://chat.mozilla.org/#/room/%23gfx:mozilla.org) -. - -## Adding nodes using DocumentFragments +## Adding nodes using DocumentFragments Sometimes you need to add several DOM nodes as part of an existing DOM tree. For example, when using XUL *\s*, you often have diff --git a/dom/base/nsDOMWindowUtils.cpp b/dom/base/nsDOMWindowUtils.cpp index 4a169c209e87..be5a841ba2ec 100644 --- a/dom/base/nsDOMWindowUtils.cpp +++ b/dom/base/nsDOMWindowUtils.cpp @@ -3869,33 +3869,6 @@ nsDOMWindowUtils::IsNodeDisabledForEvents(nsINode* aNode, bool* aRetVal) { return NS_OK; } -NS_IMETHODIMP -nsDOMWindowUtils::SetPaintFlashing(bool aPaintFlashing) { - nsPresContext* presContext = GetPresContext(); - if (presContext) { - presContext->SetPaintFlashing(aPaintFlashing); - // Clear paint flashing colors - PresShell* presShell = GetPresShell(); - if (!aPaintFlashing && presShell) { - nsIFrame* rootFrame = presShell->GetRootFrame(); - if (rootFrame) { - rootFrame->InvalidateFrameSubtree(); - } - } - } - return NS_OK; -} - -NS_IMETHODIMP -nsDOMWindowUtils::GetPaintFlashing(bool* aRetVal) { - *aRetVal = false; - nsPresContext* presContext = GetPresContext(); - if (presContext) { - *aRetVal = presContext->GetPaintFlashing(); - } - return NS_OK; -} - NS_IMETHODIMP nsDOMWindowUtils::DispatchEventToChromeOnly(EventTarget* aTarget, Event* aEvent, bool* aRetVal) { diff --git a/dom/interfaces/base/nsIDOMWindowUtils.idl b/dom/interfaces/base/nsIDOMWindowUtils.idl index 62b6fc8e0a04..fb5efd4252a3 100644 --- a/dom/interfaces/base/nsIDOMWindowUtils.idl +++ b/dom/interfaces/base/nsIDOMWindowUtils.idl @@ -1909,11 +1909,6 @@ interface nsIDOMWindowUtils : nsISupports { */ boolean isNodeDisabledForEvents(in Node aNode); - /** - * Setting paintFlashing to true will flash newly painted area. - */ - attribute boolean paintFlashing; - /* * Returns the value of a given property animated on the compositor thread. * If the property is NOT currently being animated on the compositor thread, diff --git a/gfx/layers/wr/WebRenderCommandBuilder.cpp b/gfx/layers/wr/WebRenderCommandBuilder.cpp index 55a88f9f2482..33e1cba49e5a 100644 --- a/gfx/layers/wr/WebRenderCommandBuilder.cpp +++ b/gfx/layers/wr/WebRenderCommandBuilder.cpp @@ -2058,22 +2058,12 @@ static void PaintItemByDrawTarget(nsDisplayItem* aItem, gfx::DrawTarget* aDT, break; } - if (aItem->GetType() != DisplayItemType::TYPE_MASK) { + if (aHighlight && aItem->GetType() != DisplayItemType::TYPE_MASK) { // Apply highlight fills, if the appropriate prefs are set. // We don't do this for masks because we'd be filling the A8 mask surface, // which isn't very useful. - if (aHighlight) { - aDT->SetTransform(gfx::Matrix()); - aDT->FillRect(Rect(visibleRect), gfx::ColorPattern(aHighlight.value())); - } - if (aItem->Frame()->PresContext()->GetPaintFlashing()) { - aDT->SetTransform(gfx::Matrix()); - float r = float(rand()) / float(RAND_MAX); - float g = float(rand()) / float(RAND_MAX); - float b = float(rand()) / float(RAND_MAX); - aDT->FillRect(Rect(visibleRect), - gfx::ColorPattern(gfx::DeviceColor(r, g, b, 0.5))); - } + aDT->SetTransform(gfx::Matrix()); + aDT->FillRect(Rect(visibleRect), gfx::ColorPattern(aHighlight.value())); } } diff --git a/layout/base/nsPresContext.cpp b/layout/base/nsPresContext.cpp index 2fc3001d5c34..e5b5f561f1d6 100644 --- a/layout/base/nsPresContext.cpp +++ b/layout/base/nsPresContext.cpp @@ -276,8 +276,6 @@ nsPresContext::nsPresContext(dom::Document* aDocument, nsPresContextType aType) mFontFeatureValuesDirty(true), mSuppressResizeReflow(false), mIsVisual(false), - mPaintFlashing(false), - mPaintFlashingInitialized(false), mHasWarnedAboutPositionedTableParts(false), mHasWarnedAboutTooLargeDashedOrDottedRadius(false), mQuirkSheetAdded(false), @@ -331,8 +329,6 @@ static const char* gExactCallbackPrefs[] = { "intl.accept_languages", "layout.css.devPixelsPerPx", "layout.css.dpi", - "nglayout.debug.paint_flashing_chrome", - "nglayout.debug.paint_flashing", "privacy.resistFingerprinting", "privacy.trackingprotection.enabled", nullptr, @@ -667,12 +663,6 @@ void nsPresContext::PreferenceChanged(const char* aPrefName) { } } - if (prefName.EqualsLiteral("nglayout.debug.paint_flashing") || - prefName.EqualsLiteral("nglayout.debug.paint_flashing_chrome")) { - mPaintFlashingInitialized = false; - return; - } - // We will end up calling InvalidatePreferenceSheets one from each pres // context, but all it's doing is clearing its cached sheet pointers, so it // won't be wastefully recreating the sheet multiple times. @@ -2657,18 +2647,6 @@ void nsPresContext::NotifyDOMContentFlushed() { } } -bool nsPresContext::GetPaintFlashing() const { - if (!mPaintFlashingInitialized) { - bool pref = Preferences::GetBool("nglayout.debug.paint_flashing"); - if (!pref && IsChrome()) { - pref = Preferences::GetBool("nglayout.debug.paint_flashing_chrome"); - } - mPaintFlashing = pref; - mPaintFlashingInitialized = true; - } - return mPaintFlashing; -} - nscoord nsPresContext::GfxUnitsToAppUnits(gfxFloat aGfxUnits) const { return mDeviceContext->GfxUnitsToAppUnits(aGfxUnits); } diff --git a/layout/base/nsPresContext.h b/layout/base/nsPresContext.h index 46a30cb7b65c..2459b3203f8a 100644 --- a/layout/base/nsPresContext.h +++ b/layout/base/nsPresContext.h @@ -899,16 +899,6 @@ class nsPresContext : public nsISupports, public mozilla::SupportsWeakPtr { // Is this presentation in a chrome docshell? bool IsChrome() const; - // Explicitly enable and disable paint flashing. - void SetPaintFlashing(bool aPaintFlashing) { - mPaintFlashing = aPaintFlashing; - mPaintFlashingInitialized = true; - } - - // This method should be used instead of directly accessing mPaintFlashing, - // as that value may be out of date when mPaintFlashingInitialized is false. - bool GetPaintFlashing() const; - bool SuppressingResizeReflow() const { return mSuppressResizeReflow; } gfxUserFontSet* GetUserFontSet(); @@ -1354,11 +1344,6 @@ class nsPresContext : public nsISupports, public mozilla::SupportsWeakPtr { unsigned mIsVisual : 1; - // Should we paint flash in this context? Do not use this variable directly. - // Use GetPaintFlashing() method instead. - mutable unsigned mPaintFlashing : 1; - mutable unsigned mPaintFlashingInitialized : 1; - unsigned mHasWarnedAboutPositionedTableParts : 1; unsigned mHasWarnedAboutTooLargeDashedOrDottedRadius : 1; diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 20d57ff2cb3d..ed580368e3ab 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -929,11 +929,6 @@ pref("view_source.editor.args", ""); // whether or not to draw images while dragging pref("nglayout.enable_drag_images", true); -// enable/disable paint flashing --- useful for debugging -// the first one applies to everything, the second one only to chrome -pref("nglayout.debug.paint_flashing", false); -pref("nglayout.debug.paint_flashing_chrome", false); - // URI fixup prefs pref("browser.fixup.alternate.enabled", true); pref("browser.fixup.alternate.prefix", "www."); diff --git a/widget/gtk/nsWindow.cpp b/widget/gtk/nsWindow.cpp index c158dc26dc5b..bb61e8b9daed 100644 --- a/widget/gtk/nsWindow.cpp +++ b/widget/gtk/nsWindow.cpp @@ -3388,74 +3388,6 @@ bool nsWindow::HasPendingInputEvent() { return haveEvent; } -#if 0 -# ifdef DEBUG -// Paint flashing code (disabled for cairo - see below) - -# define CAPS_LOCK_IS_ON \ - (KeymapWrapper::AreModifiersCurrentlyActive(KeymapWrapper::CAPS_LOCK)) - -# define WANT_PAINT_FLASHING (debug_WantPaintFlashing() && CAPS_LOCK_IS_ON) - -# ifdef MOZ_X11 -static void -gdk_window_flash(GdkWindow * aGdkWindow, - unsigned int aTimes, - unsigned int aInterval, // Milliseconds - GdkRegion * aRegion) -{ - gint x; - gint y; - gint width; - gint height; - guint i; - GdkGC * gc = 0; - GdkColor white; - - gdk_window_get_geometry(aGdkWindow,nullptr,nullptr,&width,&height); - - gdk_window_get_origin (aGdkWindow, - &x, - &y); - - gc = gdk_gc_new(gdk_get_default_root_window()); - - white.pixel = WhitePixel(gdk_display,DefaultScreen(gdk_display)); - - gdk_gc_set_foreground(gc,&white); - gdk_gc_set_function(gc,GDK_XOR); - gdk_gc_set_subwindow(gc,GDK_INCLUDE_INFERIORS); - - gdk_region_offset(aRegion, x, y); - gdk_gc_set_clip_region(gc, aRegion); - - /* - * Need to do this twice so that the XOR effect can replace - * the original window contents. - */ - for (i = 0; i < aTimes * 2; i++) - { - gdk_draw_rectangle(gdk_get_default_root_window(), - gc, - TRUE, - x, - y, - width, - height); - - gdk_flush(); - - PR_Sleep(PR_MillisecondsToInterval(aInterval)); - } - - gdk_gc_destroy(gc); - - gdk_region_offset(aRegion, -x, -y); -} -# endif /* MOZ_X11 */ -# endif // DEBUG -#endif - #ifdef cairo_copy_clip_rectangle_list # error "Looks like we're including Mozilla's cairo instead of system cairo" #endif diff --git a/widget/nsBaseWidget.cpp b/widget/nsBaseWidget.cpp index 539ba8e7fdbb..ec7191c195e2 100644 --- a/widget/nsBaseWidget.cpp +++ b/widget/nsBaseWidget.cpp @@ -2948,9 +2948,7 @@ static PrefPair debug_PrefValues[] = { {"nglayout.debug.event_dumping", false}, {"nglayout.debug.invalidate_dumping", false}, {"nglayout.debug.motion_event_dumping", false}, - {"nglayout.debug.paint_dumping", false}, - {"nglayout.debug.paint_flashing", false}, - {"nglayout.debug.paint_flashing_chrome", false}}; + {"nglayout.debug.paint_dumping", false}}; ////////////////////////////////////////////////////////////// bool nsBaseWidget::debug_GetCachedBoolPref(const char* aPrefName) { @@ -3033,11 +3031,6 @@ static int32_t _GetPrintCount() { } ////////////////////////////////////////////////////////////// /* static */ -bool nsBaseWidget::debug_WantPaintFlashing() { - return debug_GetCachedBoolPref("nglayout.debug.paint_flashing"); -} -////////////////////////////////////////////////////////////// -/* static */ void nsBaseWidget::debug_DumpEvent(FILE* aFileOut, nsIWidget* aWidget, WidgetGUIEvent* aGuiEvent, const char* aWidgetName, int32_t aWindowID) { diff --git a/widget/nsBaseWidget.h b/widget/nsBaseWidget.h index 5df0c4dccf79..60eb466bd91b 100644 --- a/widget/nsBaseWidget.h +++ b/widget/nsBaseWidget.h @@ -720,7 +720,6 @@ class nsBaseWidget : public nsIWidget, public nsSupportsWeakReference { protected: static nsAutoString debug_GuiEventToString( mozilla::WidgetGUIEvent* aGuiEvent); - static bool debug_WantPaintFlashing(); static void debug_DumpInvalidate(FILE* aFileOut, nsIWidget* aWidget, const LayoutDeviceIntRect* aRect, diff --git a/widget/windows/nsWindowGfx.cpp b/widget/windows/nsWindowGfx.cpp index 22f1effbeaf4..0d277aad1868 100644 --- a/widget/windows/nsWindowGfx.cpp +++ b/widget/windows/nsWindowGfx.cpp @@ -215,17 +215,6 @@ bool nsWindow::OnPaint(HDC aDC, uint32_t aNestingLevel) { } #endif -#ifdef WIDGET_DEBUG_OUTPUT - HRGN debugPaintFlashRegion = nullptr; - HDC debugPaintFlashDC = nullptr; - - if (debug_WantPaintFlashing()) { - debugPaintFlashRegion = ::CreateRectRgn(0, 0, 0, 0); - ::GetUpdateRgn(mWnd, debugPaintFlashRegion, TRUE); - debugPaintFlashDC = ::GetDC(mWnd); - } -#endif // WIDGET_DEBUG_OUTPUT - HDC hDC = aDC ? aDC : (::BeginPaint(mWnd, &ps)); mPaintDC = hDC; @@ -370,22 +359,6 @@ bool nsWindow::OnPaint(HDC aDC, uint32_t aNestingLevel) { mPaintDC = nullptr; mLastPaintEndTime = TimeStamp::Now(); -#if defined(WIDGET_DEBUG_OUTPUT) - if (debug_WantPaintFlashing()) { - // Only flash paint events which have not ignored the paint message. - // Those that ignore the paint message aren't painting anything so there - // is only the overhead of the dispatching the paint event. - if (result) { - ::InvertRgn(debugPaintFlashDC, debugPaintFlashRegion); - PR_Sleep(PR_MillisecondsToInterval(30)); - ::InvertRgn(debugPaintFlashDC, debugPaintFlashRegion); - PR_Sleep(PR_MillisecondsToInterval(30)); - } - ::ReleaseDC(mWnd, debugPaintFlashDC); - ::DeleteObject(debugPaintFlashRegion); - } -#endif // WIDGET_DEBUG_OUTPUT - // Re-get the listener since painting may have killed it. listener = GetPaintListener(); if (listener) listener->DidPaintWindow();