240100 2022-05-04 17:24:57 -0700 [GPU Process] [iOS] REGRESSION(r293570): Snapshot rendering is not scaled with the device scale factor 2022-05-05 01:20:29 -0700 1 1 1 Unclassified WebKit Layout and Rendering WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=239527 https://bugs.webkit.org/show_bug.cgi?id=225081 InRadar P2 Normal --- 1 sabouhallawa sabouhallawa bfulgham simon.fraser webkit-bug-importer zalan oldest_to_newest 1866679 0 sabouhallawa 2022-05-04 17:24:57 -0700 In r293570, the GPU Process was enabled for snapshot images. Before r293570, a local ImageBuffer was created in snapshotFrameRectWithClip(). Then in WebFrame::createSelectionSnapshot(), the pixels of the ImageBuffer were copied to a WebImage, which was backed by a ShareableBitmap. r293570 made snapshotFrameRectWithClip() create a remote ImageBuffer which is moved to a WebImage in WebFrame::createSelectionSnapshot(). This remote ImageBuffer has to have ImageBufferShareableBitmapBackend so it can provide ShareableBitmap without allocating any extra memory. The problem is in initializing the GraphicsContext of ImageBufferShareableBitmapBackend. The scaling factor is not set to the CTM of the ImageBufferShareableBitmapBackend context. See the comment in ImageBufferShareableBitmapBackend constructor and notice that this function does not call applyBaseTransformToContext(). A simple approach to fix this bug is to make snapshotFrameRectWithClip() handle the scaling outside the ImageBuffer creation. This is similar to what we do in GraphicsContext::createAlignedImageBuffer() where we scale the size and create the ImageBuffer with scaleFactor = 1. 1866680 1 sabouhallawa 2022-05-04 17:25:24 -0700 rdar://92635752 1866681 2 458839 sabouhallawa 2022-05-04 17:30:05 -0700 Created attachment 458839 Patch 1866703 3 458839 simon.fraser 2022-05-04 19:14:59 -0700 Comment on attachment 458839 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458839&action=review > Source/WebCore/page/FrameSnapshotting.cpp:124 > - auto buffer = ImageBuffer::create(imageRect.size(), purpose, scaleFactor, options.colorSpace, options.pixelFormat, { }, { hostWindow }); > + auto buffer = ImageBuffer::create(scaledImageRect.size(), purpose, 1, options.colorSpace, options.pixelFormat, { }, { hostWindow }); It's odd that ImageBuffer::create() takes a scale factor argument, but apparently that doesn't behave the way we want? 1866743 4 458839 sabouhallawa 2022-05-04 21:53:46 -0700 Comment on attachment 458839 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458839&action=review >> Source/WebCore/page/FrameSnapshotting.cpp:124 >> + auto buffer = ImageBuffer::create(scaledImageRect.size(), purpose, 1, options.colorSpace, options.pixelFormat, { }, { hostWindow }); > > It's odd that ImageBuffer::create() takes a scale factor argument, but apparently that doesn't behave the way we want? I think it is okay that we want the scaling of the ImageBuffer to happen at the creation time. And it is working for all the other backends. But there is a problem with ImageBufferShareableBitmapBackend which I can't figure it out so far. This patch is correct and it fixes the snapshot bug. But we do have a bug with creating scaled unaccelerated remote ImageBuffer which we never needed before. All the canvas ImageBuffers are not scaled and all the layer ImageBuffers are accelerated. So I will file another bug for this issue which can be fixed later. 1866776 5 ews-feeder 2022-05-05 01:20:26 -0700 Committed r293825 (250298@main): <https://commits.webkit.org/250298@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 458839. 458839 2022-05-04 17:30:05 -0700 2022-05-05 01:20:28 -0700 Patch bug-240100-20220504173004.patch text/plain 2831 sabouhallawa U3VidmVyc2lvbiBSZXZpc2lvbjogMjkzNzIxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMWM0NDkxNGEwZjRkZjE3 NDExNzhkMjc3YWI3MjAyNjZjNmI1ZGExMy4uNDRiNjY4MDg3NzlmNWQwMjFjZThmOTVmZjFlNzhl MjhhMmZjODFjMyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDIyLTA1LTA0ICBTYWlk IEFib3UtSGFsbGF3YSAgPHNhaWRAYXBwbGUuY29tPgorCisgICAgICAgIFtHUFUgUHJvY2Vzc10g W2lPU10gUkVHUkVTU0lPTihyMjkzNTcwKTogU25hcHNob3QgcmVuZGVyaW5nIGlzIG5vdCBzY2Fs ZWQgd2l0aCB0aGUgZGV2aWNlIHNjYWxlIGZhY3RvcgorICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjQwMTAwCisgICAgICAgIHJkYXI6Ly85MjYzNTc1Mgor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBzY2Fs aW5nIGZhY3RvciBpcyBub3Qgc2V0IGluIHRoZSBHcmFwaGljc0NvbnRleHQgb2YgdGhlCisgICAg ICAgIEltYWdlQnVmZmVyU2hhcmVhYmxlQml0bWFwQmFja2VuZC4KKworICAgICAgICBUbyBmaXgg dGhpcyBidWcgaXMgdG8gbWFrZSBzbmFwc2hvdEZyYW1lUmVjdFdpdGhDbGlwKCkgaGFuZGxlIHRo ZSBzY2FsaW5nCisgICAgICAgIG91dHNpZGUgdGhlIEltYWdlQnVmZmVyIGNyZWF0aW9uLiBUaGlz IGlzIHNpbWlsYXIgdG8gd2hhdCB3ZSBkbyBpbgorICAgICAgICBHcmFwaGljc0NvbnRleHQ6OmNy ZWF0ZUFsaWduZWRJbWFnZUJ1ZmZlcigpIHdoZXJlIHdlIHNjYWxlIHRoZSBzaXplIGFuZAorICAg ICAgICBjcmVhdGUgdGhlIEltYWdlQnVmZmVyIHdpdGggc2NhbGVGYWN0b3IgPSAxLgorCisgICAg ICAgICogcGFnZS9GcmFtZVNuYXBzaG90dGluZy5jcHA6CisgICAgICAgIChXZWJDb3JlOjpzbmFw c2hvdEZyYW1lUmVjdFdpdGhDbGlwKToKKwogMjAyMi0wNS0wMyAgWW91ZW5uIEZhYmxldCAgPHlv dWVubkBhcHBsZS5jb20+CiAKICAgICAgICAgU2VydmljZVdvcmtlclJlZ2lzdHJhdGlvbiB1cGRh dGUgc2hvdWxkIGZhaWwgaWYgY2FsbGVkIGZyb20gYW4gaW5zdGFsbGluZyBzZXJ2aWNlIHdvcmtl ciBjb250ZXh0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wYWdlL0ZyYW1lU25hcHNob3R0 aW5nLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BhZ2UvRnJhbWVTbmFwc2hvdHRpbmcuY3BwCmluZGV4 IGUwNTVlMTM5MTczMjhjMjk2ZmE4ZDljYmU4ZGY2MjdlYWQ2ODYyMjYuLjhjNTJmMmM3ZjBiYTA2 OWJmNzA2M2ZkZjQzM2RjMjY5MDNjNjFmYjEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3Bh Z2UvRnJhbWVTbmFwc2hvdHRpbmcuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BhZ2UvRnJhbWVT bmFwc2hvdHRpbmcuY3BwCkBAIC0xMTUsMTkgKzExNSwyMyBAQCBSZWZQdHI8SW1hZ2VCdWZmZXI+ IHNuYXBzaG90RnJhbWVSZWN0V2l0aENsaXAoRnJhbWUmIGZyYW1lLCBjb25zdCBJbnRSZWN0JiBp bWFnZQogICAgIGlmIChvcHRpb25zLmZsYWdzLmNvbnRhaW5zKFNuYXBzaG90RmxhZ3M6OlBhaW50 V2l0aEludGVncmFsU2NhbGVGYWN0b3IpKQogICAgICAgICBzY2FsZUZhY3RvciA9IGNlaWxmKHNj YWxlRmFjdG9yKTsKIAorICAgIGF1dG8gc2NhbGVkSW1hZ2VSZWN0ID0gaW1hZ2VSZWN0OworICAg IHNjYWxlZEltYWdlUmVjdC5zY2FsZShzY2FsZUZhY3Rvcik7CisKICAgICBhdXRvIHB1cnBvc2Ug PSBvcHRpb25zLmZsYWdzLmNvbnRhaW5zKFNuYXBzaG90RmxhZ3M6OlNoYXJlYWJsZSkgPyBSZW5k ZXJpbmdQdXJwb3NlOjpTaGFyZWFibGVTbmFwc2hvdCA6IFJlbmRlcmluZ1B1cnBvc2U6OlNuYXBz aG90OwogICAgIGF1dG8gaG9zdFdpbmRvdyA9IChkb2N1bWVudC0+dmlldygpICYmIGRvY3VtZW50 LT52aWV3KCktPnJvb3QoKSkgPyBkb2N1bWVudC0+dmlldygpLT5yb290KCktPmhvc3RXaW5kb3co KSA6IG51bGxwdHI7CiAKLSAgICBhdXRvIGJ1ZmZlciA9IEltYWdlQnVmZmVyOjpjcmVhdGUoaW1h Z2VSZWN0LnNpemUoKSwgcHVycG9zZSwgc2NhbGVGYWN0b3IsIG9wdGlvbnMuY29sb3JTcGFjZSwg b3B0aW9ucy5waXhlbEZvcm1hdCwgeyB9LCB7IGhvc3RXaW5kb3cgfSk7CisgICAgYXV0byBidWZm ZXIgPSBJbWFnZUJ1ZmZlcjo6Y3JlYXRlKHNjYWxlZEltYWdlUmVjdC5zaXplKCksIHB1cnBvc2Us IDEsIG9wdGlvbnMuY29sb3JTcGFjZSwgb3B0aW9ucy5waXhlbEZvcm1hdCwgeyB9LCB7IGhvc3RX aW5kb3cgfSk7CiAgICAgaWYgKCFidWZmZXIpCiAgICAgICAgIHJldHVybiBudWxscHRyOwogCi0g ICAgYnVmZmVyLT5jb250ZXh0KCkudHJhbnNsYXRlKC1pbWFnZVJlY3QueCgpLCAtaW1hZ2VSZWN0 LnkoKSk7CisgICAgYnVmZmVyLT5jb250ZXh0KCkudHJhbnNsYXRlKC1zY2FsZWRJbWFnZVJlY3Qu bG9jYXRpb24oKSk7CisgICAgYnVmZmVyLT5jb250ZXh0KCkuc2NhbGUoc2NhbGVGYWN0b3IpOwog CiAgICAgaWYgKCFjbGlwUmVjdHMuaXNFbXB0eSgpKSB7CiAgICAgICAgIFBhdGggY2xpcFBhdGg7 CiAgICAgICAgIGZvciAoYXV0byYgcmVjdCA6IGNsaXBSZWN0cykKLSAgICAgICAgICAgIGNsaXBQ YXRoLmFkZFJlY3QoZW5jbG9zZVJlY3RUb0RldmljZVBpeGVscyhyZWN0LCBzY2FsZUZhY3Rvcikp OworICAgICAgICAgICAgY2xpcFBhdGguYWRkUmVjdChyZWN0KTsKICAgICAgICAgYnVmZmVyLT5j b250ZXh0KCkuY2xpcFBhdGgoY2xpcFBhdGgpOwogICAgIH0KIAo=