Skip to content

Commit

Permalink
Remove lazy repaints from RenderView and more
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=266272

Reviewed by Aditya Keerthi.

I looked into whether the controlStates argument of
RenderTheme::paint() could be removed as it's only used by a single
theme.

I then discovered that ControlStates's needsRepaint() always returned
false. I then checked other state in ControlStates and that was
similarly not manipulated anywhere and thus could be removed.

Since needsRepaint() was false, scheduleLazyRepaint() was never called.
This allowed for removal of lazy repaints and their Timer
infrastructure.

I suspect that we want to get rid of ControlStates in its entirety in
favor of ControlStyle, but baby steps.

* Source/WebCore/platform/ControlStates.h:
(WebCore::ControlStates::setStates):
(WebCore::ControlStates::needsRepaint const): Deleted.
(WebCore::ControlStates::setNeedsRepaint): Deleted.
(WebCore::ControlStates::isDirty const): Deleted.
(WebCore::ControlStates::setDirty): Deleted.
(WebCore::ControlStates::timeSinceControlWasFocused const): Deleted.
(WebCore::ControlStates::setTimeSinceControlWasFocused): Deleted.
(WebCore::ControlStates::platformControl const): Deleted.
(WebCore::ControlStates::setPlatformControl): Deleted.
(): Deleted.
* Source/WebCore/rendering/RenderAttachment.cpp:
(WebCore::RenderAttachment::paintReplaced):
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::willBeDestroyed):
(WebCore::RenderBox::paintBoxDecorations):
(WebCore::controlStatesRendererMap): Deleted.
(WebCore::controlStatesForRenderer): Deleted.
(WebCore::removeControlStatesForRenderer): Deleted.
* Source/WebCore/rendering/RenderElement.cpp:
(WebCore::RenderElement::RenderElement):
* Source/WebCore/rendering/RenderElement.h:
(WebCore::RenderElement::setRenderBoxNeedsLazyRepaint): Deleted.
(WebCore::RenderElement::renderBoxNeedsLazyRepaint const): Deleted.
* Source/WebCore/rendering/RenderTheme.cpp:
(WebCore::RenderTheme::paint):
(WebCore::RenderTheme::updateControlStatesForRenderer const): Deleted.
* Source/WebCore/rendering/RenderTheme.h:
* Source/WebCore/rendering/RenderView.cpp:
(WebCore::RenderView::RenderView):
(WebCore::RenderView::scheduleLazyRepaint): Deleted.
(WebCore::RenderView::unscheduleLazyRepaint): Deleted.
(WebCore::RenderView::lazyRepaintTimerFired): Deleted.
* Source/WebCore/rendering/RenderView.h:

Canonical link: https://commits.webkit.org/271938@main
  • Loading branch information
annevk committed Dec 12, 2023
1 parent 952dc0f commit da12017
Show file tree
Hide file tree
Showing 9 changed files with 10 additions and 129 deletions.
37 changes: 1 addition & 36 deletions Source/WebCore/platform/ControlStates.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,9 @@
#define ControlStates_h

#include <wtf/OptionSet.h>
#include <wtf/RetainPtr.h>
#include <wtf/Seconds.h>

#if PLATFORM(COCOA)
#ifndef __OBJC__
typedef struct objc_object *id;
#endif
#endif

namespace WebCore {

#if PLATFORM(COCOA)
typedef id PlatformControlInstance;
#endif

class ControlStates {
WTF_MAKE_FAST_ALLOCATED;
public:
Expand Down Expand Up @@ -71,35 +59,12 @@ class ControlStates {
if (newStates == m_states)
return;
m_states = newStates;
m_isDirty = m_initialized;
m_initialized = true;
}

bool needsRepaint() const { return m_needsRepaint; }
void setNeedsRepaint(bool r) { m_needsRepaint = r; }

bool isDirty() const { return m_isDirty; }
void setDirty(bool d) { m_isDirty = d; }

Seconds timeSinceControlWasFocused() const { return m_timeSinceControlWasFocused; }
void setTimeSinceControlWasFocused(Seconds time) { m_timeSinceControlWasFocused = time; }

#if PLATFORM(COCOA)
PlatformControlInstance platformControl() const { return m_controlInstance.get(); }
void setPlatformControl(PlatformControlInstance instance) { m_controlInstance = instance; }
#endif

private:
OptionSet<States> m_states;
bool m_initialized { false };
bool m_needsRepaint { false };
bool m_isDirty { false };
Seconds m_timeSinceControlWasFocused { 0_s };
#if PLATFORM(COCOA)
RetainPtr<PlatformControlInstance> m_controlInstance;
#endif
};

}
} // namespace WebCore

