servo: android hardware rendering and refactor #10201

burhankhanzada · 2025-12-04T12:56:36Z

Extend vulkan implementation to support android
Add cpu fallback non optimized path in vulkan for android emulator
Refactor vulkan to its own module
Introduce safegaurd for surface to auto unbind and rebind when drop
Add docs
Improve metal erros handling

…push constants

…d update build dependencies.

…ew safe helper function.

…disable push constants" This reverts commit 57ac9c7.

expenses · 2025-12-04T14:46:39Z

examples/servo/src/webview/rendering_context/utils.rs

+/// Flips an image buffer vertically in place.
+///
+/// This is used to correct the orientation of textures read from OpenGL, which uses a
+/// bottom-left origin, whereas other APIs (like WGPU/Metal/Vulkan) typically use top-left.
+pub fn flip_image_vertically(
+    pixels: &mut [u8],
+    width: usize,
+    height: usize,
+    bytes_per_pixel: usize,
+) {
+    let stride = width * bytes_per_pixel;
+    let mut row_buffer = vec![0u8; stride];
+    for y in 0..height / 2 {
+        let top_row_start = y * stride;
+        let bottom_row_start = (height - y - 1) * stride;
+
+        // Swap rows
+        row_buffer.copy_from_slice(&pixels[top_row_start..top_row_start + stride]);
+        pixels.copy_within(bottom_row_start..bottom_row_start + stride, top_row_start);
+        pixels[bottom_row_start..bottom_row_start + stride].copy_from_slice(&row_buffer);
+    }
+}


This seems like something that should be happening on the gpu instead

Hm yeah i agree but its duplicated code used in surfman_rendering_context and vulkcan cpu path thats why i exracted it.

Why is this fallback needed?

There should be three code paths:

Rendering with servo::SoftwareRenderingContext, which should not require flipping an image around (an OpenGL "feature").

Rendering with OpenGL conversion to Vulkan texture, where the call to blit_named_framebuffer will do the y-flip on the GPU.

Rendering with OpenGL and conversion to Metal Texture, where create_flipped_texture_render() takes care of the flipping on the GPU.

expenses · 2025-12-04T14:47:47Z

examples/servo/Cargo.toml

-embedder_traits = { git = "https://github.com/servo/servo", rev = "158bf97e", features = ["baked-default-resources"] }
+libservo = { git = "https://github.com/servo/servo", rev = "39d3fc6c" }
+embedder_traits = { git = "https://github.com/servo/servo", rev = "39d3fc6c", features = ["baked-default-resources"] }
+# not used in code but added for no need to set specific rev whenever run cargo update or freash install and without this its not building


This comment isn't very clear to me

Ah its not related to vulkan or hardware rendering any way its just whenever i update servo revision to new version and run cargo update it update its internal dependencies to new version which conflict sometimes and making build to failed that why i added it here so all servo dependencies use this specific version.

expenses · 2025-12-04T14:48:08Z

examples/servo/Cargo.toml

-slint-build = { path = "../../api/rs/build" }
-
-[target.'cfg(target_os = "linux")'.build-dependencies]
+# need to add to for all host to generate gl_bindings for android


Ah this need to build gl_binding so that android build have gl_bindings even on mac as host

expenses · 2025-12-04T14:51:20Z

examples/servo/src/webview/rendering_context/vulkan/vulkan.rs

+
+pub struct WPGPUTextureFromVulkan<'a> {
+    context: &'a GPURenderingContext,
+}


It's a pet peeve but I'm really not a fan of these (AI generated?) abstraction structs that don't really do a lot except obscure the code more than needed. It'd be great if you could refactor these into pure functions.

Well its true mostly AI did this but i make did this intentionally to pass as less parametrs as possible to functions. But if you and @tronical think its bad approach then will happy to refactor as funntions. Also @expenses can you please review gl related code coz i try to understand but didn't get much its your expertize, i just modify to work it with android if your some suggestion there or way to optimize or reduce boilerplate.

I agree with @expenses . I'll go one step further: This PR moves a lot of code around and it contains crucial changes to make certain scenarios work (including the emulator detection). The fact that it's all one big diff makes it very hard to review, and the individual commits are also mixing things.

For example, commit ebabf4f appears to contain two different changes that should be separate commits: (1) It switches the GL profile to GLES for Android (makes sense, I suppose this is what fixes the rendering with GL on the phone) and (2) adds emulator detection.
But I'm guessing here, as it's mixed - and it's better if the reviewers don't have to guess :)

So: It's very cool that you've gotten this to work. That's excellent, in fact. The next logical step is IMO to turn the individual things that you've done (fixing GLES profile, fixing emulator detection, moving code around to clean it up, adding insets) into either separate PRs or clean separate commits. As the PR stands, it has unrelated changes to the wgpu_texture example that are reverted later. That means the PR can only be squashed, which in turn destroys the ability to look at the changes individual and try to understand then when 9 months down the line we're looking at the code to debug something.

This is the next stage of development.

Ah OK i understand will do this in new more small pull requestss so its easy to review and reason about

tronical · 2025-12-05T10:15:44Z

examples/servo/src/webview/rendering_context/vulkan/vulkan.rs

+
+pub struct WPGPUTextureFromVulkan<'a> {
+    context: &'a GPURenderingContext,
+}


I agree with @expenses . I'll go one step further: This PR moves a lot of code around and it contains crucial changes to make certain scenarios work (including the emulator detection). The fact that it's all one big diff makes it very hard to review, and the individual commits are also mixing things.

For example, commit ebabf4f appears to contain two different changes that should be separate commits: (1) It switches the GL profile to GLES for Android (makes sense, I suppose this is what fixes the rendering with GL on the phone) and (2) adds emulator detection.
But I'm guessing here, as it's mixed - and it's better if the reviewers don't have to guess :)

