[Win] Add an alternative Skia backend support #36469

fujii · 2024-11-10T23:59:27Z

`5bd5f37`

[Win] Add an alternative Skia backend support
https://bugs.webkit.org/show_bug.cgi?id=269117

Reviewed by Carlos Garcia Campos.

Build out the bundled Skia. Harfbuzz is required.

> build-webkit --cmakeargs="-DUSE_SKIA=1"

There are still some problems.
* custom fonts don't work.
* pixel formats of video isn't correct
* linked font doesn't work.
* Not implemented yet: GraphicsContext::releaseWindowsContext, DragImageWin.cpp and ImageAdapterWin.cpp

* Source/ThirdParty/skia/CMakeLists.txt:
* Source/WebCore/PlatformWin.cmake:
* Source/WebCore/platform/graphics/Color.h:
* Source/WebCore/platform/graphics/FontCache.cpp:
* Source/WebCore/platform/graphics/FontCache.h:
* Source/WebCore/platform/graphics/FontCustomPlatformData.h:
* Source/WebCore/platform/graphics/FontPlatformData.h:
(WebCore::FontPlatformDataAttributes::FontPlatformDataAttributes):
(WebCore::FontPlatformData::hfont const):
* Source/WebCore/platform/graphics/skia/FontCacheSkia.cpp:
(WebCore::FontCache::platformInit):
(WebCore::FontCache::fontManager const):
* Source/WebCore/platform/graphics/skia/FontPlatformDataSkia.cpp:
(WebCore::FontPlatformData::create):
(WebCore::FontPlatformData::attributes const):
* Source/WebCore/platform/graphics/skia/ImageBufferSkiaBackend.h:
* Source/WebCore/platform/graphics/texmap/BitmapTexture.cpp:
(WebCore::BitmapTexture::updateContents):
* Source/WebCore/platform/graphics/win/FontPlatformDataWin.cpp:
(WebCore::FontPlatformData::FontPlatformData):
* Source/WebCore/platform/graphics/win/GraphicsContextWin.cpp:
(WebCore::GraphicsContext::releaseWindowsContext):
* Source/WebCore/platform/graphics/win/ImageAdapterWin.cpp:
(WebCore::ImageAdapter::getHBITMAPOfSize):
(WebCore::ImageAdapter::nativeImageOfHBITMAP):
* Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:
(WebCore::MediaPlayerPrivateMediaFoundation::Direct3DPresenter::paintCurrentFrame):
* Source/WebCore/platform/graphics/win/cairo/FontPlatformDataWinCairo.cpp:
(WebCore::FontPlatformData::FontPlatformData):
(WebCore::FontPlatformData::platformIsEqual const):
(WebCore::FontPlatformData::familyName const):
* Source/WebCore/platform/win/DragImageWin.cpp:
(WebCore::createDragImageForLink):
(WebCore::createDragImageFromImage):
(WebCore::scaleDragImage):
* Source/WebKit/PlatformWin.cmake:
* Source/WebKit/Platform/Skia.cmake:
* Source/WebKit/Shared/WebCoreArgumentCoders.h:
* Source/WebKit/Shared/WebCoreFont.serialization.in:
* Source/WebKit/Shared/skia/Skia.serialization.in: Added.
* Source/WebKit/Shared/skia/WebCoreArgumentCodersSkia.cpp:
(IPC::ArgumentCoder<SkString>::encode):
(IPC::ArgumentCoder<SkString>::decode):
(IPC::ArgumentCoder<SkFontStyle::Slant>::encode):
(IPC::ArgumentCoder<SkFontStyle::Slant>::decode):
* Source/WebKit/Shared/win/WebCoreArgumentCodersWin.cpp:
* Source/WebKit/UIProcess/BackingStore.h:
* Source/WebKit/UIProcess/skia/BackingStoreSkia.cpp:
(WebKit::BackingStore::paint):
* Source/WebKit/UIProcess/win/WebPopupMenuProxyWin.cpp:
(WebKit::WebPopupMenuProxyWin::paint):
* Source/WebKit/UIProcess/win/WebPopupMenuProxyWin.h:
* Source/WebKit/UIProcess/win/WebView.cpp:
(WebKit::WebView::paint):
* Source/cmake/OptionsWin.cmake:
* Tools/ImageDiff/PlatformWin.cmake:
* Tools/TestWebKitAPI/Tests/WebCore/win/LinkedFonts.cpp:
* Tools/WebKitTestRunner/PlatformWin.cmake:
* Tools/WebKitTestRunner/win/PlatformWebViewWin.cpp:
(WTR::generateCairoSurfaceFromBitmap):
(WTR::PlatformWebView::windowSnapshotImage):

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

