[webkit-changes] [WebKit/WebKit] bf1b77: REGRESSION (255355 at main): HTML Notes: Attachment t...

Aditya Keerthi noreply at github.com
Mon Jan 30 17:22:21 PST 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: bf1b77633553bff145d17d26ae7590851131db43
      https://github.com/WebKit/WebKit/commit/bf1b77633553bff145d17d26ae7590851131db43
  Author: Aditya Keerthi <akeerthi at apple.com>
  Date:   2023-01-30 (Mon, 30 Jan 2023)

  Changed paths:
    M Source/WebCore/platform/graphics/Icon.h
    M Source/WebCore/platform/graphics/cocoa/IconCocoa.mm
    M Source/WebCore/platform/graphics/mac/IconMac.mm
    M Source/WebCore/rendering/RenderThemeMac.mm
    M Source/WebKit/Shared/Cocoa/WebIconUtilities.h
    M Source/WebKit/Shared/Cocoa/WebIconUtilities.mm
    M Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm
    M Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm
    M Source/WebKit/WebProcess/WebCoreSupport/WebChromeClientCocoa.mm
    M Source/WebKit/WebProcess/WebPage/WebPage.cpp
    M Source/WebKitLegacy/mac/WebCoreSupport/WebOpenPanelResultListener.mm

  Log Message:
  -----------
  REGRESSION (255355 at main): HTML Notes: Attachment thumbnails are low resolution
https://bugs.webkit.org/show_bug.cgi?id=251293
rdar://103533427

Reviewed by Wenson Hsieh.

In the process of supporting thumbnails for <input type=file> on macOS,
255355 at main refactored `Icon` logic to share code between Cocoa platforms.

Prior to 255355 at main, `Icon` was backed by `NSImage` on macOS, and `CGImageRef`
on other platforms. Following the change, `Icon` was backed by `CGImageRef` on
all platforms. To make this possible, `NSImage`s were converted to `CGImageRef`s
using `-[NSImage CGImageForProposedRect:context:hints]`, using a `nil` proposed
rect. This approach is problematic, as `NSImage`s are resolution-independent,
whereas `CGImageRef`s are not.

The `NSWorkspace` method used to get attachment thumbnails returns an `NSImage`
with multiple `NSImageRep`s. Before 255355 at main, WebKit would draw the `NSImage`
into a 400x400 bitmap. Now, the `NSImage` is first converted into a `CGImageRef`,
defaulting to the smallest supported representation of 32x32. That image is then
drawn into a 400x400 bitmap, resulting in low resolution.

To fix, store images in `Icon` as `NSImage`/`UIImage` and only resolve them to
`CGImageRef` when it is time to paint them. An alternate solution would be to
pass in the proposed size through multiple layers, so that
`-[NSImage CGImageForProposedRect:context:hints]` could be called with the
correct rect following the retrieval of the image from `NSWorkspace`.
However, the approach in this patch was chosen since it reduces the amount of
plumbing required, avoids conversion to `UIImage` in multiple locations in
existing iOS code, and gives clients of `Icon` more control over how they want
to present the image.

Additionally, this change removes deprecated "flippedness" logic for images.
Attachment thumbnails unnecessarily modify the "flippedness" of the image in
the UIProcess, only to undo the effect in the WebProcess. <input type=file>
thumbnails already do the right thing. It does not make sense to reintroduce the
concept of "flippedness" to `Icon` (as was the case before 255355 at main), since
it would break <input type=file> thumbnails and directory attachment thumbnails
(which have their own flipping logic, separate from files). Instead, the code
is refactored to ignore the concept of "flippedness" entirely, which AppKit
deprecated for images in macOS 10.6.

* Source/WebCore/platform/graphics/Icon.h:
(WebCore::Icon::image const):
* Source/WebCore/platform/graphics/cocoa/IconCocoa.mm:
(WebCore::Icon::Icon):
(WebCore::Icon::create):
(WebCore::Icon::paint):
(WebCore::Icon::createIconForImage): Deleted.
* Source/WebCore/platform/graphics/mac/IconMac.mm:
(WebCore::Icon::createIconForFiles):
(WebCore::Icon::createIconForFileExtension):
(WebCore::Icon::createIconForUTI):
* Source/WebCore/rendering/RenderThemeMac.mm:
(WebCore::RenderThemeMac::iconForAttachment):

Flipping the icon is no longer necessary, as the icon is drawn to a bitmap
context with a top-left origin in the UIProcess, and using a top-left origin in
the WebProcess.

(WebCore::paintAttachmentIcon):

Use the default origin, as attachment icons are no longer being flipped.

* Source/WebKit/Shared/Cocoa/WebIconUtilities.h:
* Source/WebKit/Shared/Cocoa/WebIconUtilities.mm:
(WebKit::thumbnailSizedImageForImage):
(WebKit::fallbackIconForFile):
(WebKit::iconForImageFile):
(WebKit::iconForVideoFile):
(WebKit::iconForFiles):
* Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:
(WebKit::convertPlatformImageToBitmap):

Use a simpler, equivalent AppKit method to share more code between Cocoa ports.

(WebKit::WebPageProxy::updateIconForDirectory):

Flipping the icon is no longer necessary, as the icon is drawn to a bitmap
context with a top-left origin in the UIProcess, and using a top-left origin in
the WebProcess.

* Source/WebKit/UIProcess/ios/forms/WKFileUploadPanel.mm:
(-[_WKImageFileUploadItem displayImage]):
(-[_WKVideoFileUploadItem displayImage]):
* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClientCocoa.mm:
(WebKit::WebChromeClient::createIconForFiles):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::didChooseFilesForOpenPanelWithDisplayStringAndIcon):
* Source/WebKitLegacy/mac/WebCoreSupport/WebOpenPanelResultListener.mm:
(-[WebOpenPanelResultListener chooseFilenames:displayString:iconImage:]):

Canonical link: https://commits.webkit.org/259598@main




More information about the webkit-changes mailing list