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

Canvas does not show results of CanvasRenderingContext2D.putImageData until forced re-render. #16157

Merged
merged 1 commit into from
Jul 28, 2023
Merged

Canvas does not show results of CanvasRenderingContext2D.putImageData until forced re-render. #16157

merged 1 commit into from
Jul 28, 2023

Conversation

grorg
Copy link
Contributor

@grorg grorg commented Jul 28, 2023 •
edited by webkit-early-warning-system
Loading

fdd0bc1

Canvas does not show results of CanvasRenderingContext2D.putImageData until forced re-render.
https://bugs.webkit.org/show_bug.cgi?id=259584
rdar://112901862

Reviewed by Tim Nguyen.

The commit in 263694@main caused a regression when calculating the dirty region
of a putImageDataUpdate. It accidentally removed a reference marker on a parameter.

* Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:
(WebCore::computeImageDataRect): Make sure the destRect parameter is a reference because it is updated.
(WebCore::CanvasRenderingContext2DBase::putImageData): Leave a fixme!

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

897573f

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 ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim

@grorg grorg requested review from cdumez and rniwa as code owners July 28, 2023 02:01
@grorg grorg self-assigned this Jul 28, 2023
@grorg grorg added the Canvas Bugs related to the canvas element. label Jul 28, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 28, 2023 •
edited
Loading

EWS run on current version of this PR (hash 897573f)

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 ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim

@grorg grorg requested review from smfr and sysrqb July 28, 2023 02:07
@grorg
Copy link
Contributor Author

grorg commented Jul 28, 2023

I'm adding a test.

@grorg
Copy link
Contributor Author

grorg commented Jul 28, 2023

Alas, reftests cause a repaint so can't be used as a test. And a repaint test seems to suggest the entire canvas is repainted. So I'm not sure how to test this, sorry.

@grorg grorg added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jul 28, 2023
@grorg
Canvas does not show results of CanvasRenderingContext2D.putImageData…
fdd0bc1 
… until forced re-render.

https://bugs.webkit.org/show_bug.cgi?id=259584
rdar://112901862

Reviewed by Tim Nguyen.

The commit in 263694@main caused a regression when calculating the dirty region
of a putImageDataUpdate. It accidentally removed a reference marker on a parameter.

* Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:
(WebCore::computeImageDataRect): Make sure the destRect parameter is a reference because it is updated.
(WebCore::CanvasRenderingContext2DBase::putImageData): Leave a fixme!

Canonical link: https://commits.webkit.org/266397@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Canvas-does-not-show-results-of-CanvasRenderingContext2D-putImageData-until-forced-re-render- branch from 897573f to fdd0bc1 Compare July 28, 2023 19:53
@webkit-commit-queue
Copy link
Collaborator

Committed 266397@main (fdd0bc1): https://commits.webkit.org/266397@main

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

@webkit-commit-queue webkit-commit-queue merged commit fdd0bc1 into WebKit:main Jul 28, 2023
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jul 28, 2023
@grorg grorg deleted the eng/Canvas-does-not-show-results-of-CanvasRenderingContext2D-putImageData-until-forced-re-render- branch July 28, 2023 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nt1m nt1m nt1m approved these changes

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

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

@smfr smfr Awaiting requested review from smfr

@sysrqb sysrqb Awaiting requested review from sysrqb

Assignees

@grorg grorg

Labels
Canvas Bugs related to the canvas element.
Projects
None yet
Milestone
No milestone
4 participants
@grorg @webkit-early-warning-system @webkit-commit-queue @nt1m