Skip to content

Commit

Permalink
REGRESSION(270320@main): Speedometer3/Editor-TipTap 4-6%
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=264744
rdar://118312367

Unreviewed, partial revert of 270320@main to address the Speedometer
regression. In 270320@main, I had converted all remaining
CheckedPtr<Font> to WeakPtr<Font> to address crashes. However, we
didn't need to convert GlyphData's font data member. Reverting this
part of the change doesn't regress the layout test introduced in
270320@main but I have confirmed that it resolves the Speedometer
regression.

* Source/WebCore/platform/graphics/Font.h:
* Source/WebCore/platform/graphics/FontCascade.cpp:
(WebCore::FontCascade::emphasisMarkAscent const):
(WebCore::FontCascade::emphasisMarkDescent const):
(WebCore::FontCascade::drawEmphasisMarks const):
* Source/WebCore/platform/graphics/FontCascadeFonts.cpp:
(WebCore::MixedFontGlyphPage::setGlyphDataForIndex):
* Source/WebCore/platform/graphics/GlyphPage.h:

Canonical link: https://commits.webkit.org/270646@main
  • Loading branch information
cdumez committed Nov 13, 2023
1 parent 91a7856 commit 7c1250d
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 8 deletions.
3 changes: 2 additions & 1 deletion Source/WebCore/platform/graphics/Font.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "RenderingResourceIdentifier.h"
#include <variant>
#include <wtf/BitVector.h>
#include <wtf/CheckedRef.h>
#include <wtf/Hasher.h>
#include <wtf/WeakPtr.h>
#include <wtf/text/StringHash.h>
Expand Down Expand Up @@ -74,7 +75,7 @@ bool fontHasEitherTable(CTFontRef, unsigned tableTag1, unsigned tableTag2);
#endif

DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER(Font);
class Font : public RefCounted<Font>, public CanMakeWeakPtr<Font> {
class Font : public RefCounted<Font>, public CanMakeWeakPtr<Font>, public CanMakeCheckedPtr {
WTF_MAKE_FAST_ALLOCATED_WITH_HEAP_IDENTIFIER(Font);
public:
// Used to create platform fonts.
Expand Down
6 changes: 3 additions & 3 deletions Source/WebCore/platform/graphics/FontCascade.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1224,7 +1224,7 @@ int FontCascade::emphasisMarkAscent(const AtomString& mark) const
if (!markGlyphData)
return 0;

WeakPtr markFontData = markGlyphData.value().font;
CheckedPtr markFontData = markGlyphData.value().font;
ASSERT(markFontData);
if (!markFontData)
return 0;
Expand All @@ -1238,7 +1238,7 @@ int FontCascade::emphasisMarkDescent(const AtomString& mark) const
if (!markGlyphData)
return 0;

WeakPtr markFontData = markGlyphData.value().font;
CheckedPtr markFontData = markGlyphData.value().font;
ASSERT(markFontData);
if (!markFontData)
return 0;
Expand Down Expand Up @@ -1402,7 +1402,7 @@ void FontCascade::drawEmphasisMarks(GraphicsContext& context, const GlyphBuffer&
if (!markGlyphData)
return;

WeakPtr markFontData = markGlyphData.value().font;
CheckedPtr markFontData = markGlyphData.value().font;
ASSERT(markFontData);
if (!markFontData)
return;
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/FontCascadeFonts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class MixedFontGlyphPage {
{
ASSERT_WITH_SECURITY_IMPLICATION(index < GlyphPage::size);
m_glyphs[index] = glyphData.glyph;
m_fonts[index] = glyphData.font;
m_fonts[index] = glyphData.font.get();
}

Glyph m_glyphs[GlyphPage::size] { };
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/GlyphPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ struct GlyphData {

Glyph glyph;
ColorGlyphType colorGlyphType;
WeakPtr<const Font> font;
CheckedPtr<const Font> font;
};

// A GlyphPage contains a fixed-size set of GlyphData mappings for a contiguous
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,14 +123,14 @@ const Font* FontCascade::fontForCombiningCharacterSequence(StringView stringView
else if (characters[length - 1] == 0xFE0F)
preferColoredFont = true;

WeakPtr baseFont = glyphDataForCharacter(character, false, NormalVariant).font;
CheckedPtr baseFont = glyphDataForCharacter(character, false, NormalVariant).font;
if (baseFont
&& (clusterLength == length || baseFont->canRenderCombiningCharacterSequence(normalizedString.view))
&& (!preferColoredFont || baseFont->platformData().isColorBitmapFont()))
return baseFont.get();

for (unsigned i = 0; !fallbackRangesAt(i).isNull(); ++i) {
WeakPtr fallbackFont = fallbackRangesAt(i).fontForCharacter(character);
CheckedPtr fallbackFont = fallbackRangesAt(i).fontForCharacter(character);
if (!fallbackFont || fallbackFont == baseFont)
continue;

Expand Down

0 comments on commit 7c1250d

Please sign in to comment.