#endif
3 changes: 1 addition & 2 deletions Source/WebCore/rendering/RenderAttachment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,7 @@ void RenderAttachment::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& of
auto paintRect = borderBoxRect();
paintRect.moveBy(offset);

ControlStates controlStates;
theme().paint(*this, controlStates, paintInfo, paintRect);
theme().paint(*this, paintInfo, paintRect);
}

void RenderAttachment::layoutShadowContent(const LayoutSize& size)
Expand Down
31 changes: 2 additions & 29 deletions Source/WebCore/rendering/RenderBox.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "BackgroundPainter.h"
#include "BorderPainter.h"
#include "CSSFontSelector.h"
#include "ControlStates.h"
#include "Document.h"
#include "Editing.h"
#include "EventHandler.h"
Expand Down Expand Up @@ -131,25 +130,6 @@ static OverrideOptionalSizeMap* gOverridingContainingBlockContentLogicalWidthMap
static const int autoscrollBeltSize = 20;
static const unsigned backgroundObscurationTestMaxDepth = 4;

using ControlStatesRendererMap = HashMap<const RenderObject*, std::unique_ptr<ControlStates>>;
static ControlStatesRendererMap& controlStatesRendererMap()
{
static NeverDestroyed<ControlStatesRendererMap> map;
return map;
}

static ControlStates* controlStatesForRenderer(const RenderBox& renderer)
{
return controlStatesRendererMap().ensure(&renderer, [] {
return makeUnique<ControlStates>();
}).iterator->value.get();
}

static void removeControlStatesForRenderer(const RenderBox& renderer)
{
controlStatesRendererMap().remove(&renderer);
}

bool RenderBox::s_hadNonVisibleOverflow = false;

RenderBox::RenderBox(Type type, Element& element, RenderStyle&& style, BaseTypeFlags baseTypeFlags)
Expand Down Expand Up @@ -183,9 +163,6 @@ void RenderBox::willBeDestroyed()

ShapeOutsideInfo::removeInfo(*this);

view().unscheduleLazyRepaint(*this);
removeControlStatesForRenderer(*this);