fc99e13

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 vision	~~🧪 mac-AS-debug-wk2~~	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	~~🧪 mac-intel-wk2~~	✅ 🛠 jsc-armv7
✅ 🛠 🧪 unsafe-merge	✅ 🛠 tv	✅ 🛠 mac-safer-cpp	✅ 🧪 jsc-armv7-tests
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-11-10T23:59:34Z

EWS run on previous version of this PR (hash 12c1ba4)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	❌ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 vision	~~🧪 mac-AS-debug-wk2~~	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 jsc-armv7
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp	✅ 🧪 jsc-armv7-tests
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-11-11T01:48:12Z

EWS run on previous version of this PR (hash f5109d6)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 jsc-armv7
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp	✅ 🧪 jsc-armv7-tests
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

donny-dont

Overall this is fine in terms of the build but have one large issue with it

We should not be hand rolling any serialization code and land #27902 . Can you check to see if it works for you?

Other thing is do we really need harfbuzz? If we can avoid adding it in that would be ideal.

webkit-early-warning-system · 2024-11-14T07:45:33Z

EWS run on previous version of this PR (hash 92d7c1c)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 jsc-armv7
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp	✅ 🧪 jsc-armv7-tests
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

Source/ThirdParty/skia/CMakeLists.txt

carlosgcampos · 2024-11-14T08:01:49Z

Source/WebCore/platform/graphics/texmap/BitmapTexture.cpp

Is this SkImage always unaccelerated?

Yes, at the moment.

WebKit/Source/WebCore/platform/graphics/texmap/BitmapTexture.cpp

Lines 221 to 223 in 1b0ca73

// Making an unconditionally unaccelerated buffer here is OK because this code

// isn't used by any platforms that respect the accelerated bit.

auto imageBuffer = ImageBuffer::create(targetRect.size(), RenderingPurpose::Unspecified, 1, DestinationColorSpace::SRGB(), ImageBufferPixelFormat::BGRA8);

ImageBufferOptions::Accelerated has to be specified.

carlosgcampos · 2024-11-14T08:09:47Z

Source/WebKit/Shared/skia/Skia.serialization.in

Can't we add this to WebCoreFont with a skia ifdef? I'm not sure we want a generic Skia file for serializations, I think it's better to have different files for different things.

I think all skia types SkString, SkFontStyle::Slant, SkColorSpace, SkData can be moved to this Skia.serialization.in.

achristensen07 · 2024-11-14T20:03:43Z

Source/WebKit/Shared/skia/WebCoreArgumentCodersSkia.cpp

Please use serialization.in files instead of this.

We need more documents to migrate. It took hours to me to find out the undocumented "wrapped by" keyword.

wrapped by is only for ObjC type. I can't use it for Skia types now. Needs to modify generate-serializers.py. Will try in a follow up patch.

webkit-early-warning-system · 2024-11-15T05:06:24Z

EWS run on current version of this PR (hash fc99e13)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 vision	~~🧪 mac-AS-debug-wk2~~	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	~~🧪 mac-intel-wk2~~	✅ 🛠 jsc-armv7
✅ 🛠 🧪 unsafe-merge	✅ 🛠 tv	✅ 🛠 mac-safer-cpp	✅ 🧪 jsc-armv7-tests
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

