Attachment 432982 Details for Bug 227550 – Patch

[patch] Patch

bug-227550-20210706152638.patch (text/plain), 9.20 KB, created by Myles C. Maxfield on 2021-07-06 15:26:39 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Myles C. Maxfield

Created: 2021-07-06 15:26:39 PDT

Size: 9.20 KB

patch

obsolete

>Subversion Revision: 279603
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index e9d1e0f6a56653e815ea1d399dc4241411e1c9f7..2d3213f8bcc8351cd6f6ab13d633731a67bd4777 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,25 @@
>+2021-06-30  Myles C. Maxfield  <mmaxfield@apple.com>
>+
>+        [GPU Process] Draw PDFs using an intermediate ImageBuffer when using the GPU process
>+        https://bugs.webkit.org/show_bug.cgi?id=227550
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When drawing PDFs, we usually already have a cached ImageBuffer of the contents of the PDF.
>+        However, in the situation where we don't have this cached ImageBuffer, we can't just draw
>+        the PDF into the platformContext when we're using the GPU Process. So, instead, we need to
>+        draw the PDF into an intermediate ImageBuffer and then send that to the GPU Process.
>+
>+        When we have a fully functional custom CGContext, we can use that to send all the PDF drawing
>+        commands to the GPU Process, and this patch should then be reverted.
>+
>+        Test: fast/images/pdf-in-canvas.html
>+
>+        * platform/graphics/cg/PDFDocumentImage.cpp:
>+        (WebCore::PDFDocumentImage::draw):
>+        * platform/mac/LocalCurrentGraphicsContextMac.mm:
>+        (WebCore::LocalCurrentGraphicsContext::LocalCurrentGraphicsContext):
>+
> 2021-07-06  Youenn Fablet  <youenn@apple.com>
> 
>         Unable to use 'data:application/javascript' url for Worker
>diff --git a/Source/WebCore/platform/graphics/cg/PDFDocumentImage.cpp b/Source/WebCore/platform/graphics/cg/PDFDocumentImage.cpp
>index dc4985f9c59aaacd3df5b5dc56b9d8908481bf9b..d89f437ee438fbc96e72ea95381d45362fa5e47a 100644
>--- a/Source/WebCore/platform/graphics/cg/PDFDocumentImage.cpp
>+++ b/Source/WebCore/platform/graphics/cg/PDFDocumentImage.cpp
>@@ -161,6 +161,8 @@ static void transformContextForPainting(GraphicsContext& context, const FloatRec
>     context.translate(dstRect.location() - srcRect.location());
>     context.scale(FloatSize(hScale, -vScale));
>     context.translate(0, -srcRect.height());
>+
>+    // FIXME: Surely there needs to be a clip operation here, if the srcRect is smaller than the whole PDF.
> }
> 
> // To avoid the jetsam on iOS, we are going to limit the size of all the PDF cachedImages to be 64MB.
>@@ -246,6 +248,7 @@ void PDFDocumentImage::updateCachedImageIfNeeded(GraphicsContext& context, const
>     // We need to transform the coordinate system such that top-left of m_cachedImageRect will be mapped to the
>     // top-left of dstRect. Although only m_cachedImageRect.size() of the image copied, the sizes of srcRect
>     // and dstRect should be passed to this function because they are used to calculate the image scaling.
>+    // FIXME: Passing in m_cachedImageRect.location() for the source rect position is confusing here.
>     transformContextForPainting(bufferContext, dstRect, FloatRect(m_cachedImageRect.location(), srcRect.size()));
>     drawPDFPage(bufferContext);
> 
>@@ -275,10 +278,25 @@ ImageDrawResult PDFDocumentImage::draw(GraphicsContext& context, const FloatRect
>             // of the source PDF was copied to 'm_cachedImageBuffer', the sizes of the source
>             // and the destination rectangles will be equal and no scaling will be needed here.
>             context.drawImageBuffer(*m_cachedImageBuffer, m_cachedImageRect);
>-        }
>-        else {
>+        } else if (context.hasPlatformContext()) {
>             transformContextForPainting(context, dstRect, srcRect);
>             drawPDFPage(context);
>+        } else {
>+            // We can't draw the PDF directly because we have no platform context (probably because we're doing display list drawing).
>+            // We also need to be careful about not getting jetsam'ed.
>+            //
>+            // (scalar * width) * (scalar * height) = max number of pixels
>+            // Solve for scalar...
>+            // scalar = sqrt(max number of pixels / (width * height))
>+            auto scalar = std::min(1.f, std::sqrt(static_cast<float>(s_maxCachedImageArea) / (dstRect.width() * dstRect.height())));
>+            FloatRect localDestinationRect(FloatPoint(), dstRect.size() * scalar);
>+            if (auto imageBuffer = ImageBuffer::createCompatibleBuffer(localDestinationRect.size(), context)) {
>+                auto& bufferContext = imageBuffer->context();
>+                transformContextForPainting(bufferContext, localDestinationRect, srcRect);
>+                drawPDFPage(bufferContext);
>+                context.drawImageBuffer(*imageBuffer, dstRect);
>+            } else
>+                return ImageDrawResult::DidNothing;
>         }
>     }
> 
>diff --git a/Source/WebCore/platform/mac/LocalCurrentGraphicsContextMac.mm b/Source/WebCore/platform/mac/LocalCurrentGraphicsContextMac.mm
>index a41eb4ad88c46bbecc3477bef392d261961f2e3d..54f5a1980be4c8c0b5298ef329ca5dc19b13b124 100644
>--- a/Source/WebCore/platform/mac/LocalCurrentGraphicsContextMac.mm
>+++ b/Source/WebCore/platform/mac/LocalCurrentGraphicsContextMac.mm
>@@ -32,7 +32,8 @@ LocalCurrentGraphicsContext::LocalCurrentGraphicsContext(GraphicsContext& graphi
>     m_savedGraphicsContext.save();
> 
>     if (!m_savedGraphicsContext.hasPlatformContext()) {
>-        WTFLogAlways("LocalCurrentGraphicsContext is not setting the global context because the provided GraphicsContext does not have a platform context (likely display list recording)");
>+        WTFLogAlways("LocalCurrentGraphicsContext is setting the global context to nil because the provided GraphicsContext does not have a platform context (likely display list recording)");
>+        [NSGraphicsContext setCurrentContext:nil];
>         return;
>     }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index c6a6ba41f7a9365dde16b9d8c9c4d269220430c1..68cdf939d651b122ad94aadcabcb0028969ee95f 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2021-06-30  Myles C. Maxfield  <mmaxfield@apple.com>
>+
>+        [GPU Process] Temporarily disable drawing large PDFs in display list drawing
>+        https://bugs.webkit.org/show_bug.cgi?id=227550
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/images/pdf-in-canvas-expected.html: Added.
>+        * fast/images/pdf-in-canvas.html: Added.
>+
> 2021-07-06  Youenn Fablet  <youenn@apple.com>
> 
>         Unable to use 'data:application/javascript' url for Worker
>diff --git a/LayoutTests/fast/images/pdf-in-canvas-expected.html b/LayoutTests/fast/images/pdf-in-canvas-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..7510bf99b7a38da9ecb0b30665d962f19e8dbace
>--- /dev/null
>+++ b/LayoutTests/fast/images/pdf-in-canvas-expected.html
>@@ -0,0 +1,15 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+</head>
>+<body>
>+<p>This test passes if you see a green box below (possibly with a green border).</p>
>+<div style="position: relative;">
>+    <div style="position: absolute; left: 0px; top: 0px; width: 100px; height: 100px; background: #008E00;"></div>
>+    <div style="position: absolute; left: -3px; top: -3px; width: 6px; height: 106px; background: green;"></div>
>+    <div style="position: absolute; left: 97px; top: -3px; width: 6px; height: 106px; background: green;"></div>
>+    <div style="position: absolute; left: -3px; top: -3px; width: 106px; height: 6px; background: green;"></div>
>+    <div style="position: absolute; left: -3px; top: 97px; width: 106px; height: 6px; background: green;"></div
>+</div>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/images/pdf-in-canvas.html b/LayoutTests/fast/images/pdf-in-canvas.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..881282161da8ba4ab69d237a7283c7d7f1940110
>--- /dev/null
>+++ b/LayoutTests/fast/images/pdf-in-canvas.html
>@@ -0,0 +1,44 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+</head>
>+<body>
>+<p>This test passes if you see a green box below (possibly with a green border).</p>
>+<img id="image">
>+<div style="position: relative;">
>+    <canvas id="canvas" width="100" height="100"></canvas>
>+    <div style="position: absolute; left: -3px; top: -3px; width: 6px; height: 106px; background: green;"></div>
>+    <div style="position: absolute; left: 97px; top: -3px; width: 6px; height: 106px; background: green;"></div>
>+    <div style="position: absolute; left: -3px; top: -3px; width: 106px; height: 6px; background: green;"></div>
>+    <div style="position: absolute; left: -3px; top: 97px; width: 106px; height: 6px; background: green;"></div
>+</div>
>+<script>
>+if (window.testRunner) {
>+    testRunner.waitUntilDone();
>+}
>+// Emulate a big PDF on iOS (which has the PDFImageCachingPolicy set to BelowMemoryLimit).
>+if (window.internals)
>+    internals.settings.setPDFImageCachingPolicy("Disabled");
>+let canvas = document.getElementById("canvas");
>+let context = canvas.getContext("2d");
>+let image = document.getElementById("image");
>+image.addEventListener("load", function() {
>+    var state = 0;
>+    function tick() {
>+        context.drawImage(image, 0, 0, 100, 100);
>+        if (state < 10) {
>+            context.fillRect(0, 0, 100, 100);
>+            ++state;
>+            requestAnimationFrame(tick);
>+        } else {
>+            image.style.display = "none";
>+            if (window.testRunner)
>+                testRunner.notifyDone();
>+        }
>+    }
>+    requestAnimationFrame(tick);
>+});
>+image.src = "resources/green_rectangle.pdf";
>+</script>
>+</body>
>+</html>

Flags:

thorton: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 227550: 432635 | 432982