- Revision
- 265261
- Author
- mmaxfi...@apple.com
- Date
- 2020-08-04 15:02:26 -0700 (Tue, 04 Aug 2020)
Log Message
Add glyph origins member to GlyphBuffer
https://bugs.webkit.org/show_bug.cgi?id=215057
Reviewed by Darin Adler.
This is in preparation for https://bugs.webkit.org/show_bug.cgi?id=214769
and https://bugs.webkit.org/show_bug.cgi?id=206208.
The solution for https://bugs.webkit.org/show_bug.cgi?id=214769 requires applying
letter-spacing after text shaping. Today, we apply letter-spacing before text
shaping, which is wrong. However, if we want to apply letter-spacing after text
shaping, we need to use CTFontShapeGlyphs(), which outputs glyph origins in
addition to glyph advances. Adding a glyph origins field to GlyphBuffer allows us
to flatten these origins at the appropriate places.
This patch is meant to be applied on top of
https://bugs.webkit.org/show_bug.cgi?id=215051, which decreases the inline sizes
of these vector members from 2048 to 1024. I measured the median and mean of these
strings in our Page Load Test to be 6 and 7.4, which indicates we were being wildly
inefficient with the members of GlyphBuffer. Decreasing these inline sizes means
we're now using less memory than we were before.
No new tests becasue there is no behavior change yet. This patch just adds the
field. The next patch will hook up CTFontShapeGlyphs() itself.
* platform/graphics/FontCascade.cpp:
(WebCore::FontCascade::drawText const):
(WebCore::FontCascade::drawEmphasisMarks const):
(WebCore::FontCascade::displayListForTextRun const):
(WebCore::FontCascade::drawGlyphBuffer const):
* platform/graphics/GlyphBuffer.h:
(WebCore::GlyphBufferAdvance::GlyphBufferAdvance):
(WebCore::GlyphBufferOrigin::GlyphBufferOrigin):
(WebCore::GlyphBufferOrigin::operator FloatPoint):
(WebCore::GlyphBufferOrigin::setX):
(WebCore::GlyphBufferOrigin::setY):
(WebCore::GlyphBufferOrigin::x const):
(WebCore::GlyphBufferOrigin::y const):
(WebCore::GlyphBufferOrigin::encode const):
(WebCore::GlyphBufferOrigin::decode):
(WebCore::GlyphBuffer::clear):
(WebCore::GlyphBuffer::origins):
(WebCore::GlyphBuffer::origins const):
(WebCore::GlyphBuffer::add):
(WebCore::GlyphBuffer::remove):
(WebCore::GlyphBuffer::makeHole):
(WebCore::GlyphBuffer::shrink):
(WebCore::GlyphBuffer::flatten):
(WebCore::GlyphBuffer::isFlattened const):
(WebCore::GlyphBuffer::swap):
* platform/graphics/GraphicsContext.cpp:
(WebCore::GraphicsContext::drawGlyphs):
* platform/graphics/displaylists/DisplayListItems.cpp:
(WebCore::DisplayList::DrawGlyphs::generateGlyphBuffer const):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (265260 => 265261)
--- trunk/Source/WebCore/ChangeLog 2020-08-04 21:03:14 UTC (rev 265260)
+++ trunk/Source/WebCore/ChangeLog 2020-08-04 22:02:26 UTC (rev 265261)
@@ -1,3 +1,60 @@
+2020-08-04 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ Add glyph origins member to GlyphBuffer
+ https://bugs.webkit.org/show_bug.cgi?id=215057
+
+ Reviewed by Darin Adler.
+
+ This is in preparation for https://bugs.webkit.org/show_bug.cgi?id=214769
+ and https://bugs.webkit.org/show_bug.cgi?id=206208.
+
+ The solution for https://bugs.webkit.org/show_bug.cgi?id=214769 requires applying
+ letter-spacing after text shaping. Today, we apply letter-spacing before text
+ shaping, which is wrong. However, if we want to apply letter-spacing after text
+ shaping, we need to use CTFontShapeGlyphs(), which outputs glyph origins in
+ addition to glyph advances. Adding a glyph origins field to GlyphBuffer allows us
+ to flatten these origins at the appropriate places.
+
+ This patch is meant to be applied on top of
+ https://bugs.webkit.org/show_bug.cgi?id=215051, which decreases the inline sizes
+ of these vector members from 2048 to 1024. I measured the median and mean of these
+ strings in our Page Load Test to be 6 and 7.4, which indicates we were being wildly
+ inefficient with the members of GlyphBuffer. Decreasing these inline sizes means
+ we're now using less memory than we were before.
+
+ No new tests becasue there is no behavior change yet. This patch just adds the
+ field. The next patch will hook up CTFontShapeGlyphs() itself.
+
+ * platform/graphics/FontCascade.cpp:
+ (WebCore::FontCascade::drawText const):
+ (WebCore::FontCascade::drawEmphasisMarks const):
+ (WebCore::FontCascade::displayListForTextRun const):
+ (WebCore::FontCascade::drawGlyphBuffer const):
+ * platform/graphics/GlyphBuffer.h:
+ (WebCore::GlyphBufferAdvance::GlyphBufferAdvance):
+ (WebCore::GlyphBufferOrigin::GlyphBufferOrigin):
+ (WebCore::GlyphBufferOrigin::operator FloatPoint):
+ (WebCore::GlyphBufferOrigin::setX):
+ (WebCore::GlyphBufferOrigin::setY):
+ (WebCore::GlyphBufferOrigin::x const):
+ (WebCore::GlyphBufferOrigin::y const):
+ (WebCore::GlyphBufferOrigin::encode const):
+ (WebCore::GlyphBufferOrigin::decode):
+ (WebCore::GlyphBuffer::clear):
+ (WebCore::GlyphBuffer::origins):
+ (WebCore::GlyphBuffer::origins const):
+ (WebCore::GlyphBuffer::add):
+ (WebCore::GlyphBuffer::remove):
+ (WebCore::GlyphBuffer::makeHole):
+ (WebCore::GlyphBuffer::shrink):
+ (WebCore::GlyphBuffer::flatten):
+ (WebCore::GlyphBuffer::isFlattened const):
+ (WebCore::GlyphBuffer::swap):
+ * platform/graphics/GraphicsContext.cpp:
+ (WebCore::GraphicsContext::drawGlyphs):
+ * platform/graphics/displaylists/DisplayListItems.cpp:
+ (WebCore::DisplayList::DrawGlyphs::generateGlyphBuffer const):
+
2020-08-04 Chris Fleizach <cfleiz...@apple.com>
AX: VoiceOver needs access to font styling at insertion point
Modified: trunk/Source/WebCore/platform/graphics/FontCascade.cpp (265260 => 265261)
--- trunk/Source/WebCore/platform/graphics/FontCascade.cpp 2020-08-04 21:03:14 UTC (rev 265260)
+++ trunk/Source/WebCore/platform/graphics/FontCascade.cpp 2020-08-04 22:02:26 UTC (rev 265261)
@@ -297,6 +297,7 @@
{
unsigned destination = to.valueOr(run.length());
auto glyphBuffer = layoutText(codePath(run, from, to), run, from, destination);
+ glyphBuffer.flatten();
if (glyphBuffer.isEmpty())
return FloatSize();
@@ -314,6 +315,7 @@
unsigned destination = to.valueOr(run.length());
auto glyphBuffer = layoutText(codePath(run, from, to), run, from, destination, ForTextEmphasisOrNot::ForTextEmphasis);
+ glyphBuffer.flatten();
if (glyphBuffer.isEmpty())
return;
@@ -333,6 +335,7 @@
codePathToUse = Complex;
auto glyphBuffer = layoutText(codePathToUse, run, from, destination);
+ glyphBuffer.flatten();
if (glyphBuffer.isEmpty())
return nullptr;
@@ -1442,7 +1445,7 @@
void FontCascade::drawGlyphBuffer(GraphicsContext& context, const GlyphBuffer& glyphBuffer, FloatPoint& point, CustomFontNotReadyAction customFontNotReadyAction) const
{
- // Draw each contiguous run of glyphs that use the same font data.
+ ASSERT(glyphBuffer.isFlattened());
const Font* fontData = glyphBuffer.fontAt(0);
FloatPoint startPoint = point;
float nextX = startPoint.x() + glyphBuffer.advanceAt(0).width();
@@ -1488,6 +1491,7 @@
void FontCascade::drawEmphasisMarks(GraphicsContext& context, const GlyphBuffer& glyphBuffer, const AtomString& mark, const FloatPoint& point) const
{
+ ASSERT(glyphBuffer.isFlattened());
Optional<GlyphData> markGlyphData = getEmphasisMarkGlyphData(mark);
if (!markGlyphData)
return;
Modified: trunk/Source/WebCore/platform/graphics/GlyphBuffer.h (265260 => 265261)
--- trunk/Source/WebCore/platform/graphics/GlyphBuffer.h 2020-08-04 21:03:14 UTC (rev 265260)
+++ trunk/Source/WebCore/platform/graphics/GlyphBuffer.h 2020-08-04 22:02:26 UTC (rev 265261)
@@ -55,9 +55,8 @@
struct GlyphBufferAdvance : CGSize {
public:
- GlyphBufferAdvance() : CGSize(CGSizeZero) { }
- GlyphBufferAdvance(CGSize size)
- : CGSize(size)
+ GlyphBufferAdvance()
+ : CGSize(CGSizeZero)
{
}
GlyphBufferAdvance(FloatSize size)
@@ -74,10 +73,10 @@
operator FloatSize() { return { static_cast<float>(this->CGSize::width), static_cast<float>(this->CGSize::height) }; }
- void setWidth(CGFloat width) { this->CGSize::width = width; }
- void setHeight(CGFloat height) { this->CGSize::height = height; }
- CGFloat width() const { return this->CGSize::width; }
- CGFloat height() const { return this->CGSize::height; }
+ void setWidth(float width) { this->CGSize::width = width; }
+ void setHeight(float height) { this->CGSize::height = height; }
+ float width() const { return this->CGSize::width; }
+ float height() const { return this->CGSize::height; }
};
template<class Encoder>
@@ -90,24 +89,74 @@
template<class Decoder>
Optional<GlyphBufferAdvance> GlyphBufferAdvance::decode(Decoder& decoder)
{
- Optional<CGFloat> width;
+ Optional<float> width;
decoder >> width;
if (!width)
return WTF::nullopt;
- Optional<CGFloat> height;
+ Optional<float> height;
decoder >> height;
if (!height)
return WTF::nullopt;
- return GlyphBufferAdvance(CGSizeMake(*width, *height));
+ return GlyphBufferAdvance(*width, *height);
}
+struct GlyphBufferOrigin : CGPoint {
+public:
+ GlyphBufferOrigin()
+ : CGPoint(CGPointZero)
+ {
+ }
+ GlyphBufferOrigin(FloatPoint point)
+ : CGPoint(point)
+ {
+ }
+ GlyphBufferOrigin(float x, float y)
+ : CGPoint(CGPointMake(x, y))
+ {
+ }
+
+ template<class Encoder> void encode(Encoder&) const;
+ template<class Decoder> static Optional<GlyphBufferOrigin> decode(Decoder&);
+
+ operator FloatPoint() { return { static_cast<float>(this->CGPoint::x), static_cast<float>(this->CGPoint::y) }; }
+
+ void setX(float x) { this->CGPoint::x = x; }
+ void setY(float y) { this->CGPoint::y = y; }
+ float x() const { return this->CGPoint::x; }
+ float y() const { return this->CGPoint::y; }
+};
+
+template<class Encoder>
+void GlyphBufferOrigin::encode(Encoder& encoder) const
+{
+ encoder << x();
+ encoder << y();
+}
+
+template<class Decoder>
+Optional<GlyphBufferOrigin> GlyphBufferOrigin::decode(Decoder& decoder)
+{
+ Optional<float> x;
+ decoder >> x;
+ if (!x)
+ return WTF::nullopt;
+
+ Optional<float> y;
+ decoder >> y;
+ if (!y)
+ return WTF::nullopt;
+
+ return GlyphBufferOrigin(*x, *y);
+}
+
using GlyphBufferStringOffset = CFIndex;
#else
using GlyphBufferAdvance = FloatSize;
+using GlyphBufferOrigin = FloatPoint;
using GlyphBufferStringOffset = unsigned;
#endif // #if USE(CG)
@@ -127,6 +176,7 @@
m_fonts.clear();
m_glyphs.clear();
m_advances.clear();
+ m_origins.clear();
m_offsetsInString.clear();
}
@@ -133,10 +183,12 @@
const Font** fonts(unsigned from) { return m_fonts.data() + from; }
GlyphBufferGlyph* glyphs(unsigned from) { return m_glyphs.data() + from; }
GlyphBufferAdvance* advances(unsigned from) { return m_advances.data() + from; }
+ GlyphBufferOrigin* origins(unsigned from) { return m_origins.data() + from; }
GlyphBufferStringOffset* offsetsInString(unsigned from) { return m_offsetsInString.data() + from; }
const Font* const * fonts(unsigned from) const { return m_fonts.data() + from; }
const GlyphBufferGlyph* glyphs(unsigned from) const { return m_glyphs.data() + from; }
const GlyphBufferAdvance* advances(unsigned from) const { return m_advances.data() + from; }
+ const GlyphBufferOrigin* origins(unsigned from) const { return m_origins.data() + from; }
const GlyphBufferStringOffset* offsetsInString(unsigned from) const { return m_offsetsInString.data() + from; }
const Font* fontAt(unsigned index) const { return m_fonts[index]; }
@@ -161,6 +213,7 @@
m_fonts.append(font);
m_glyphs.append(glyph);
m_advances.append(advance);
+ m_origins.append(GlyphBufferOrigin());
m_offsetsInString.append(offsetInString);
}
@@ -169,6 +222,7 @@
m_fonts.remove(location, length);
m_glyphs.remove(location, length);
m_advances.remove(location, length);
+ m_origins.remove(location, length);
m_offsetsInString.remove(location, length);
}
@@ -179,6 +233,7 @@
m_fonts.insertVector(location, Vector<const Font*>(length, font));
m_glyphs.insertVector(location, Vector<GlyphBufferGlyph>(length, 0xFFFF));
m_advances.insertVector(location, Vector<GlyphBufferAdvance>(length, GlyphBufferAdvance(0, 0)));
+ m_origins.insertVector(location, Vector<GlyphBufferOrigin>(length, GlyphBufferOrigin()));
m_offsetsInString.insertVector(location, Vector<GlyphBufferStringOffset>(length, 0));
}
@@ -208,25 +263,56 @@
m_fonts.shrink(truncationPoint);
m_glyphs.shrink(truncationPoint);
m_advances.shrink(truncationPoint);
+ m_origins.shrink(truncationPoint);
m_offsetsInString.shrink(truncationPoint);
}
+ // FontCascade::layoutText() returns a GlyphBuffer which includes layout information that is split
+ // into "advances" and "origins". See the ASCII-art diagram in ComplexTextController.h
+ // In order to get paint advances, we need to run this "flatten" operation.
+ // This merges the layout advances and origins together,
+ // leaves the paint advances in the "m_advances" field,
+ // and zeros-out the origins in the "m_origins" field.
+ void flatten()
+ {
+ for (unsigned i = 0; i < size(); ++i) {
+ m_advances[i] = GlyphBufferAdvance(
+ -m_origins[i].x() + m_advances[i].width() + (i + 1 < size() ? m_origins[i + 1].x() : 0),
+ -m_origins[i].y() + m_advances[i].height() + (i + 1 < size() ? m_origins[i + 1].y() : 0)
+ );
+ m_origins[i] = GlyphBufferOrigin();
+ }
+ }
+
+ bool isFlattened() const
+ {
+ for (unsigned i = 0; i < size(); ++i) {
+ if (m_origins[i] != GlyphBufferOrigin())
+ return false;
+ }
+ return true;
+ }
+
private:
void swap(unsigned index1, unsigned index2)
{
- const Font* font = m_fonts[index1];
+ auto font = m_fonts[index1];
m_fonts[index1] = m_fonts[index2];
m_fonts[index2] = font;
- GlyphBufferGlyph glyph = m_glyphs[index1];
+ auto glyph = m_glyphs[index1];
m_glyphs[index1] = m_glyphs[index2];
m_glyphs[index2] = glyph;
- GlyphBufferAdvance advance = m_advances[index1];
+ auto advance = m_advances[index1];
m_advances[index1] = m_advances[index2];
m_advances[index2] = advance;
- GlyphBufferStringOffset offset = m_offsetsInString[index1];
+ auto origin = m_origins[index1];
+ m_origins[index1] = m_origins[index2];
+ m_origins[index2] = origin;
+
+ auto offset = m_offsetsInString[index1];
m_offsetsInString[index1] = m_offsetsInString[index2];
m_offsetsInString[index2] = offset;
}
@@ -234,6 +320,7 @@
Vector<const Font*, 1024> m_fonts;
Vector<GlyphBufferGlyph, 1024> m_glyphs;
Vector<GlyphBufferAdvance, 1024> m_advances;
+ Vector<GlyphBufferOrigin, 1024> m_origins;
Vector<GlyphBufferStringOffset, 1024> m_offsetsInString;
GlyphBufferAdvance m_initialAdvance;
};
Modified: trunk/Source/WebCore/platform/graphics/GraphicsContext.cpp (265260 => 265261)
--- trunk/Source/WebCore/platform/graphics/GraphicsContext.cpp 2020-08-04 21:03:14 UTC (rev 265260)
+++ trunk/Source/WebCore/platform/graphics/GraphicsContext.cpp 2020-08-04 22:02:26 UTC (rev 265261)
@@ -671,6 +671,7 @@
void GraphicsContext::drawGlyphs(const Font& font, const GlyphBuffer& buffer, unsigned from, unsigned numGlyphs, const FloatPoint& point, FontSmoothingMode fontSmoothingMode)
{
+ ASSERT(buffer.isFlattened());
if (paintingDisabled())
return;
Modified: trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp (265260 => 265261)
--- trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp 2020-08-04 21:03:14 UTC (rev 265260)
+++ trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp 2020-08-04 22:02:26 UTC (rev 265261)
@@ -543,9 +543,8 @@
inline GlyphBuffer DrawGlyphs::generateGlyphBuffer() const
{
GlyphBuffer result;
- for (size_t i = 0; i < m_glyphs.size(); ++i) {
+ for (size_t i = 0; i < m_glyphs.size(); ++i)
result.add(m_glyphs[i], &m_font.get(), m_advances[i], GlyphBuffer::noOffset);
- }
return result;
}