https://bugs.webkit.org/show_bug.cgi?id=269117 Reviewed by Carlos Garcia Campos. Build out the bundled Skia. Harfbuzz is required. > build-webkit --cmakeargs="-DUSE_SKIA=1" There are still some problems. * custom fonts don't work. * pixel formats of video isn't correct * linked font doesn't work. * Not implemented yet: GraphicsContext::releaseWindowsContext, DragImageWin.cpp and ImageAdapterWin.cpp * Source/ThirdParty/skia/CMakeLists.txt: * Source/WebCore/PlatformWin.cmake: * Source/WebCore/platform/graphics/Color.h: * Source/WebCore/platform/graphics/FontCache.cpp: * Source/WebCore/platform/graphics/FontCache.h: * Source/WebCore/platform/graphics/FontCustomPlatformData.h: * Source/WebCore/platform/graphics/FontPlatformData.h: (WebCore::FontPlatformDataAttributes::FontPlatformDataAttributes): (WebCore::FontPlatformData::hfont const): * Source/WebCore/platform/graphics/skia/FontCacheSkia.cpp: (WebCore::FontCache::platformInit): (WebCore::FontCache::fontManager const): * Source/WebCore/platform/graphics/skia/FontPlatformDataSkia.cpp: (WebCore::FontPlatformData::create): (WebCore::FontPlatformData::attributes const): * Source/WebCore/platform/graphics/skia/ImageBufferSkiaBackend.h: * Source/WebCore/platform/graphics/texmap/BitmapTexture.cpp: (WebCore::BitmapTexture::updateContents): * Source/WebCore/platform/graphics/win/FontPlatformDataWin.cpp: (WebCore::FontPlatformData::FontPlatformData): * Source/WebCore/platform/graphics/win/GraphicsContextWin.cpp: (WebCore::GraphicsContext::releaseWindowsContext): * Source/WebCore/platform/graphics/win/ImageAdapterWin.cpp: (WebCore::ImageAdapter::getHBITMAPOfSize): (WebCore::ImageAdapter::nativeImageOfHBITMAP): * Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp: (WebCore::MediaPlayerPrivateMediaFoundation::Direct3DPresenter::paintCurrentFrame): * Source/WebCore/platform/graphics/win/cairo/FontPlatformDataWinCairo.cpp: (WebCore::FontPlatformData::FontPlatformData): (WebCore::FontPlatformData::platformIsEqual const): (WebCore::FontPlatformData::familyName const): * Source/WebCore/platform/win/DragImageWin.cpp: (WebCore::createDragImageForLink): (WebCore::createDragImageFromImage): (WebCore::scaleDragImage): * Source/WebKit/PlatformWin.cmake: * Source/WebKit/Platform/Skia.cmake: * Source/WebKit/Shared/WebCoreArgumentCoders.h: * Source/WebKit/Shared/WebCoreFont.serialization.in: * Source/WebKit/Shared/skia/Skia.serialization.in: Added. * Source/WebKit/Shared/skia/WebCoreArgumentCodersSkia.cpp: (IPC::ArgumentCoder<SkString>::encode): (IPC::ArgumentCoder<SkString>::decode): (IPC::ArgumentCoder<SkFontStyle::Slant>::encode): (IPC::ArgumentCoder<SkFontStyle::Slant>::decode): * Source/WebKit/Shared/win/WebCoreArgumentCodersWin.cpp: * Source/WebKit/UIProcess/BackingStore.h: * Source/WebKit/UIProcess/skia/BackingStoreSkia.cpp: (WebKit::BackingStore::paint): * Source/WebKit/UIProcess/win/WebPopupMenuProxyWin.cpp: (WebKit::WebPopupMenuProxyWin::paint): * Source/WebKit/UIProcess/win/WebPopupMenuProxyWin.h: * Source/WebKit/UIProcess/win/WebView.cpp: (WebKit::WebView::paint): * Source/cmake/OptionsWin.cmake: * Tools/ImageDiff/PlatformWin.cmake: * Tools/TestWebKitAPI/Tests/WebCore/win/LinkedFonts.cpp: * Tools/WebKitTestRunner/PlatformWin.cmake: * Tools/WebKitTestRunner/win/PlatformWebViewWin.cpp: (WTR::generateCairoSurfaceFromBitmap): (WTR::PlatformWebView::windowSnapshotImage): Canonical link: https://commits.webkit.org/286632@main

