Skip to content

Commit

Permalink
Post-NEW_THEME modest RenderTheme refactor
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=265753

Reviewed by Aditya Keerthi.

This is a modest improvement (as I discovered several methods were not
virtual which made less modest improvements harder) to the painting and
adjust style side of things.

macOS (and iOS for a single control) uses its own paint() method that
sends instructions to the GPU process. iOS (usually) and Adwaita paint
directly (which is preferable), but Adwaita uses an older code path for
this. Make this more apparent by using the USE(THEME_ADWAITA) directive
and adding some FIXMEs to avoid other themes copying that.

There was a similar situation for adjust style, but there macOS and
Adwaita were aligned, but iOS had its own code path. Again make this
more apparent by giving macOS and Adwaita a shared method the
individual adjust style methods can delegate to.

More work remains here, but this should make it easier for anyone
trying to do something new here.

* Source/WebCore/platform/Theme.cpp:
(WebCore::Theme::paint): Deleted.
(WebCore::Theme::inflateControlPaintRect const): Deleted.
* Source/WebCore/platform/Theme.h:
(WebCore::Theme::paint):
(WebCore::Theme::inflateControlPaintRect const):
* Source/WebCore/platform/adwaita/ThemeAdwaita.cpp:
(WebCore::ThemeAdwaita::paint):
* Source/WebCore/platform/adwaita/ThemeAdwaita.h:
* Source/WebCore/rendering/RenderTheme.cpp:
(WebCore::RenderTheme::adjustStyle):
(WebCore::RenderTheme::paint):
(WebCore::RenderTheme::adjustButtonOrCheckboxOrColorWellOrInnerSpinButtonOrRadioOrSwitchStyle const):
(WebCore::RenderTheme::adjustCheckboxStyle const):
(WebCore::RenderTheme::adjustRadioStyle const):
(WebCore::RenderTheme::adjustColorWellStyle const):
(WebCore::RenderTheme::adjustButtonStyle const):
(WebCore::RenderTheme::adjustInnerSpinButtonStyle const):
(WebCore::RenderTheme::adjustSwitchStyle const):
(WebCore::RenderTheme::paintColorWell): Deleted.
* Source/WebCore/rendering/RenderTheme.h:
(WebCore::RenderTheme::paintColorWell):
(WebCore::RenderTheme::adjustButtonStyle const): Deleted.
(WebCore::RenderTheme::adjustSwitchStyle const): Deleted.
* Source/WebCore/rendering/RenderThemeAdwaita.cpp:
(WebCore::RenderThemeAdwaita::paintMenuList):
* Source/WebCore/rendering/RenderThemeIOS.h:
* Source/WebCore/rendering/RenderThemeIOS.mm:
(WebCore::RenderThemeIOS::paintColorWell): Deleted.

Canonical link: https://commits.webkit.org/271895@main
  • Loading branch information
@annevk
annevk committed Dec 11, 2023
1 parent cc12d00 commit 87efd11
Show file tree
Hide file tree
Showing 9 changed files with 132 additions and 159 deletions.
4 changes: 0 additions & 4 deletions Source/WebCore/platform/Theme.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ LengthSize Theme::minimumControlSize(StyleAppearance, const FontCascade&, const
return { { 0, LengthType::Fixed }, { 0, LengthType::Fixed } };
}

void Theme::paint(StyleAppearance, ControlStates&, GraphicsContext&, const FloatRect&, float, ScrollView*, float, float, bool, bool, const Color&)
{
}