if (hasInitializedStyle()) {
if (style().hasSnapPosition())
view().unregisterBoxWithScrollSnapPositions(*this);
Expand Down Expand Up @@ -1730,12 +1707,8 @@ void RenderBox::paintBoxDecorations(PaintInfo& paintInfo, const LayoutPoint& pai
if (style().hasEffectiveAppearance()) {
if (auto* control = ensureControlPartForRenderer())
borderOrBackgroundPaintingIsNeeded = theme().paint(*this, *control, paintInfo, paintRect);
else {
ControlStates* controlStates = controlStatesForRenderer(*this);
borderOrBackgroundPaintingIsNeeded = theme().paint(*this, *controlStates, paintInfo, paintRect);
if (controlStates->needsRepaint())
view().scheduleLazyRepaint(*this);
}
else
borderOrBackgroundPaintingIsNeeded = theme().paint(*this, paintInfo, paintRect);
}

BorderPainter borderPainter { *this, paintInfo };
Expand Down
1 change: 0 additions & 1 deletion Source/WebCore/rendering/RenderElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ inline RenderElement::RenderElement(Type type, ContainerNode& elementOrDocument,
, m_baseTypeFlags(baseTypeFlags)
, m_ancestorLineBoxDirty(false)
, m_hasInitializedStyle(false)
, m_renderBoxNeedsLazyRepaint(false)
, m_hasPausedImageAnimations(false)
, m_hasCounterNodeMap(false)
, m_hasContinuationChainNode(false)
Expand Down
4 changes: 0 additions & 4 deletions Source/WebCore/rendering/RenderElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,6 @@ class RenderElement : public RenderObject {
bool hasPausedImageAnimations() const { return m_hasPausedImageAnimations; }
void setHasPausedImageAnimations(bool b) { m_hasPausedImageAnimations = b; }

void setRenderBoxNeedsLazyRepaint(bool b) { m_renderBoxNeedsLazyRepaint = b; }
bool renderBoxNeedsLazyRepaint() const { return m_renderBoxNeedsLazyRepaint; }

bool hasCounterNodeMap() const { return m_hasCounterNodeMap; }
void setHasCounterNodeMap(bool f) { m_hasCounterNodeMap = f; }

Expand Down Expand Up @@ -412,7 +409,6 @@ class RenderElement : public RenderObject {
unsigned m_ancestorLineBoxDirty : 1;
unsigned m_hasInitializedStyle : 1;

unsigned m_renderBoxNeedsLazyRepaint : 1;
unsigned m_hasPausedImageAnimations : 1;
unsigned m_hasCounterNodeMap : 1;
unsigned m_hasContinuationChainNode : 1;
Expand Down
22 changes: 5 additions & 17 deletions Source/WebCore/rendering/RenderTheme.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,7 @@ bool RenderTheme::paint(const RenderBox& box, ControlPart& part, const PaintInfo
return false;
}

bool RenderTheme::paint(const RenderBox& box, ControlStates& controlStates, const PaintInfo& paintInfo, const LayoutRect& rect)
bool RenderTheme::paint(const RenderBox& box, const PaintInfo& paintInfo, const LayoutRect& rect)
{
// If painting is disabled, but we aren't updating control tints, then just bail.
// If we are updating control tints, just schedule a repaint if the theme supports tinting
Expand All @@ -776,11 +776,6 @@ bool RenderTheme::paint(const RenderBox& box, ControlStates& controlStates, cons
float deviceScaleFactor = box.document().deviceScaleFactor();
FloatRect devicePixelSnappedRect = snapRectToDevicePixels(rect, deviceScaleFactor);


#if !USE(THEME_ADWAITA)
UNUSED_PARAM(controlStates);
#endif

switch (appearance) {
#if USE(THEME_ADWAITA)
case StyleAppearance::Checkbox:
Expand All @@ -792,10 +787,11 @@ bool RenderTheme::paint(const RenderBox& box, ControlStates& controlStates, cons
#endif
case StyleAppearance::DefaultButton:
case StyleAppearance::Button:
case StyleAppearance::InnerSpinButton:
updateControlStatesForRenderer(box, controlStates);
Theme::singleton().paint(appearance, controlStates, paintInfo.context(), devicePixelSnappedRect, box.useDarkAppearance(), box.style().effectiveAccentColor());
case StyleAppearance::InnerSpinButton: {
ControlStates states(extractControlStatesForRenderer(box));
Theme::singleton().paint(appearance, states, paintInfo.context(), devicePixelSnappedRect, box.useDarkAppearance(), box.style().effectiveAccentColor());
return false;
}
#else // !USE(THEME_ADWAITA)
case StyleAppearance::Checkbox:
return paintCheckbox(box, paintInfo, devicePixelSnappedRect);
Expand Down Expand Up @@ -1148,14 +1144,6 @@ bool RenderTheme::stateChanged(const RenderObject& o, ControlStates::States stat
return true;
}

void RenderTheme::updateControlStatesForRenderer(const RenderBox& box, ControlStates& controlStates) const
{
ControlStates newStates = extractControlStatesForRenderer(box);
controlStates.setStates(newStates.states());
if (isFocused(box))
controlStates.setTimeSinceControlWasFocused(box.page().focusController().timeSinceFocusWasSet());
}

OptionSet<ControlStates::States> RenderTheme::extractControlStatesForRenderer(const RenderObject& o) const
{
OptionSet<ControlStates::States> states;
Expand Down
3 changes: 1 addition & 2 deletions Source/WebCore/rendering/RenderTheme.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class RenderTheme {
// text of a button, is always rendered by the engine itself. The boolean return value indicates
// whether the CSS border/background should also be painted.
bool paint(const RenderBox&, ControlPart&, const PaintInfo&, const LayoutRect&);
bool paint(const RenderBox&, ControlStates&, const PaintInfo&, const LayoutRect&);
bool paint(const RenderBox&, const PaintInfo&, const LayoutRect&);

bool paintBorderOnly(const RenderBox&, const PaintInfo&, const LayoutRect&);
void paintDecorations(const RenderBox&, const PaintInfo&, const LayoutRect&);
Expand Down Expand Up @@ -389,7 +389,6 @@ class RenderTheme {
void adjustButtonOrCheckboxOrColorWellOrInnerSpinButtonOrRadioOrSwitchStyle(RenderStyle&, const Element*) const;

public:
void updateControlStatesForRenderer(const RenderBox&, ControlStates&) const;
OptionSet<ControlStates::States> extractControlStatesForRenderer(const RenderObject&) const;
bool isWindowActive(const RenderObject&) const;
bool isChecked(const RenderObject&) const;
Expand Down
30 changes: 0 additions & 30 deletions Source/WebCore/rendering/RenderView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ RenderView::RenderView(Document& document, RenderStyle&& style)
, m_initialContainingBlock(makeUniqueRef<Layout::InitialContainingBlock>(RenderStyle::clone(this->style())))
, m_layoutState(makeUniqueRef<Layout::LayoutState>(document, *m_initialContainingBlock))
, m_selection(*this)
, m_lazyRepaintTimer(*this, &RenderView::lazyRepaintTimerFired)
{
// FIXME: We should find a way to enforce this at compile time.
ASSERT(document.view());
Expand Down Expand Up @@ -123,35 +122,6 @@ void RenderView::styleDidChange(StyleDifference diff, const RenderStyle* oldStyl
frameView().topContentDirectionDidChange();
}

void RenderView::scheduleLazyRepaint(RenderBox& renderer)
{
if (renderer.renderBoxNeedsLazyRepaint())
return;
renderer.setRenderBoxNeedsLazyRepaint(true);
m_renderersNeedingLazyRepaint.add(renderer);
if (!m_lazyRepaintTimer.isActive())
m_lazyRepaintTimer.startOneShot(0_s);
}

void RenderView::unscheduleLazyRepaint(RenderBox& renderer)
{
if (!renderer.renderBoxNeedsLazyRepaint())
return;
renderer.setRenderBoxNeedsLazyRepaint(false);
m_renderersNeedingLazyRepaint.remove(renderer);
if (m_renderersNeedingLazyRepaint.isEmptyIgnoringNullReferences())
m_lazyRepaintTimer.stop();
}

void RenderView::lazyRepaintTimerFired()
{
for (auto& renderer : m_renderersNeedingLazyRepaint) {
renderer.repaint();
renderer.setRenderBoxNeedsLazyRepaint(false);
}
m_renderersNeedingLazyRepaint.clear();
}

RenderBox::LogicalExtentComputedValues RenderView::computeLogicalHeight(LayoutUnit logicalHeight, LayoutUnit) const
{
return { !shouldUsePrintingLayout() ? LayoutUnit(viewLogicalHeight()) : logicalHeight, 0_lu, ComputedMarginValues() };
Expand Down
8 changes: 0 additions & 8 deletions Source/WebCore/rendering/RenderView.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,6 @@ class RenderView final : public RenderBlockFlow {
bool m_wasAccumulatingRepaintRegion { false };
};

void scheduleLazyRepaint(RenderBox&);
void unscheduleLazyRepaint(RenderBox&);

void layerChildrenChangedDuringStyleChange(RenderLayer&);
RenderLayer* takeStyleChangeLayerTreeMutationRoot();

Expand Down Expand Up @@ -261,11 +258,6 @@ class RenderView final : public RenderBlockFlow {

bool shouldUsePrintingLayout() const;

void lazyRepaintTimerFired();

Timer m_lazyRepaintTimer;
SingleThreadWeakHashSet<RenderBox> m_renderersNeedingLazyRepaint;

std::unique_ptr<ImageQualityController> m_imageQualityController;
std::optional<LayoutSize> m_pageLogicalSize;
bool m_pageLogicalHeightChanged { false };
Expand Down

0 comments on commit da12017

Please sign in to comment.