Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Skia] Fix corner cases involving drawing shadows #28168

Merged
merged 1 commit into from
Jun 14, 2024
Merged

[Skia] Fix corner cases involving drawing shadows #28168

merged 1 commit into from
Jun 14, 2024

Conversation

Scony
Copy link
Contributor

@Scony Scony commented May 6, 2024 •
edited by webkit-early-warning-system

120f5b1

[Skia] Fix corner cases involving drawing shadows
https://bugs.webkit.org/show_bug.cgi?id=273239

Reviewed by Adrian Perez de Castro.

So far - in the ports that use skia - shadows were being drawn in the same draw call as the original drawing operation.
This was possible as shadows were basically created using a filter that can be added to every drawing operation in skia.
The problem with such apporach is, however, that it does not work properly with some composite operations.
Some composite operations such as e.g. 'destination-atop' are expected to alter drawing in a way that shadow
is being drawn in front of it's origin.
To achieve that with skia:
 - drawing of shadow must be done as a separate draw call.
 - drawing of both shadow and shadow origin must be done on separate transparency layers so that shadow, shadow origin,
   and the original content present before draw operation will overlap correctly.

* LayoutTests/fast/canvas/canvas-composite-fill-with-shadow-and-fitting-perfectly-expected.txt: Added.
* LayoutTests/fast/canvas/canvas-composite-fill-with-shadow-and-fitting-perfectly.html: Added.
* LayoutTests/fast/canvas/canvas-composite-fill-with-shadow-expected.txt: Added.
* LayoutTests/fast/canvas/canvas-composite-fill-with-shadow.html: Added.
* LayoutTests/platform/glib/TestExpectations:
* Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:
(WebCore::CanvasRenderingContext2DBase::beginCompositeLayer):
(WebCore::CanvasRenderingContext2DBase::endCompositeLayer):
(WebCore::CanvasRenderingContext2DBase::fillRect):
* Source/WebCore/platform/graphics/BifurcatedGraphicsContext.cpp:
(WebCore::BifurcatedGraphicsContext::beginTransparencyLayer):
* Source/WebCore/platform/graphics/BifurcatedGraphicsContext.h:
* Source/WebCore/platform/graphics/GraphicsContext.cpp:
(WebCore::GraphicsContext::beginTransparencyLayer):
* Source/WebCore/platform/graphics/GraphicsContext.h:
* Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:
(WebCore::GraphicsContextCG::beginTransparencyLayer):
* Source/WebCore/platform/graphics/cg/GraphicsContextCG.h:
* Source/WebCore/platform/graphics/displaylists/DisplayListItem.h:
* Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp:
(WebCore::DisplayList::BeginTransparencyLayerWithCompositeMode::apply const):
(WebCore::DisplayList::BeginTransparencyLayerWithCompositeMode::dump const):
* Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:
(WebCore::DisplayList::BeginTransparencyLayerWithCompositeMode::BeginTransparencyLayerWithCompositeMode):
(WebCore::DisplayList::BeginTransparencyLayerWithCompositeMode::compositeMode const):
* Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp:
(WebCore::DisplayList::Recorder::beginTransparencyLayer):
* Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h:
* Source/WebCore/platform/graphics/displaylists/DisplayListRecorderImpl.cpp:
(WebCore::DisplayList::RecorderImpl::recordBeginTransparencyLayer):
* Source/WebCore/platform/graphics/displaylists/DisplayListRecorderImpl.h:
* Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:
(Nicosia::CairoOperationRecorder::beginTransparencyLayer):
* Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.h:
* Source/WebCore/platform/graphics/skia/FontCascadeSkia.cpp:
(WebCore::FontCascade::drawGlyphs):
* Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:
(WebCore::GraphicsContextSkia::drawRect):
(WebCore::GraphicsContextSkia::drawNativeImageInternal):
(WebCore::GraphicsContextSkia::drawSkiaPath):
(WebCore::GraphicsContextSkia::fillPath):
(WebCore::GraphicsContextSkia::strokePath):
(WebCore::GraphicsContextSkia::createDropShadowFilterIfNeeded const):
(WebCore::GraphicsContextSkia::drawOutsetShadow):
(WebCore::GraphicsContextSkia::drawSkiaRect):
(WebCore::GraphicsContextSkia::fillRect):
(WebCore::GraphicsContextSkia::beginTransparencyLayer):
(WebCore::GraphicsContextSkia::endTransparencyLayer):
(WebCore::GraphicsContextSkia::strokeRect):
(WebCore::GraphicsContextSkia::fillRoundedRectImpl):
(WebCore::GraphicsContextSkia::drawSkiaText):
* Source/WebCore/platform/graphics/skia/GraphicsContextSkia.h:
* Source/WebCore/rendering/GlyphDisplayListCache.cpp:
(WebCore::GlyphDisplayListCache::canShareDisplayList):
* Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.cpp:
(WebKit::RemoteDisplayListRecorder::beginTransparencyLayerWithCompositeMode):
* Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.h:
* Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.messages.in:
* Source/WebKit/Scripts/webkit/messages.py:
(headers_for_type):
* Source/WebKit/Shared/DisplayListArgumentCoders.serialization.in:
* Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in:
* Source/WebKit/WebProcess/GPU/graphics/RemoteDisplayListRecorderProxy.cpp:
(WebKit::RemoteDisplayListRecorderProxy::recordBeginTransparencyLayer):
* Source/WebKit/WebProcess/GPU/graphics/RemoteDisplayListRecorderProxy.h:

Co-authored-by: Carlos Garcia Campos <[email protected]>
Canonical link: https://commits.webkit.org/280008@main

165007e

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@Scony Scony self-assigned this May 6, 2024
@Scony Scony added the WPE WebKit WebKit WPE component label May 6, 2024
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 6b77a21)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-skia
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 watch
✅ 🛠 watch-sim

@Scony
Copy link
Contributor Author

Scony commented May 6, 2024

To be continued once #27513 lands

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 6, 2024
@Scony Scony removed the merging-blocked Applied to prevent a change from being merged label May 8, 2024
@Scony Scony force-pushed the 273239-skia-fix-shadows branch from 6b77a21 to bc704b8 Compare May 8, 2024 13:27
@webkit-early-warning-system
Copy link
Collaborator

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

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-skia
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 8, 2024
@Scony Scony removed the merging-blocked Applied to prevent a change from being merged label May 9, 2024
@Scony Scony force-pushed the 273239-skia-fix-shadows branch from bc704b8 to 74e35e6 Compare May 9, 2024 09:32
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 74e35e6)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
  🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-skia
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 watch
✅ 🛠 watch-sim

@Scony Scony force-pushed the 273239-skia-fix-shadows branch from 74e35e6 to ebfd534 Compare May 9, 2024 10:49
@webkit-early-warning-system
Copy link
Collaborator

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

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-skia
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2   🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress   🧪 api-gtk
✅ 🛠 watch
✅ 🛠 watch-sim

@Scony Scony marked this pull request as ready for review May 9, 2024 10:50
@Scony
Copy link
Contributor Author

Scony commented May 9, 2024

The change in current form makes skia implementation to behave as close to CG/cairo as possible, although full alignment is not possible as CG and cairo work differently in some cases due to likely problems with spec, see whatwg/html#10338

@csaavedra
Copy link
Member

Can you rebase this? For some reason the base commit is not building in some bots.

@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 29df349)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ⏳ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

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

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ✅ 🛠 wpe ❌ 🛠 wincairo
✅ 🧪 bindings ❌ 🛠 ios-sim ❌ 🛠 mac-AS-debug   🧪 wpe-wk2 ❌ 🧪 wincairo-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ❌ 🧪 api-mac ✅ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
❌ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🛠 gtk
❌ 🛠 tv ❌ 🧪 mac-AS-debug-wk2   🧪 gtk-wk2
❌ 🛠 tv-sim ❌ 🧪 mac-wk2-stress ⏳ 🧪 api-gtk
❌ 🛠 watch
❌ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 29, 2024
@Scony Scony removed the merging-blocked Applied to prevent a change from being merged label May 29, 2024
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 910e7da)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style   🛠 ios   🛠 mac ✅ 🛠 wpe   🛠 wincairo
✅ 🧪 bindings ❌ 🛠 ios-sim ❌ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 wincairo-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2   🧪 api-mac   🧪 api-wpe
❌ 🧪 ios-wk2-wpt   🧪 mac-wk1 ✅ 🛠 wpe-cairo
❌ 🧪 api-ios   🧪 mac-wk2 ✅ 🛠 gtk
❌ 🛠 tv ❌ 🧪 mac-AS-debug-wk2   🧪 gtk-wk2
❌ 🛠 tv-sim   🧪 mac-wk2-stress ⏳ 🧪 api-gtk
  🛠 watch
❌ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

Starting EWS tests for cae4d4e. Live statuses available at the PR page, #28168

@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 69bfa2f)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ✅ 🛠 wpe ❌ 🛠 wincairo
✅ 🧪 bindings ❌ 🛠 ios-sim ❌ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ❌ 🧪 wincairo-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ❌ 🧪 api-mac ✅ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
❌ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🛠 gtk
❌ 🛠 tv ❌ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
❌ 🛠 tv-sim ❌ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
❌ 🛠 watch
❌ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 7, 2024
@Scony Scony removed the merging-blocked Applied to prevent a change from being merged label Jun 7, 2024
@webkit-early-warning-system
Copy link
Collaborator