webkit-commit-queue · 2024-11-15T07:50:01Z

Committed 286632@main (5bd5f37): https://commits.webkit.org/286632@main

Reviewed commits have been landed. Closing PR #36469 and removing active labels.

fujii requested review from JonWBedard, alexgcastro, carlosgcampos, cdumez, magomez, nikolaszimmermann and zdobersek as code owners November 10, 2024 23:59

fujii self-assigned this Nov 10, 2024

fujii added the CMake Bugzilla component for CMake build system changes label Nov 10, 2024

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 11, 2024

fujii removed the merging-blocked Applied to prevent a change from being merged label Nov 11, 2024

fujii force-pushed the skia-win branch from 12c1ba4 to f5109d6 Compare November 11, 2024 01:48

fujii requested review from donny-dont and rkirsling November 11, 2024 21:53

donny-dont reviewed Nov 12, 2024

View reviewed changes

fujii force-pushed the skia-win branch from f5109d6 to 92d7c1c Compare November 14, 2024 07:45

carlosgcampos approved these changes Nov 14, 2024

View reviewed changes

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 14, 2024

achristensen07 requested changes Nov 14, 2024

View reviewed changes

fujii removed the merging-blocked Applied to prevent a change from being merged label Nov 15, 2024

fujii force-pushed the skia-win branch from 92d7c1c to fc99e13 Compare November 15, 2024 05:06

fujii added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Nov 15, 2024

webkit-commit-queue force-pushed the skia-win branch from fc99e13 to 5bd5f37 Compare November 15, 2024 07:49

webkit-commit-queue merged commit 5bd5f37 into WebKit:main Nov 15, 2024

webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Nov 15, 2024

	// Making an unconditionally unaccelerated buffer here is OK because this code
	// isn't used by any platforms that respect the accelerated bit.
	auto imageBuffer = ImageBuffer::create(targetRect.size(), RenderingPurpose::Unspecified, 1, DestinationColorSpace::SRGB(), ImageBufferPixelFormat::BGRA8);

[Win] Add an alternative Skia backend support #36469

[Win] Add an alternative Skia backend support #36469

Uh oh!

Conversation

fujii commented Nov 10, 2024 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

5bd5f37

Uh oh!

webkit-early-warning-system commented Nov 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

donny-dont left a comment

Choose a reason for hiding this comment

Uh oh!

webkit-early-warning-system commented Nov 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

carlosgcampos Nov 14, 2024

Choose a reason for hiding this comment

Uh oh!

fujii Nov 15, 2024

Choose a reason for hiding this comment

Uh oh!

carlosgcampos Nov 14, 2024

Choose a reason for hiding this comment

Uh oh!

fujii Nov 15, 2024

Choose a reason for hiding this comment

Uh oh!

achristensen07 Nov 14, 2024

Choose a reason for hiding this comment

Uh oh!

fujii Nov 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fujii Nov 15, 2024

Choose a reason for hiding this comment

Uh oh!

webkit-early-warning-system commented Nov 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-commit-queue commented Nov 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

fujii commented Nov 10, 2024 •

edited by webkit-early-warning-system

Loading

`5bd5f37`

webkit-early-warning-system commented Nov 10, 2024 •

edited

Loading

webkit-early-warning-system commented Nov 11, 2024 •

edited

Loading

webkit-early-warning-system commented Nov 14, 2024 •

edited

Loading

fujii Nov 15, 2024 •

edited

Loading

webkit-early-warning-system commented Nov 15, 2024 •

edited

Loading