So: It's very cool that you've gotten this to work. That's excellent, in fact. The next logical step is IMO to turn the individual things that you've done (fixing GLES profile, fixing emulator detection, moving code around to clean it up, adding insets) into either separate PRs or clean separate commits. As the PR stands, it has unrelated changes to the wgpu_texture example that are reverted later. That means the PR can only be squashed, which in turn destroys the ability to look at the changes individual and try to understand then when 9 months down the line we're looking at the code to debug something.

This is the next stage of development.

tronical · 2025-12-05T10:21:19Z

examples/servo/src/webview/rendering_context/utils.rs

+/// Flips an image buffer vertically in place.
+///
+/// This is used to correct the orientation of textures read from OpenGL, which uses a
+/// bottom-left origin, whereas other APIs (like WGPU/Metal/Vulkan) typically use top-left.
+pub fn flip_image_vertically(
+    pixels: &mut [u8],
+    width: usize,
+    height: usize,
+    bytes_per_pixel: usize,
+) {
+    let stride = width * bytes_per_pixel;
+    let mut row_buffer = vec![0u8; stride];
+    for y in 0..height / 2 {
+        let top_row_start = y * stride;
+        let bottom_row_start = (height - y - 1) * stride;
+
+        // Swap rows
+        row_buffer.copy_from_slice(&pixels[top_row_start..top_row_start + stride]);
+        pixels.copy_within(bottom_row_start..bottom_row_start + stride, top_row_start);
+        pixels[bottom_row_start..bottom_row_start + stride].copy_from_slice(&row_buffer);
+    }
+}


Why is this fallback needed?

There should be three code paths:

Rendering with servo::SoftwareRenderingContext, which should not require flipping an image around (an OpenGL "feature").

Rendering with OpenGL conversion to Vulkan texture, where the call to blit_named_framebuffer will do the y-flip on the GPU.

Rendering with OpenGL and conversion to Metal Texture, where create_flipped_texture_render() takes care of the flipping on the GPU.

tronical · 2025-12-05T10:24:31Z

examples/servo/build.rs


        // Config copied from https://github.com/YaLTeR/bxt-rs/blob/9f621251b8ce5c2af00b67d2feab731e48d1dae9/build.rs.

+        let api = if target_os == "android" { Api::Gles2 } else { Api::Gl };


In this code, the decision is "sort of" made at run-time by comparing strings. However, this can be done at compile-time, using either #[cfg(target_os = "android"] or using if cfg!(target_os = "android").

Given that either branch is compilable, I think this should be written like this:

let api = if cfg!(target_os = "android") { ... } else { ... };

That said, I wonder if we can get away with just OpenGL ES 3.0 everywhere. On Android and Linux Embedded that'll be preferred. On Windows I thought servo is using angle, so that's also OpenGL ES.

@expenses I'm unsure, but is there perhaps a reason to use "Desktop GL" over GLES here?

I'm not too familiar with the different OpenGL versions. I believe that if we're only using the GLES feature set and nothing exclusive to Desktop GL then using GLES everywhere is fine.

burhankhanzada added 26 commits December 4, 2025 13:51

fix: fix wgpu_texture to run on android emulator on mac with disable …

da16c24

…push constants

refactor: remove cfgs for android to enable hardware rendering

8fb53c0

refactor: Make GL binding generation unconditional for all targets an…

eb32bed

…d update build dependencies.

refactor: Update servo and its depncies and code for ne APi changes

ec07860

wip: Fix crash on android

bf4cc19

refactor: use hardware rendering wiht NativeWidget

457b65a

fix: Upside down texture

0e1f2f2

refactor: remove native widget

e3474af

refactor: bring back original linux implementation

881163b

refactor: clean up

515a842

refactor: clean up

b41b71d

refactor: run cpu fallback on emulator

ebabf4f

feat: Added vulkan module to consolidate vulkan related code

1f493c6

refactor: improve code and fix metal crash

01e4ec4

refactor: improve imports

b83c59a

refactor: improve code

4ecda44

fix: added insets

8cd711c

refactor: clean up surfman_context remove dead code

db989ec

refactor: improve code

d427263

refactor: improve code

3f1577c

refactor: Extract image flipping, and into shared utility functions.

d666278

refactor: move Vulkan image creation and OpenGL import logic into a n…

88b9812

…ew safe helper function.

refactor: improve imports

d198c1b

refactor: improve metal errors handling

75a6cf9

docs: add docs

ef08457

Revert "fix: fix wgpu_texture to run on android emulator on mac with …

a44d2ce

…disable push constants" This reverts commit 57ac9c7.

burhankhanzada requested review from expenses and tronical December 4, 2025 12:57

[autofix.ci] apply automated fixes

2ec55d8

burhankhanzada changed the title ~~Servo android hardware rendering and refactor~~ servo: android hardware rendering and refactor Dec 4, 2025

fix: remove local surfman and raw-handle

34bcb4f

expenses reviewed Dec 4, 2025

View reviewed changes

tronical reviewed Dec 5, 2025

View reviewed changes


		// Config copied from https://github.com/YaLTeR/bxt-rs/blob/9f621251b8ce5c2af00b67d2feab731e48d1dae9/build.rs.

		let api = if target_os == "android" { Api::Gles2 } else { Api::Gl };

servo: android hardware rendering and refactor #10201

Are you sure you want to change the base?

servo: android hardware rendering and refactor #10201

Uh oh!

Conversation

burhankhanzada commented Dec 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

burhankhanzada Dec 4, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

burhankhanzada Dec 4, 2025 •

edited

Loading