Bug 1743310 - Remove paint flashing. r=gfx-reviewers,lsalzman

It's only meaningfully read in two places:

 * WebRender fallback code.
 * Some widget DEBUG-only code, which on Linux is ifdef'd and on Windows does
   some rather sketchy things like sleeping for 30ms

So I think it should be ok to remove, since WR fallback has its own flashing
pref as well, IIUC.

Differential Revision: https://phabricator.services.mozilla.com/D132313
This commit is contained in:
Emilio Cobos Álvarez 2021-11-30 11:46:08 +00:00
Родитель 52174f1ecb
Коммит c9d4405306
11 изменённых файлов: 15 добавлений и 246 удалений

Просмотреть файл

@ -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 dont add more unintentional reflow
### Writing tests to ensure you dont 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 its 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 whats 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 youre 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 youre touching some DOM nodes in such a way that unexpected
repaints are occurring in an area that dont 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 youre 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 *\<xul:menupopup\>s*, you often have

Просмотреть файл

@ -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) {

Просмотреть файл

@ -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,

Просмотреть файл

@ -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()));
}
}

Просмотреть файл

@ -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);
}

Просмотреть файл

@ -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;

Просмотреть файл

@ -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.");

Просмотреть файл

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

Просмотреть файл

@ -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) {

Просмотреть файл

@ -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,

Просмотреть файл

@ -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();