Starting EWS tests for 18d9fdd. Live statuses available at the PR page, #28168

@webkit-early-warning-system
Copy link
Collaborator

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

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ✅ 🛠 wpe ❌ 🛠 wincairo
✅ 🧪 bindings ❌ 🛠 ios-sim ❌ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ❌ 🧪 wincairo-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ❌ 🧪 api-mac ✅ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
❌ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🛠 gtk
❌ 🛠 tv ❌ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
❌ 🛠 tv-sim ❌ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
❌ 🛠 watch
❌ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 7, 2024
@Scony Scony removed the merging-blocked Applied to prevent a change from being merged label Jun 7, 2024
@webkit-early-warning-system
Copy link
Collaborator

Starting EWS tests for 1fb2acf. Live statuses available at the PR page, #28168

@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 18d9fdd)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ✅ 🛠 wpe loading-orange 🛠 wincairo
✅ 🧪 bindings ❌ 🛠 ios-sim ❌ 🛠 mac-AS-debug loading-orange 🧪 wpe-wk2 loading-orange 🧪 wincairo-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ❌ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 webkitpy ❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
❌ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🛠 gtk
❌ 🛠 tv ❌ 🧪 mac-AS-debug-wk2 loading 🧪 gtk-wk2
❌ 🛠 tv-sim ❌ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
loading-orange 🛠 watch
loading-orange 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 7, 2024
@webkit-early-warning-system
Copy link
Collaborator

Starting EWS tests for 1fb2acf. Live statuses available at the PR page, #28168

@Scony Scony removed the merging-blocked Applied to prevent a change from being merged label Jun 11, 2024
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 01d8c19)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 1fb2acf)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ✅ 🛠 wpe ❌ 🛠 wincairo
✅ 🧪 bindings ❌ 🛠 ios-sim ❌ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ❌ 🧪 wincairo-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ❌ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 webkitpy ❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
❌ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🛠 gtk
loading 🛠 🧪 jsc-arm64 ❌ 🛠 tv ❌ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
❌ 🛠 tv-sim ❌ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
❌ 🛠 watch
❌ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

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

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 watch
✅ 🛠 watch-sim

aperezdc
aperezdc approved these changes Jun 12, 2024
View reviewed changes
Copy link
Contributor

@aperezdc aperezdc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patch looks reasonable to me, I left only a very small nit.

FWIW, good idea reusing the cross-port entry points for setting the temporary composition layer; and thanks for taking the time to write the commit log notes—they do help to understand what's the intention 🙏🏼

Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp Outdated Show resolved Hide resolved
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash 165007e)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@carlosgcampos carlosgcampos added the merge-queue Applied to send a pull request to merge-queue label Jun 14, 2024
@Scony @carlosgcampos
[Skia] Fix corner cases involving drawing shadows
120f5b1 
https://bugs.webkit.org/show_bug.cgi?id=273239

Reviewed by Adrian Perez de Castro.

So far - in the ports that use skia - shadows were being drawn in the same draw call as the original drawing operation.
This was possible as shadows were basically created using a filter that can be added to every drawing operation in skia.
The problem with such apporach is, however, that it does not work properly with some composite operations.
Some composite operations such as e.g. 'destination-atop' are expected to alter drawing in a way that shadow
is being drawn in front of it's origin.
To achieve that with skia:
 - drawing of shadow must be done as a separate draw call.
 - drawing of both shadow and shadow origin must be done on separate transparency layers so that shadow, shadow origin,
   and the original content present before draw operation will overlap correctly.

