| CARVIEW |
Select Language
HTTP/2 200
date: Sat, 27 Dec 2025 06:47:24 GMT
content-type: text/html; charset=utf-8
cache-control: no-cache
content-security-policy: default-src 'none'; base-uri 'self'; child-src github.githubassets.com github.com/assets-cdn/worker/ github.com/assets/ gist.github.com/assets-cdn/worker/; connect-src 'self' uploads.github.com www.githubstatus.com collector.github.com raw.githubusercontent.com api.github.com github-cloud.s3.amazonaws.com github-production-repository-file-5c1aeb.s3.amazonaws.com github-production-upload-manifest-file-7fdce7.s3.amazonaws.com github-production-user-asset-6210df.s3.amazonaws.com *.rel.tunnels.api.visualstudio.com wss://*.rel.tunnels.api.visualstudio.com github.githubassets.com objects-origin.githubusercontent.com copilot-proxy.githubusercontent.com proxy.individual.githubcopilot.com proxy.business.githubcopilot.com proxy.enterprise.githubcopilot.com *.actions.githubusercontent.com wss://*.actions.githubusercontent.com productionresultssa0.blob.core.windows.net/ productionresultssa1.blob.core.windows.net/ productionresultssa2.blob.core.windows.net/ productionresultssa3.blob.core.windows.net/ productionresultssa4.blob.core.windows.net/ productionresultssa5.blob.core.windows.net/ productionresultssa6.blob.core.windows.net/ productionresultssa7.blob.core.windows.net/ productionresultssa8.blob.core.windows.net/ productionresultssa9.blob.core.windows.net/ productionresultssa10.blob.core.windows.net/ productionresultssa11.blob.core.windows.net/ productionresultssa12.blob.core.windows.net/ productionresultssa13.blob.core.windows.net/ productionresultssa14.blob.core.windows.net/ productionresultssa15.blob.core.windows.net/ productionresultssa16.blob.core.windows.net/ productionresultssa17.blob.core.windows.net/ productionresultssa18.blob.core.windows.net/ productionresultssa19.blob.core.windows.net/ github-production-repository-image-32fea6.s3.amazonaws.com github-production-release-asset-2e65be.s3.amazonaws.com insights.github.com wss://alive.github.com wss://alive-staging.github.com api.githubcopilot.com api.individual.githubcopilot.com api.business.githubcopilot.com api.enterprise.githubcopilot.com; font-src github.githubassets.com; form-action 'self' github.com gist.github.com copilot-workspace.githubnext.com objects-origin.githubusercontent.com; frame-ancestors 'none'; frame-src viewscreen.githubusercontent.com notebooks.githubusercontent.com; img-src 'self' data: blob: github.githubassets.com media.githubusercontent.com camo.githubusercontent.com identicons.github.com avatars.githubusercontent.com private-avatars.githubusercontent.com github-cloud.s3.amazonaws.com objects.githubusercontent.com release-assets.githubusercontent.com secured-user-images.githubusercontent.com/ user-images.githubusercontent.com/ private-user-images.githubusercontent.com opengraph.githubassets.com marketplace-screenshots.githubusercontent.com/ copilotprodattachments.blob.core.windows.net/github-production-copilot-attachments/ github-production-user-asset-6210df.s3.amazonaws.com customer-stories-feed.github.com spotlights-feed.github.com objects-origin.githubusercontent.com *.githubusercontent.com; manifest-src 'self'; media-src github.com user-images.githubusercontent.com/ secured-user-images.githubusercontent.com/ private-user-images.githubusercontent.com github-production-user-asset-6210df.s3.amazonaws.com gist.github.com github.githubassets.com; script-src github.githubassets.com; style-src 'unsafe-inline' github.githubassets.com; upgrade-insecure-requests; worker-src github.githubassets.com github.com/assets-cdn/worker/ github.com/assets/ gist.github.com/assets-cdn/worker/
referrer-policy: no-referrer-when-downgrade
server-timing: pull_request_layout-fragment;desc="pull_request_layout fragment";dur=280.685274,conversation_content-fragment;desc="conversation_content fragment";dur=1755.442954,conversation_sidebar-fragment;desc="conversation_sidebar fragment";dur=326.019617,nginx;desc="NGINX";dur=0.594703,glb;desc="GLB";dur=96.745779
strict-transport-security: max-age=31536000; includeSubdomains; preload
vary: X-PJAX, X-PJAX-Container, Turbo-Visit, Turbo-Frame, X-Requested-With,Accept-Encoding, Accept, X-Requested-With
x-content-type-options: nosniff
x-frame-options: deny
x-voltron-version: aab62e3
x-xss-protection: 0
server: github.com
content-encoding: gzip
accept-ranges: bytes
set-cookie: _gh_sess=CBbRT0vVbxumf7juxdEIz9Mw7F3rG%2Bwp75uHZgAWpk00HXuFzJqOZ0NoMQDfU8SjHWvfGxwG35TnDds%2FT2wlJb5zH3p%2BgxGmschm19cRla%2BODTX6qnBeqhiN8MRosPuhL0y4gHgk%2BSKMvTdkIfCp1qYM3qJ0LnR3IfbIVcSmyvCNefwu1dI19s1ypWA2MgPPvwN7zzy60uP3mUJ47fZBerw991bTVVE%2FPoOcFgMw2%2F0U7YWLGjhb438FM%2BEI9tZEJAS7vvpP%2BLbTwDDF%2F6mL2w%3D%3D--SLqGR7zYEek9KWyb--SzOzowcmrnSOV3PgRg0ThA%3D%3D; Path=/; HttpOnly; Secure; SameSite=Lax
set-cookie: _octo=GH1.1.748435000.1766818042; Path=/; Domain=github.com; Expires=Sun, 27 Dec 2026 06:47:22 GMT; Secure; SameSite=Lax
set-cookie: logged_in=no; Path=/; Domain=github.com; Expires=Sun, 27 Dec 2026 06:47:22 GMT; HttpOnly; Secure; SameSite=Lax
x-github-request-id: B8BA:211B61:4004587:4D2384E:694F80FA
pixels: Store decoded images as RGBA premultiplied data by mrobinson · Pull Request #40158 · servo/servo · GitHub
Copy link
Copy link
Copy link
mrobinson
force-pushed
the
rgba-premultiplied
branch
from
October 25, 2025 09:54
Copy link
Copy link
Copy link
Copy link
mrobinson
force-pushed
the
rgba-premultiplied
branch
from
October 26, 2025 16:33
servo-highfive
added
the
S-awaiting-review
There is new code that needs to be reviewed.
label
Oct 26, 2025
Copy link
Copy link
Copy link
Copy link
Copy link
mrobinson
force-pushed
the
rgba-premultiplied
branch
from
October 29, 2025 08:03
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
Copy link
servo-highfive
removed
the
S-awaiting-review
There is new code that needs to be reviewed.
label
Oct 30, 2025
Copy link
Copy link
mrobinson
force-pushed
the
rgba-premultiplied
branch
from
October 31, 2025 09:02
servo-highfive
added
the
S-awaiting-review
There is new code that needs to be reviewed.
label
Oct 31, 2025
servo-highfive
added
the
S-awaiting-merge
The PR is in the process of compiling and running tests on the automated CI.
label
Oct 31, 2025
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Oct 31, 2025
servo-highfive
added
S-tests-failed
The changes caused existing tests to fail.
and removed
S-awaiting-merge
The PR is in the process of compiling and running tests on the automated CI.
labels
Oct 31, 2025
mrobinson
force-pushed
the
rgba-premultiplied
branch
from
October 31, 2025 10:01
servo-highfive
removed
the
S-tests-failed
The changes caused existing tests to fail.
label
Oct 31, 2025
servo-highfive
added
the
S-awaiting-merge
The PR is in the process of compiling and running tests on the automated CI.
label
Oct 31, 2025
servo-highfive
removed
the
S-awaiting-merge
The PR is in the process of compiling and running tests on the automated CI.
label
Oct 31, 2025
Skip to content
Navigation Menu
{{ message }}
-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
pixels: Store decoded images as RGBA premultiplied data #40158
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
🔨 Triggering try run (#18800532404) for Linux (WPT) |
|
Test results for linux-wpt from try job (#18800532404): Flaky unexpected result (20)
Stable unexpected results that are known to be intermittent (23)
Stable unexpected results (74)
|
|
|
mrobinson
force-pushed
the
rgba-premultiplied
branch
from
ff1b015 to
70389a1
Compare
|
🔨 Triggering try run (#18801523051) for Linux (WPT) |
|
Test results for linux-wpt from try job (#18801523051): Flaky unexpected result (22)
Stable unexpected results that are known to be intermittent (18)
Stable unexpected results (10)
|
|
|
Member
Author
|
I added a note in the PR description about the two new WebGL failures. Summary: I think this change is important even though it causes a pretty minor regression in the WebGL comformance suite. |
mrobinson
force-pushed
the
rgba-premultiplied
branch
from
70389a1 to
0e78411
Compare
servo-highfive
added
the
S-awaiting-review
Member
I think that's reasonable, but we should document this (losing correctness for performance) in places were we are currently premultiplying and open a new issue about it (also link it in the comments). Do we have any numbers of performance improvement? |
sagudev
reviewed
Oct 26, 2025
Member
Author
I can try to get some numbers tomorrow. Performance number might be hard to get (and I suspect that other things are dominating as performance is still very bad for canvas), but I can do some simple counting of the conversions that we avoid in Servo. |
tharkum
reviewed
Oct 27, 2025
| // to the source rectangle with formatting. | ||
| let snapshot = raster_image.as_snapshot(); | ||
| let Some(bitmap_data) = | ||
| ImageBitmap::crop_and_transform_bitmap_data(snapshot, sx, sy, sw, sh, options) |
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ImageBitmap::crop_and_transform_bitmap_data(raster_image.as_snapshot(), ....) ?
Member
Author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could do this though it adds a line of code for every argument of crop_and_transform_bitmap_data due to rustfmt. Normally I'm happy to do this, but this function takes 6 arguments.
Member
Author
I've gone ahead and added a comment to the place where we convert image data to premultiplied data with a link to #40257.
I ran https://cookieclicker.eu/cookieclicker/ before and after this change, placing a print statement right before Before: Unfortunately, Cookie Clicker is still slow with my change, but I think this is a big step on the way toward fixing it. |
mrobinson
force-pushed
the
rgba-premultiplied
branch
from
0e78411 to
6e54ff4
Compare
Member
Author
|
@sagudev I think I've responded to all of your comments. Thanks for the review. PTAL. |
|
🔨 Triggering try run (#18901069038) for Linux (WPT) |
|
Test results for linux-wpt from try job (#18901069038): Flaky unexpected result (20)
Stable unexpected results that are known to be intermittent (14)
|
|
✨ Try run (#18901069038) succeeded. |
sagudev
approved these changes
Oct 30, 2025
Comment on lines
+402
to
+405
| bytes.extend_from_slice(&[rgb[2], rgb[1], rgb[0], 0xff]); | ||
| } | ||
| ( | ||
| WebRenderImageFormat::BGRA8, |
Member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-blocking: Given that we started preferring RBGA everywhere else, should we use it here too?
Member
Author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I was thinking that this might be a good idea, but was planning on doing it in a followup in order to minimize the changes in one PR.
Comment on lines
+293
to
+294
| /// Whether or not all of the frames of this image are opaque. | ||
| pub is_opaque: bool, |
Member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
though(non-blocking): Maybe we should make this per frame? Although I am not sure how much that would matter in practice.
Member
Author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually a bit tricky, because the value is per ImageKey in WebRender and we share an ImageKey among all frames of an animated image. As many gifs are looping, perhaps the potential performance isn't so important ATM.
Member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually a bit tricky, because the value is per
ImageKeyin WebRender and we share anImageKeyamong all frames of an animated image. As many gifs are looping, perhaps the potential performance isn't so important ATM.
IIRC Every time you update image you also provide the flags (if opaque) so it's per image rather then per ImageKey. But I agree that this might not be really important or worth the complexity.
servo-highfive
removed
the
S-awaiting-review
mrobinson
commented
Oct 31, 2025
Member
Author
mrobinson
left a comment
mrobinson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @sagudev!
Comment on lines
+402
to
+405
| bytes.extend_from_slice(&[rgb[2], rgb[1], rgb[0], 0xff]); | ||
| } | ||
| ( | ||
| WebRenderImageFormat::BGRA8, |
Member
Author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I was thinking that this might be a good idea, but was planning on doing it in a followup in order to minimize the changes in one PR.
mrobinson
force-pushed
the
rgba-premultiplied
branch
from
6e54ff4 to
0ae2fa4
Compare
servo-highfive
added
the
S-awaiting-review
servo-highfive
added
the
S-awaiting-merge
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
servo-highfive
added
S-tests-failed
Canvas expects input data to be in RGBA premultiplied format and WebRender already supports RGBA and BGRA data as long as they are premultiplied. Pre-multiplying up front allows: - Avoiding many conversion when painting images to canvas. - Passing the `RasterImage` IpcSharedMemory of the image instead of creating a new one with the premultiplied data every time we upload to WebRender. This is a big deal for animated gifs, because before every frame was creating a new shared memory segment. It seems that for rasterized SVGs were were already putting premultplied data into the `RasterImage` so it's quite likely SVGs were being composited incorrectly. Signed-off-by: Martin Robinson <mrobinson@igalia.com>
mrobinson
force-pushed
the
rgba-premultiplied
branch
from
0ae2fa4 to
b95822e
Compare
servo-highfive
removed
the
S-tests-failed
servo-highfive
added
the
S-awaiting-merge
servo-highfive
removed
the
S-awaiting-merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
You can’t perform that action at this time.
Canvas expects input data to be in RGBA premultiplied format and
WebRender already supports RGBA and BGRA data as long as they are
premultiplied. Pre-multiplying up front allows:
RasterImageIpcSharedMemory of the image instead of creatinga new one with the premultiplied data every time we upload to
WebRender. This is a big deal for animated gifs, because before every
frame was creating a new shared memory segment.
It seems that for rasterized SVGs were were already putting
premultplied data into the
RasterImageso it's quite likely SVGs werebeing composited incorrectly.
Testing: This causes 8 tests to start passing and 2 tests to fail in the WebGL conformance suite.
The failures are due to the fact that premultiplying alpha is lossy when alpha is 0. In that case,
the resulting color of a blend operation might be wrong. This is typically only a problem if you
use RGBA data as RGB data, which is pretty unusual. In the case that you are blending with
RGBA the final color values will be 0 or close to 0 anyway. Gecko solves this issue by having a
cacheable surface generation API that can fetch both premulitplied and unpremulitplied data
from things like image elements. We do not have that yet, but I argue that this change
is important anyway due to the amount that it reduces memory and file descriptor usage
as well as the cost of copying image data so much in memory.