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.

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

CG display list graphics contexts are sometimes still flipped #8810

Conversation

hortont424
Copy link
Contributor

@hortont424 hortont424 commented Jan 19, 2023

9502f85

CG display list graphics contexts are sometimes still flipped
https://bugs.webkit.org/show_bug.cgi?id=250816
rdar://104388673

Reviewed by Dean Jackson.

* Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.mm:
(WebKit::GraphicsContextCGDisplayList::GraphicsContextCGDisplayList):
Remove the workaround introduced in r252402@main, since we have reinstated the toplevel
context flip in 258997@main. I missed this in 258997@main because it only affects
cases where we end up doing setCTM(getCTM()), which is just rare enough to miss
with cursory testing.

* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.h:
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:
(WebKit::RemoteLayerBackingStore::paintContents):
(WebKit::RemoteLayerBackingStore::drawInContext):
While debugging the above, I also noticed that we were leaving the display
list context with a missing trailing `restore` operation, because we encode the
display list inside drawInContext. To avoid this, move the flip inside our existing
state saver, which is popped before encoding the display list. This has no
actual effect at the moment (except making the dumped list look more correct),
but will eventually be necessary when the replayed destination context is reused.

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

178d352

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe
βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ›  gtk ❌ πŸ›  wincairo
βœ… πŸ§ͺ webkitperl ⏳ πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ gtk-wk2
⏳ πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch-sim

@hortont424 hortont424 self-assigned this Jan 19, 2023
@hortont424 hortont424 added the WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore). label Jan 19, 2023
@hortont424 hortont424 force-pushed the eng/CG-display-list-graphics-contexts-are-sometimes-still-flipped branch from 3a3af8d to 4c84f02 Compare January 19, 2023 02:15
@hortont424 hortont424 requested review from smfr and grorg January 19, 2023 02:15
@hortont424 hortont424 force-pushed the eng/CG-display-list-graphics-contexts-are-sometimes-still-flipped branch from 4c84f02 to 178d352 Compare January 19, 2023 02:17
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 19, 2023
@hortont424 hortont424 removed the merging-blocked Applied to prevent a change from being merged label Jan 19, 2023
@grorg grorg added the merge-queue Applied to send a pull request to merge-queue label Jan 19, 2023
https://bugs.webkit.org/show_bug.cgi?id=250816
rdar://104388673

Reviewed by Dean Jackson.

* Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.mm:
(WebKit::GraphicsContextCGDisplayList::GraphicsContextCGDisplayList):
Remove the workaround introduced in r252402@main, since we have reinstated the toplevel
context flip in 258997@main. I missed this in 258997@main because it only affects
cases where we end up doing setCTM(getCTM()), which is just rare enough to miss
with cursory testing.

* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.h:
* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:
(WebKit::RemoteLayerBackingStore::paintContents):
(WebKit::RemoteLayerBackingStore::drawInContext):
While debugging the above, I also noticed that we were leaving the display
list context with a missing trailing `restore` operation, because we encode the
display list inside drawInContext. To avoid this, move the flip inside our existing
state saver, which is popped before encoding the display list. This has no
actual effect at the moment (except making the dumped list look more correct),
but will eventually be necessary when the replayed destination context is reused.

Canonical link: https://commits.webkit.org/259076@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/CG-display-list-graphics-contexts-are-sometimes-still-flipped branch from 178d352 to 9502f85 Compare January 19, 2023 11:00
@webkit-commit-queue
Copy link
Collaborator

Committed 259076@main (9502f85): https://commits.webkit.org/259076@main

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

@webkit-early-warning-system webkit-early-warning-system merged commit 9502f85 into WebKit:main Jan 19, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore).
Projects
None yet
5 participants