* LayoutTests/fast/canvas/canvas-composite-fill-with-shadow-and-fitting-perfectly-expected.txt: Added.
* LayoutTests/fast/canvas/canvas-composite-fill-with-shadow-and-fitting-perfectly.html: Added.
* LayoutTests/fast/canvas/canvas-composite-fill-with-shadow-expected.txt: Added.
* LayoutTests/fast/canvas/canvas-composite-fill-with-shadow.html: Added.
* LayoutTests/platform/glib/TestExpectations:
* Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:
(WebCore::CanvasRenderingContext2DBase::beginCompositeLayer):
(WebCore::CanvasRenderingContext2DBase::endCompositeLayer):
(WebCore::CanvasRenderingContext2DBase::fillRect):
* Source/WebCore/platform/graphics/BifurcatedGraphicsContext.cpp:
(WebCore::BifurcatedGraphicsContext::beginTransparencyLayer):
* Source/WebCore/platform/graphics/BifurcatedGraphicsContext.h:
* Source/WebCore/platform/graphics/GraphicsContext.cpp:
(WebCore::GraphicsContext::beginTransparencyLayer):
* Source/WebCore/platform/graphics/GraphicsContext.h:
* Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:
(WebCore::GraphicsContextCG::beginTransparencyLayer):
* Source/WebCore/platform/graphics/cg/GraphicsContextCG.h:
* Source/WebCore/platform/graphics/displaylists/DisplayListItem.h:
* Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp:
(WebCore::DisplayList::BeginTransparencyLayerWithCompositeMode::apply const):
(WebCore::DisplayList::BeginTransparencyLayerWithCompositeMode::dump const):
* Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:
(WebCore::DisplayList::BeginTransparencyLayerWithCompositeMode::BeginTransparencyLayerWithCompositeMode):
(WebCore::DisplayList::BeginTransparencyLayerWithCompositeMode::compositeMode const):
* Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp:
(WebCore::DisplayList::Recorder::beginTransparencyLayer):
* Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h:
* Source/WebCore/platform/graphics/displaylists/DisplayListRecorderImpl.cpp:
(WebCore::DisplayList::RecorderImpl::recordBeginTransparencyLayer):
* Source/WebCore/platform/graphics/displaylists/DisplayListRecorderImpl.h:
* Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:
(Nicosia::CairoOperationRecorder::beginTransparencyLayer):
* Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.h:
* Source/WebCore/platform/graphics/skia/FontCascadeSkia.cpp:
(WebCore::FontCascade::drawGlyphs):
* Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:
(WebCore::GraphicsContextSkia::drawRect):
(WebCore::GraphicsContextSkia::drawNativeImageInternal):
(WebCore::GraphicsContextSkia::drawSkiaPath):
(WebCore::GraphicsContextSkia::fillPath):
(WebCore::GraphicsContextSkia::strokePath):
(WebCore::GraphicsContextSkia::createDropShadowFilterIfNeeded const):
(WebCore::GraphicsContextSkia::drawOutsetShadow):
(WebCore::GraphicsContextSkia::drawSkiaRect):
(WebCore::GraphicsContextSkia::fillRect):
(WebCore::GraphicsContextSkia::beginTransparencyLayer):
(WebCore::GraphicsContextSkia::endTransparencyLayer):
(WebCore::GraphicsContextSkia::strokeRect):
(WebCore::GraphicsContextSkia::fillRoundedRectImpl):
(WebCore::GraphicsContextSkia::drawSkiaText):
* Source/WebCore/platform/graphics/skia/GraphicsContextSkia.h:
* Source/WebCore/rendering/GlyphDisplayListCache.cpp:
(WebCore::GlyphDisplayListCache::canShareDisplayList):
* Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.cpp:
(WebKit::RemoteDisplayListRecorder::beginTransparencyLayerWithCompositeMode):
* Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.h:
* Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.messages.in:
* Source/WebKit/Scripts/webkit/messages.py:
(headers_for_type):
* Source/WebKit/Shared/DisplayListArgumentCoders.serialization.in:
* Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in:
* Source/WebKit/WebProcess/GPU/graphics/RemoteDisplayListRecorderProxy.cpp:
(WebKit::RemoteDisplayListRecorderProxy::recordBeginTransparencyLayer):
* Source/WebKit/WebProcess/GPU/graphics/RemoteDisplayListRecorderProxy.h:

Co-authored-by: Carlos Garcia Campos <[email protected]>
Canonical link: https://commits.webkit.org/280008@main
@webkit-commit-queue
Copy link
Collaborator

Committed 280008@main (120f5b1): https://commits.webkit.org/280008@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 14, 2024
@webkit-commit-queue webkit-commit-queue merged commit 120f5b1 into WebKit:main Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlosgcampos carlosgcampos carlosgcampos left review comments

@aperezdc aperezdc aperezdc approved these changes

@alexgcastro alexgcastro Awaiting requested review from alexgcastro alexgcastro is a code owner

@nikolaszimmermann nikolaszimmermann Awaiting requested review from nikolaszimmermann nikolaszimmermann is a code owner

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

@rniwa rniwa Awaiting requested review from rniwa rniwa is a code owner

@magomez magomez Awaiting requested review from magomez magomez is a code owner

@zdobersek zdobersek Awaiting requested review from zdobersek zdobersek is a code owner

Assignees

@Scony Scony

Labels
WPE WebKit WebKit WPE component
Projects
None yet
Milestone
No milestone
7 participants
@Scony @webkit-early-warning-system @csaavedra @webkit-commit-queue @aperezdc @carlosgcampos @webkit-ews-buildbot