Diff
Modified: trunk/LayoutTests/ChangeLog (286888 => 286889)
--- trunk/LayoutTests/ChangeLog 2021-12-11 00:53:30 UTC (rev 286888)
+++ trunk/LayoutTests/ChangeLog 2021-12-11 00:53:37 UTC (rev 286889)
@@ -1,3 +1,14 @@
+2021-12-10 Myles C. Maxfield <[email protected]>
+
+ [Cocoa] OT-SVG glyphs don't draw into canvases (because of the GPU process)
+ https://bugs.webkit.org/show_bug.cgi?id=234171
+ <rdar://problem/70166552>
+
+ Reviewed by Devin Rousso.
+
+ * fast/text/otsvg-canvas-expected.html: Added.
+ * fast/text/otsvg-canvas.html: Added.
+
2021-12-10 Robert Jenner <[email protected]>
REGRESSION(r286795):REBASELINE [ iOS EWS ] 4X CSS (layout-tests) are constant text failures
Added: trunk/LayoutTests/fast/text/otsvg-canvas-expected.html (0 => 286889)
--- trunk/LayoutTests/fast/text/otsvg-canvas-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/otsvg-canvas-expected.html 2021-12-11 00:53:37 UTC (rev 286889)
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+ font-family: "WebFont";
+ src: url("resources/Ahem-SVG.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+<p>
+This test makes sure that OT-SVG glyphs can be drawn into canvas.
+The test passes if you see:
+<ol>
+ <li>A medium-sized black square, then</li>
+ <li>A slightly smaller green square, then</li>
+ <li>A medium-sized black square, then</li>
+ <li>A slightly smaller green square</li>
+</p>
+<canvas id="canvas" width="400" height="400"></canvas>
+<script>
+var context = document.getElementById("canvas").getContext("2d");
+context.fillStyle = "black";
+context.fillRect(10, 60, 50, 50);
+context.fillStyle = "green";
+context.fillRect(65, 65, 40, 40);
+context.fillStyle = "black";
+context.fillRect(110, 60, 50, 50);
+context.fillStyle = "green";
+context.fillRect(165, 65, 40, 40);
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/fast/text/otsvg-canvas.html (0 => 286889)
--- trunk/LayoutTests/fast/text/otsvg-canvas.html (rev 0)
+++ trunk/LayoutTests/fast/text/otsvg-canvas.html 2021-12-11 00:53:37 UTC (rev 286889)
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+ font-family: "WebFont";
+ src: url("resources/Ahem-SVG.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+<p>
+This test makes sure that OT-SVG glyphs can be drawn into canvas.
+The test passes if you see:
+<ol>
+ <li>A medium-sized black square, then</li>
+ <li>A slightly smaller green square, then</li>
+ <li>A medium-sized black square, then</li>
+ <li>A slightly smaller green square</li>
+</p>
+<canvas id="canvas" width="400" height="400"></canvas>
+<script>
+var context = document.getElementById("canvas").getContext("2d");
+[...document.fonts][0].load().then(function() {
+ context.font = "50px 'WebFont'";
+ context.fillText("bAbA", 10, 100);
+});
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (286888 => 286889)
--- trunk/Source/WebCore/ChangeLog 2021-12-11 00:53:30 UTC (rev 286888)
+++ trunk/Source/WebCore/ChangeLog 2021-12-11 00:53:37 UTC (rev 286889)
@@ -1,3 +1,34 @@
+2021-12-10 Myles C. Maxfield <[email protected]>
+
+ [Cocoa] OT-SVG glyphs don't draw into canvases (because of the GPU process)
+ https://bugs.webkit.org/show_bug.cgi?id=234171
+ <rdar://problem/70166552>
+
+ Reviewed by Devin Rousso.
+
+ Drawing OT-SVG glyphs into canvas was intentionally disabled in https://trac.webkit.org/changeset/269211/webkit.
+ This patch enables it again. Rather than doing anything complicated like supporting all of SVG in DrawGlyphsRecorder,
+ we can simply support this by drawing the glyphs into a ImageBuffer and sending the ImageBuffer to the GPU process.
+
+ For text, it's pretty important that the pixel grid of the ImageBuffer matches the pixel grid of the destination,
+ rather than being offset by half a pixel or something. This patch adds a new creation function to ImageBuffer which
+ accepts a FloatRect (instead of the previous FloatSize which it used to accept). The FloatRect is necessary because
+ inflating the geometry has to happen on both the left and the right if we want the pixel grids to match.
+
+ Test: fast/text/otsvg-canvas.html
+
+ * platform/graphics/DrawGlyphsRecorder.h:
+ * platform/graphics/ImageBuffer.cpp:
+ (WebCore::ImageBuffer::createCompatibleBuffer):
+ (WebCore::ImageBuffer::compatibleBufferInfo):
+ * platform/graphics/ImageBuffer.h:
+ * platform/graphics/coretext/DrawGlyphsRecorderCoreText.cpp:
+ (WebCore::DrawGlyphsRecorder::drawOTSVGRun):
+ (WebCore::DrawGlyphsRecorder::drawNonOTSVGRun):
+ (WebCore::DrawGlyphsRecorder::drawBySplittingIntoOTSVGAndNonOTSVGRuns):
+ (WebCore::DrawGlyphsRecorder::drawGlyphs):
+ (WebCore::filterOutOTSVGGlyphs): Deleted.
+
2021-12-10 Megan Gardner <[email protected]>
Image does not update after Markup Pane is dismissed.
Modified: trunk/Source/WebCore/platform/graphics/DrawGlyphsRecorder.h (286888 => 286889)
--- trunk/Source/WebCore/platform/graphics/DrawGlyphsRecorder.h 2021-12-11 00:53:30 UTC (rev 286888)
+++ trunk/Source/WebCore/platform/graphics/DrawGlyphsRecorder.h 2021-12-11 00:53:37 UTC (rev 286889)
@@ -75,6 +75,10 @@
UniqueRef<GraphicsContext> createInternalContext();
#endif
+ void drawBySplittingIntoOTSVGAndNonOTSVGRuns(const Font&, const GlyphBufferGlyph*, const GlyphBufferAdvance*, unsigned numGlyphs, const FloatPoint& anchorPoint, FontSmoothingMode);
+ void drawOTSVGRun(const Font&, const GlyphBufferGlyph*, const GlyphBufferAdvance*, unsigned numGlyphs, const FloatPoint& anchorPoint, FontSmoothingMode);
+ void drawNonOTSVGRun(const Font&, const GlyphBufferGlyph*, const GlyphBufferAdvance*, unsigned numGlyphs, const FloatPoint& anchorPoint, FontSmoothingMode);
+
void populateInternalState(const GraphicsContextState&);
void populateInternalContext(const GraphicsContextState&);
void prepareInternalContext(const Font&, FontSmoothingMode);
Modified: trunk/Source/WebCore/platform/graphics/ImageBuffer.cpp (286888 => 286889)
--- trunk/Source/WebCore/platform/graphics/ImageBuffer.cpp 2021-12-11 00:53:30 UTC (rev 286888)
+++ trunk/Source/WebCore/platform/graphics/ImageBuffer.cpp 2021-12-11 00:53:37 UTC (rev 286889)
@@ -90,11 +90,37 @@
if (!imageBuffer)
return nullptr;
- // Set up a corresponding scale factor on the graphics context.
imageBuffer->context().scale(scaledSize / size);
return imageBuffer;
}
+// This is useful when you need to make sure the pixel grid of the ImageBuffer aligns with the pixel grid of the context.
+// Simply saying createCompatibleBuffer(rect.size(), context) isn't sufficient in this situation, because rect.location() may not lie on a pixel boundary.
+// In this situation, we have to inflate both the left side and the right side, which can lead to different results than
+// createCompatibleBuffer(rect.size(), context) would have produced.
+auto ImageBuffer::createCompatibleBuffer(const FloatRect& rect, const GraphicsContext& context) -> std::optional<CompatibleBufferDescription>
+{
+ if (rect.isEmpty())
+ return std::nullopt;
+
+ auto info = ImageBuffer::compatibleBufferInfo(rect, context);
+
+ RefPtr<ImageBuffer> imageBuffer;
+
+ if (context.renderingMode() == RenderingMode::Accelerated)
+ imageBuffer = AcceleratedImageBuffer::create(info.physicalSizeInDeviceCoordinates, context);
+
+ if (!imageBuffer)
+ imageBuffer = UnacceleratedImageBuffer::create(info.physicalSizeInDeviceCoordinates, context);
+
+ if (!imageBuffer)
+ return std::nullopt;
+
+ imageBuffer->context().scale(info.scale);
+ imageBuffer->context().translate(-info.inflatedRectInUserCoordinates.location());
+ return { { imageBuffer.releaseNonNull(), info.inflatedRectInUserCoordinates } };
+}
+
RefPtr<ImageBuffer> ImageBuffer::createCompatibleBuffer(const FloatSize& size, const DestinationColorSpace& colorSpace, const GraphicsContext& context)
{
if (size.isEmpty())
@@ -106,7 +132,6 @@
if (!imageBuffer)
return nullptr;
- // Set up a corresponding scale factor on the graphics context.
imageBuffer->context().scale(scaledSize / size);
return imageBuffer;
}
@@ -167,6 +192,36 @@
return expandedIntSize(size * context.scaleFactor());
}
+auto ImageBuffer::compatibleBufferInfo(const FloatRect& rect, const GraphicsContext& context) -> CompatibleBufferInfo
+{
+ auto scaleFactor = context.scaleFactor();
+ auto scaledRect = rect;
+ scaledRect.scale(scaleFactor);
+ auto inflatedScaledRect = enclosingIntRect(scaledRect);
+ auto inflatedRectInUserCoordinates = FloatRect(inflatedScaledRect);
+
+ // We don't want to allocate huge ImageBuffers because they can take a lot of memory,
+ // so if the size would have been too big, decrease resolution and scale the resulting context.
+ // This will mean that the ImageBuffer will lose quality, but will still generally look correct.
+ constexpr int maxDimension = 4096;
+ if (inflatedScaledRect.width() > maxDimension) {
+ // The pixel grids won't align any more, so there's no need to try to handle fractions of pixels.
+ inflatedScaledRect.setWidth(maxDimension);
+ inflatedRectInUserCoordinates.scale(maxDimension / inflatedRectInUserCoordinates.width(), 1);
+ scaleFactor.setWidth(maxDimension / rect.width());
+ }
+ if (inflatedScaledRect.height() > maxDimension) {
+ // The pixel grids won't align any more, so there's no need to try to handle fractions of pixels.
+ inflatedScaledRect.setHeight(maxDimension);
+ inflatedRectInUserCoordinates.scale(1, maxDimension / inflatedRectInUserCoordinates.height());
+ scaleFactor.setHeight(maxDimension / rect.height());
+ }
+
+ auto physicalSizeInDeviceCoordinates = inflatedScaledRect.size();
+ inflatedRectInUserCoordinates.scale(1.0f / scaleFactor);
+ return { WTFMove(physicalSizeInDeviceCoordinates), WTFMove(inflatedRectInUserCoordinates), scaleFactor };
+}
+
RefPtr<ImageBuffer> ImageBuffer::copyRectToBuffer(const FloatRect& rect, const DestinationColorSpace& colorSpace, const GraphicsContext& context)
{
if (rect.isEmpty())
Modified: trunk/Source/WebCore/platform/graphics/ImageBuffer.h (286888 => 286889)
--- trunk/Source/WebCore/platform/graphics/ImageBuffer.h 2021-12-11 00:53:30 UTC (rev 286888)
+++ trunk/Source/WebCore/platform/graphics/ImageBuffer.h 2021-12-11 00:53:37 UTC (rev 286889)
@@ -52,6 +52,11 @@
// Create an image buffer compatible with the context, with suitable resolution for drawing into the buffer and then into this context.
static RefPtr<ImageBuffer> createCompatibleBuffer(const FloatSize&, const GraphicsContext&);
+ struct CompatibleBufferDescription {
+ Ref<ImageBuffer> imageBuffer;
+ FloatRect inflatedRectInUserCoordinates;
+ };
+ static std::optional<CompatibleBufferDescription> createCompatibleBuffer(const FloatRect& rectInUserCoordinates, const GraphicsContext&);
WEBCORE_EXPORT static RefPtr<ImageBuffer> createCompatibleBuffer(const FloatSize&, const DestinationColorSpace&, const GraphicsContext&);
static RefPtr<ImageBuffer> createCompatibleBuffer(const FloatSize&, float resolutionScale, const DestinationColorSpace&, const GraphicsContext&);
@@ -63,6 +68,12 @@
static FloatRect clampedRect(const FloatRect&);
static IntSize compatibleBufferSize(const FloatSize&, const GraphicsContext&);
+ struct CompatibleBufferInfo {
+ IntSize physicalSizeInDeviceCoordinates;
+ FloatRect inflatedRectInUserCoordinates;
+ FloatSize scale;
+ };
+ static CompatibleBufferInfo compatibleBufferInfo(const FloatRect&, const GraphicsContext&);
WEBCORE_EXPORT virtual ~ImageBuffer() = default;
Modified: trunk/Source/WebCore/platform/graphics/coretext/DrawGlyphsRecorderCoreText.cpp (286888 => 286889)
--- trunk/Source/WebCore/platform/graphics/coretext/DrawGlyphsRecorderCoreText.cpp 2021-12-11 00:53:30 UTC (rev 286888)
+++ trunk/Source/WebCore/platform/graphics/coretext/DrawGlyphsRecorderCoreText.cpp 2021-12-11 00:53:37 UTC (rev 286889)
@@ -34,6 +34,7 @@
#include "FontPlatformData.h"
#include "GlyphBuffer.h"
#include "GraphicsContextCG.h"
+#include "ImageBuffer.h"
#include <CoreText/CoreText.h>
#include <wtf/Vector.h>
@@ -370,53 +371,65 @@
m_owner.translate(0, -(rect.size.height + 2 * rect.origin.y));
}
-struct GlyphsAndAdvancesStorage {
- Vector<GlyphBufferGlyph> glyphs;
- Vector<GlyphBufferAdvance> advances;
-};
+void DrawGlyphsRecorder::drawOTSVGRun(const Font& font, const GlyphBufferGlyph* glyphs, const GlyphBufferAdvance* advances, unsigned numGlyphs, const FloatPoint& startPoint, FontSmoothingMode smoothingMode)
+{
+ FloatPoint penPosition = startPoint;
+ for (unsigned i = 0; i < numGlyphs; ++i) {
+ auto bounds = font.boundsForGlyph(glyphs[i]);
+ if (auto imageBufferDescription = ImageBuffer::createCompatibleBuffer(bounds, m_owner)) {
+ FontCascade::drawGlyphs(imageBufferDescription->imageBuffer->context(), font, glyphs + i, advances + i, 1, FloatPoint(), smoothingMode);
+ auto destinationRect = imageBufferDescription->inflatedRectInUserCoordinates;
+ destinationRect.moveBy(penPosition);
+ m_owner.drawImageBuffer(imageBufferDescription->imageBuffer, destinationRect);
+ }
+ penPosition.move(size(advances[i]));
+ }
+}
-struct GlyphsAndAdvances {
- const GlyphBufferGlyph* glyphs;
- const GlyphBufferAdvance* advances;
- unsigned numGlyphs;
- GlyphBufferAdvance initialAdvance;
- std::optional<GlyphsAndAdvancesStorage> storage;
-};
+void DrawGlyphsRecorder::drawNonOTSVGRun(const Font& font, const GlyphBufferGlyph* glyphs, const GlyphBufferAdvance* advances, unsigned numGlyphs, const FloatPoint& startPoint, FontSmoothingMode smoothingMode)
+{
+ prepareInternalContext(font, smoothingMode);
+ FontCascade::drawGlyphs(m_internalContext, font, glyphs, advances, numGlyphs, startPoint, smoothingMode);
+ concludeInternalContext();
+}
-static GlyphsAndAdvances filterOutOTSVGGlyphs(const Font& font, const GlyphBufferGlyph* glyphs, const GlyphBufferAdvance* advances, unsigned numGlyphs)
+void DrawGlyphsRecorder::drawBySplittingIntoOTSVGAndNonOTSVGRuns(const Font& font, const GlyphBufferGlyph* glyphs, const GlyphBufferAdvance* advances, unsigned numGlyphs, const FloatPoint& startPoint, FontSmoothingMode smoothingMode)
{
auto otsvgGlyphs = font.findOTSVGGlyphs(glyphs, numGlyphs);
- if (!otsvgGlyphs)
- return { glyphs, advances, numGlyphs, makeGlyphBufferAdvance(), { }};
+ if (!otsvgGlyphs) {
+ drawNonOTSVGRun(font, glyphs, advances, numGlyphs, startPoint, smoothingMode);
+ return;
+ }
ASSERT(otsvgGlyphs->size() >= numGlyphs);
- GlyphsAndAdvances result;
- result.initialAdvance = makeGlyphBufferAdvance();
- result.storage = GlyphsAndAdvancesStorage();
-
- result.storage->glyphs.reserveInitialCapacity(numGlyphs);
- result.storage->advances.reserveInitialCapacity(numGlyphs);
-
- for (unsigned i = 0; i < numGlyphs; ++i) {
- ASSERT(result.storage->glyphs.size() == result.storage->advances.size());
- if (otsvgGlyphs->quickGet(i)) {
- if (result.storage->advances.isEmpty())
- result.initialAdvance = makeGlyphBufferAdvance(size(result.initialAdvance) + size(advances[i]));
- else
- result.storage->advances.last() = makeGlyphBufferAdvance(size(result.storage->advances.last()) + size(advances[i]));
- } else {
- result.storage->glyphs.uncheckedAppend(glyphs[i]);
- result.storage->advances.uncheckedAppend(advances[i]);
+ // We can't just partition the glyphs into OT-SVG glyphs and non-OT-SVG glyphs because glyphs are allowed to draw outside of their layout boxes.
+ // This means that glyphs can overlap, which means we have to get the z-order correct. We can't have an earlier run be drawn on top of a later run.
+ FloatPoint runOrigin = startPoint;
+ FloatPoint penPosition = startPoint;
+ size_t glyphCountInRun = 0;
+ bool isOTSVGRun = false;
+ unsigned i;
+ auto draw = [&] () {
+ if (!glyphCountInRun)
+ return;
+ if (isOTSVGRun)
+ drawOTSVGRun(font, glyphs + i - glyphCountInRun, advances + i - glyphCountInRun, glyphCountInRun, runOrigin, smoothingMode);
+ else
+ drawNonOTSVGRun(font, glyphs + i - glyphCountInRun, advances + i - glyphCountInRun, glyphCountInRun, runOrigin, smoothingMode);
+ };
+ for (i = 0; i < numGlyphs; ++i) {
+ bool isOTSVGGlyph = otsvgGlyphs->quickGet(i);
+ if (isOTSVGGlyph != isOTSVGRun) {
+ draw();
+ isOTSVGRun = isOTSVGGlyph;
+ glyphCountInRun = 0;
+ runOrigin = penPosition;
}
- ASSERT(result.storage->glyphs.size() == result.storage->advances.size());
+ ++glyphCountInRun;
+ penPosition.move(size(advances[i]));
}
-
- result.glyphs = result.storage->glyphs.data();
- result.advances = result.storage->advances.data();
- result.numGlyphs = result.storage->glyphs.size();
-
- return result;
+ draw();
}
void DrawGlyphsRecorder::drawGlyphs(const Font& font, const GlyphBufferGlyph* glyphs, const GlyphBufferAdvance* advances, unsigned numGlyphs, const FloatPoint& startPoint, FontSmoothingMode smoothingMode)
@@ -428,14 +441,7 @@
ASSERT(m_deconstructDrawGlyphs == DeconstructDrawGlyphs::Yes);
- // FIXME: <rdar://problem/70166552> Record OTSVG glyphs.
- GlyphsAndAdvances glyphsAndAdvancesWithoutOTSVGGlyphs = filterOutOTSVGGlyphs(font, glyphs, advances, numGlyphs);
- ASSERT(glyphsAndAdvancesWithoutOTSVGGlyphs.glyphs == glyphs || glyphsAndAdvancesWithoutOTSVGGlyphs.glyphs == glyphsAndAdvancesWithoutOTSVGGlyphs.storage->glyphs.data());
- ASSERT(glyphsAndAdvancesWithoutOTSVGGlyphs.advances == advances || glyphsAndAdvancesWithoutOTSVGGlyphs.advances == glyphsAndAdvancesWithoutOTSVGGlyphs.storage->advances.data());
-
- prepareInternalContext(font, smoothingMode);
- FontCascade::drawGlyphs(m_internalContext, font, glyphsAndAdvancesWithoutOTSVGGlyphs.glyphs, glyphsAndAdvancesWithoutOTSVGGlyphs.advances, glyphsAndAdvancesWithoutOTSVGGlyphs.numGlyphs, startPoint + size(glyphsAndAdvancesWithoutOTSVGGlyphs.initialAdvance), smoothingMode);
- concludeInternalContext();
+ drawBySplittingIntoOTSVGAndNonOTSVGRuns(font, glyphs, advances, numGlyphs, startPoint, smoothingMode);
}
void DrawGlyphsRecorder::drawNativeText(CTFontRef font, CGFloat fontSize, CTLineRef line, CGRect lineRect)