LengthBox Theme::controlBorder(StyleAppearance appearance, const FontCascade&, const LengthBox& zoomedBox, float) const
{
switch (appearance) {
Expand Down
7 changes: 4 additions & 3 deletions Source/WebCore/platform/Theme.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ class Theme {
// Whether or not whitespace: pre should be forced on always.
virtual bool controlRequiresPreWhiteSpace(StyleAppearance) const { return false; }

// Method for painting a control. The rect is in zoomed coordinates.
// FIXME: <https://webkit.org/b/231637> Move parameters to a struct.
virtual void paint(StyleAppearance, ControlStates&, GraphicsContext&, const FloatRect& zoomedRect, float zoomFactor, ScrollView*, float deviceScaleFactor, float pageScaleFactor, bool useSystemAppearance, bool useDarkAppearance, const Color& tintColor);
#if USE(THEME_ADWAITA)
// FIXME: Merge this into RenderThemeAdwaita.
virtual void paint(StyleAppearance, ControlStates&, GraphicsContext&, const FloatRect&, bool, const Color&) { }
#endif

// Some controls may spill out of their containers (e.g., the check on an OS X checkbox). When these controls repaint,
// the theme needs to communicate this inflated rect to the engine so that it can invalidate the whole control.
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/adwaita/ThemeAdwaita.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ LengthBox ThemeAdwaita::controlBorder(StyleAppearance appearance, const FontCasc
return Theme::controlBorder(appearance, font, zoomedBox, zoomFactor);
}

void ThemeAdwaita::paint(StyleAppearance appearance, ControlStates& states, GraphicsContext& context, const FloatRect& zoomedRect, float, ScrollView*, float, float, bool, bool useDarkAppearance, const Color& effectiveAccentColor)
void ThemeAdwaita::paint(StyleAppearance appearance, ControlStates& states, GraphicsContext& context, const FloatRect& zoomedRect, bool useDarkAppearance, const Color& effectiveAccentColor)
{
switch (appearance) {
case StyleAppearance::Checkbox:
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/adwaita/ThemeAdwaita.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class ThemeAdwaita : public Theme {
LengthSize controlSize(StyleAppearance, const FontCascade&, const LengthSize&, float) const final;
LengthSize minimumControlSize(StyleAppearance, const FontCascade&, const LengthSize&, float) const final;
LengthBox controlBorder(StyleAppearance, const FontCascade&, const LengthBox&, float) const final;
void paint(StyleAppearance, ControlStates&, GraphicsContext&, const FloatRect&, float, ScrollView*, float, float, bool, bool, const Color&) final;
void paint(StyleAppearance, ControlStates&, GraphicsContext&, const FloatRect&, bool, const Color&) final;

void paintCheckbox(ControlStates&, GraphicsContext&, const FloatRect&, bool, const Color&);
void paintRadio(ControlStates&, GraphicsContext&, const FloatRect&, bool, const Color&);
Expand Down
250 changes: 115 additions & 135 deletions Source/WebCore/rendering/RenderTheme.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,111 +220,7 @@ void RenderTheme::adjustStyle(RenderStyle& style, const Element* element, const
if (!supportsBoxShadow(style))
style.setBoxShadow(nullptr);

#if !PLATFORM(IOS_FAMILY)
switch (appearance) {
case StyleAppearance::Checkbox:
case StyleAppearance::InnerSpinButton:
case StyleAppearance::Radio:
case StyleAppearance::PushButton:
case StyleAppearance::SquareButton:
case StyleAppearance::Switch:
#if ENABLE(INPUT_TYPE_COLOR)
case StyleAppearance::ColorWell:
#endif
case StyleAppearance::DefaultButton:
case StyleAppearance::Button: {
// Border
LengthBox borderBox(style.borderTopWidth(), style.borderRightWidth(), style.borderBottomWidth(), style.borderLeftWidth());
borderBox = Theme::singleton().controlBorder(appearance, style.fontCascade(), borderBox, style.effectiveZoom());

auto supportsVerticalWritingMode = [](StyleAppearance appearance) {
return appearance == StyleAppearance::Button
#if ENABLE(INPUT_TYPE_COLOR)
|| appearance == StyleAppearance::ColorWell
#endif
|| appearance == StyleAppearance::DefaultButton
|| appearance == StyleAppearance::SquareButton
|| appearance == StyleAppearance::PushButton;
};
// Transpose for vertical writing mode:
if (!style.isHorizontalWritingMode() && supportsVerticalWritingMode(appearance))
borderBox = LengthBox(borderBox.left().value(), borderBox.top().value(), borderBox.right().value(), borderBox.bottom().value());

if (borderBox.top().value() != static_cast<int>(style.borderTopWidth())) {
if (borderBox.top().value())
style.setBorderTopWidth(borderBox.top().value());
else
style.resetBorderTop();
}
if (borderBox.right().value() != static_cast<int>(style.borderRightWidth())) {
if (borderBox.right().value())
style.setBorderRightWidth(borderBox.right().value());
else
style.resetBorderRight();
}
if (borderBox.bottom().value() != static_cast<int>(style.borderBottomWidth())) {
style.setBorderBottomWidth(borderBox.bottom().value());
if (borderBox.bottom().value())
style.setBorderBottomWidth(borderBox.bottom().value());
else
style.resetBorderBottom();
}
if (borderBox.left().value() != static_cast<int>(style.borderLeftWidth())) {
style.setBorderLeftWidth(borderBox.left().value());
if (borderBox.left().value())
style.setBorderLeftWidth(borderBox.left().value());
else
style.resetBorderLeft();
}

// Padding
LengthBox paddingBox = Theme::singleton().controlPadding(appearance, style.fontCascade(), style.paddingBox(), style.effectiveZoom());
if (paddingBox != style.paddingBox())
style.setPaddingBox(WTFMove(paddingBox));

// Whitespace
if (Theme::singleton().controlRequiresPreWhiteSpace(appearance)) {
style.setWhiteSpaceCollapse(WhiteSpaceCollapse::Preserve);
style.setTextWrapMode(TextWrapMode::NoWrap);
}

// Width / Height
// The width and height here are affected by the zoom.
// FIXME: Check is flawed, since it doesn't take min-width/max-width into account.
LengthSize controlSize = Theme::singleton().controlSize(appearance, style.fontCascade(), { style.width(), style.height() }, style.effectiveZoom());
if (controlSize.width != style.width())
style.setWidth(WTFMove(controlSize.width));
if (controlSize.height != style.height())
style.setHeight(WTFMove(controlSize.height));

// Min-Width / Min-Height
LengthSize minControlSize = Theme::singleton().minimumControlSize(appearance, style.fontCascade(), { style.minWidth(), style.minHeight() }, { style.width(), style.height() }, style.effectiveZoom());
if (minControlSize.width.value() > style.minWidth().value())
style.setMinWidth(WTFMove(minControlSize.width));
if (minControlSize.height.value() > style.minHeight().value())
style.setMinHeight(WTFMove(minControlSize.height));

// Font
if (auto themeFont = Theme::singleton().controlFont(appearance, style.fontCascade(), style.effectiveZoom())) {
// If overriding the specified font with the theme font, also override the line height with the standard line height.
style.setLineHeight(RenderStyle::initialLineHeight());
if (style.setFontDescription(WTFMove(themeFont.value())))
style.fontCascade().update(nullptr);
}

// Special style that tells enabled default buttons in active windows to use the ActiveButtonText color.
// The active window part of the test has to be done at paint time since it's not triggered by a style change.
style.setInsideDefaultButton(appearance == StyleAppearance::DefaultButton && element && !element->isDisabledFormControl());
break;
}
default:
break;
}
#endif

// Call the appropriate style adjustment method based off the appearance value.
switch (appearance) {
#if PLATFORM(IOS_FAMILY)
case StyleAppearance::Checkbox:
return adjustCheckboxStyle(style, element);
case StyleAppearance::Radio:
Expand All @@ -338,7 +234,8 @@ void RenderTheme::adjustStyle(RenderStyle& style, const Element* element, const
case StyleAppearance::DefaultButton:
case StyleAppearance::Button:
return adjustButtonStyle(style, element);
#endif
case StyleAppearance::InnerSpinButton:
return adjustInnerSpinButtonStyle(style, element);
case StyleAppearance::TextField:
return adjustTextFieldStyle(style, element);
case StyleAppearance::TextArea:
Expand Down Expand Up @@ -879,10 +776,13 @@ bool RenderTheme::paint(const RenderBox& box, ControlStates& controlStates, cons
float deviceScaleFactor = box.document().deviceScaleFactor();
FloatRect devicePixelSnappedRect = snapRectToDevicePixels(rect, deviceScaleFactor);

#if !PLATFORM(IOS_FAMILY)
float pageScaleFactor = box.page().pageScaleFactor();

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

switch (appearance) {
#if USE(THEME_ADWAITA)
case StyleAppearance::Checkbox:
case StyleAppearance::Radio:
case StyleAppearance::PushButton:
Expand All @@ -894,18 +794,9 @@ bool RenderTheme::paint(const RenderBox& box, ControlStates& controlStates, cons
case StyleAppearance::Button:
case StyleAppearance::InnerSpinButton:
updateControlStatesForRenderer(box, controlStates);
Theme::singleton().paint(appearance, controlStates, paintInfo.context(), devicePixelSnappedRect, box.style().effectiveZoom(), &box.view().frameView(), deviceScaleFactor, pageScaleFactor, box.document().useSystemAppearance(), box.useDarkAppearance(), box.style().effectiveAccentColor());
Theme::singleton().paint(appearance, controlStates, paintInfo.context(), devicePixelSnappedRect, box.useDarkAppearance(), box.style().effectiveAccentColor());
return false;
default:
break;
}
#else
UNUSED_PARAM(controlStates);
#endif

// Call the appropriate paint method based off the appearance value.
switch (appearance) {
#if PLATFORM(IOS_FAMILY)
#else // !USE(THEME_ADWAITA)
case StyleAppearance::Checkbox:
return paintCheckbox(box, paintInfo, devicePixelSnappedRect);
case StyleAppearance::Radio:
Expand All @@ -919,7 +810,7 @@ bool RenderTheme::paint(const RenderBox& box, ControlStates& controlStates, cons
case StyleAppearance::DefaultButton:
case StyleAppearance::Button:
return paintButton(box, paintInfo, integralSnappedRect);
#endif
#endif // !USE(THEME_ADWAITA)
case StyleAppearance::Menulist:
return paintMenuList(box, paintInfo, devicePixelSnappedRect);
case StyleAppearance::Meter:
Expand Down Expand Up @@ -1401,37 +1292,121 @@ bool RenderTheme::hasListButtonPressed(const RenderObject& renderer) const
}
#endif

#if PLATFORM(IOS_FAMILY)
// FIXME: iOS does not use this so arguably this should be better abstracted. Or maybe we should
// investigate if we can bring the various ports closer together.
void RenderTheme::adjustButtonOrCheckboxOrColorWellOrInnerSpinButtonOrRadioOrSwitchStyle(RenderStyle& style, const Element* element) const
{
auto appearance = style.effectiveAppearance();

void RenderTheme::adjustCheckboxStyle(RenderStyle& style, const Element*) const
LengthBox borderBox(style.borderTopWidth(), style.borderRightWidth(), style.borderBottomWidth(), style.borderLeftWidth());
borderBox = Theme::singleton().controlBorder(appearance, style.fontCascade(), borderBox, style.effectiveZoom());

auto supportsVerticalWritingMode = [](StyleAppearance appearance) {
return appearance == StyleAppearance::Button
#if ENABLE(INPUT_TYPE_COLOR)
|| appearance == StyleAppearance::ColorWell
#endif
|| appearance == StyleAppearance::DefaultButton
|| appearance == StyleAppearance::SquareButton
|| appearance == StyleAppearance::PushButton;
};
// Transpose for vertical writing mode:
if (!style.isHorizontalWritingMode() && supportsVerticalWritingMode(appearance))
borderBox = LengthBox(borderBox.left().value(), borderBox.top().value(), borderBox.right().value(), borderBox.bottom().value());

if (borderBox.top().value() != static_cast<int>(style.borderTopWidth())) {
if (borderBox.top().value())
style.setBorderTopWidth(borderBox.top().value());
else
style.resetBorderTop();
}
if (borderBox.right().value() != static_cast<int>(style.borderRightWidth())) {
if (borderBox.right().value())
style.setBorderRightWidth(borderBox.right().value());
else
style.resetBorderRight();
}
if (borderBox.bottom().value() != static_cast<int>(style.borderBottomWidth())) {
style.setBorderBottomWidth(borderBox.bottom().value());
if (borderBox.bottom().value())
style.setBorderBottomWidth(borderBox.bottom().value());
else
style.resetBorderBottom();
}
if (borderBox.left().value() != static_cast<int>(style.borderLeftWidth())) {
style.setBorderLeftWidth(borderBox.left().value());
if (borderBox.left().value())
style.setBorderLeftWidth(borderBox.left().value());
else
style.resetBorderLeft();
}

// Padding
LengthBox paddingBox = Theme::singleton().controlPadding(appearance, style.fontCascade(), style.paddingBox(), style.effectiveZoom());
if (paddingBox != style.paddingBox())
style.setPaddingBox(WTFMove(paddingBox));

// Whitespace
if (Theme::singleton().controlRequiresPreWhiteSpace(appearance)) {
style.setWhiteSpaceCollapse(WhiteSpaceCollapse::Preserve);
style.setTextWrapMode(TextWrapMode::NoWrap);
}

// Width / Height
// The width and height here are affected by the zoom.
// FIXME: Check is flawed, since it doesn't take min-width/max-width into account.
LengthSize controlSize = Theme::singleton().controlSize(appearance, style.fontCascade(), { style.width(), style.height() }, style.effectiveZoom());
if (controlSize.width != style.width())
style.setWidth(WTFMove(controlSize.width));
if (controlSize.height != style.height())
style.setHeight(WTFMove(controlSize.height));

// Min-Width / Min-Height
LengthSize minControlSize = Theme::singleton().minimumControlSize(appearance, style.fontCascade(), { style.minWidth(), style.minHeight() }, { style.width(), style.height() }, style.effectiveZoom());
if (minControlSize.width.value() > style.minWidth().value())
style.setMinWidth(WTFMove(minControlSize.width));
if (minControlSize.height.value() > style.minHeight().value())
style.setMinHeight(WTFMove(minControlSize.height));

// Font
if (auto themeFont = Theme::singleton().controlFont(appearance, style.fontCascade(), style.effectiveZoom())) {
// If overriding the specified font with the theme font, also override the line height with the standard line height.
style.setLineHeight(RenderStyle::initialLineHeight());
if (style.setFontDescription(WTFMove(themeFont.value())))
style.fontCascade().update(nullptr);
}

// Special style that tells enabled default buttons in active windows to use the ActiveButtonText color.
// The active window part of the test has to be done at paint time since it's not triggered by a style change.
style.setInsideDefaultButton(appearance == StyleAppearance::DefaultButton && element && !element->isDisabledFormControl());
}

void RenderTheme::adjustCheckboxStyle(RenderStyle& style, const Element* element) const
{
style.resetPadding();
style.resetBorder();
style.setBoxShadow(nullptr);
adjustButtonOrCheckboxOrColorWellOrInnerSpinButtonOrRadioOrSwitchStyle(style, element);
}

void RenderTheme::adjustRadioStyle(RenderStyle& style, const Element*) const
void RenderTheme::adjustRadioStyle(RenderStyle& style, const Element* element) const
{
style.resetPadding();
style.resetBorder();
style.setBoxShadow(nullptr);
adjustButtonOrCheckboxOrColorWellOrInnerSpinButtonOrRadioOrSwitchStyle(style, element);
}

#if ENABLE(INPUT_TYPE_COLOR)

void RenderTheme::adjustColorWellStyle(RenderStyle& style, const Element* element) const
{
adjustButtonStyle(style, element);
adjustButtonOrCheckboxOrColorWellOrInnerSpinButtonOrRadioOrSwitchStyle(style, element);
}
#endif

bool RenderTheme::paintColorWell(const RenderObject& box, const PaintInfo& paintInfo, const IntRect& rect)
void RenderTheme::adjustButtonStyle(RenderStyle& style, const Element* element) const
{
return paintButton(box, paintInfo, rect);
adjustButtonOrCheckboxOrColorWellOrInnerSpinButtonOrRadioOrSwitchStyle(style, element);
}

#endif // ENABLE(INPUT_TYPE_COLOR)

#endif // PLATFORM(IOS_FAMILY)
void RenderTheme::adjustInnerSpinButtonStyle(RenderStyle& style, const Element* element) const
{
adjustButtonOrCheckboxOrColorWellOrInnerSpinButtonOrRadioOrSwitchStyle(style, element);
}

void RenderTheme::adjustMenuListStyle(RenderStyle& style, const Element*) const
{
Expand Down Expand Up @@ -1561,6 +1536,11 @@ void RenderTheme::adjustSliderThumbStyle(RenderStyle& style, const Element* elem
adjustSliderThumbSize(style, element);
}

void RenderTheme::adjustSwitchStyle(RenderStyle& style, const Element* element) const
{
adjustButtonOrCheckboxOrColorWellOrInnerSpinButtonOrRadioOrSwitchStyle(style, element);
}

void RenderTheme::purgeCaches()
{
m_colorCacheMap.clear();
Expand Down
Loading

0 comments on commit 87efd11

Please sign in to comment.