feat(docker): opt-in GPU acceleration via CLOAKBROWSER_GPU_ACCEL#230
feat(docker): opt-in GPU acceleration via CLOAKBROWSER_GPU_ACCEL#230mvanhorn wants to merge 2 commits into
Conversation
Cloak-HQ
left a comment
There was a problem hiding this comment.
Thanks for this — clean implementation and good test coverage on the flag generation side.
A few things before we merge:
1. Did you test this end-to-end in Docker with a GPU?
The unit tests verify the flags get appended correctly, but did you run CloakBrowser with CLOAKBROWSER_GPU_ACCEL=1 on an actual GPU-equipped Docker host and confirm GPU acceleration is working? (e.g. checking chrome://gpu or comparing rendering output)
Also, did you test with the flag enabled on a regular Docker container with no GPU? Our binary ships libEGL.so and SwiftShader so it should degrade gracefully, but want to confirm --use-gl=egl doesn't cause startup issues.
2. launch_context() / launch_context_async() missing gpu_accel
The param was added to launch(), launch_async(), launch_persistent_context(), and launch_persistent_context_async(), but not to launch_context() (line 490) or launch_context_async() (line 589). These list params explicitly and call launch() with named args only, so gpu_accel=True would silently fall into **kwargs and get passed to browser.new_context() instead. The env var path still works, but the kwarg doesn't.
JS side is fine since LaunchContextOptions extends LaunchOptions.
Both functions list params explicitly and call launch()/launch_async() with named args, so gpu_accel=True silently fell into **kwargs and got passed to browser.new_context() instead of the launch path. Add the param to both signatures and propagate to the inner launch call. Add unit tests covering forward propagation and the default-false case. Per maintainer review on CloakHQ#230.
f19e49b to
e603ede
Compare
|
Thanks for the review.
|
Summary
Adds opt-in GPU acceleration for the Docker image. Setting
CLOAKBROWSER_GPU_ACCEL=1(or passinggpu_accel=Truein Python /gpuAccel: truein JS) appends--use-gl=egl,--enable-gpu-rasterization, and on Linux--enable-features=VaapiVideoDecoderto the Chrome args. Defaults stay unchanged.What changed
cloakbrowser/browser.py:954—build_args()reads the env / kwarg and appends the GPU flags. VaapiVideoDecoder is Linux-gated.js/src/args.ts+js/src/types.ts— JS parity (gpuAccel: trueoption, same env var fallback).examples/docker-compose.gpu.yml— new example with NVIDIA Container Toolkitdeploy.resources.reservations.devicesconfig.tests/test_build_args.pycovers env-on, env-off, Linux-only Vaapi, dedupe with existing user-supplied args.js/tests/config.test.tscovers the same matrix on the JS side.Why opt-in
The reporter explicitly asked for opt-in (
CLOAKBROWSER_GPU_ACCEL=1) so users without a GPU container don't get destabilized.Testing
pytest tests/test_build_args.py— 26/26 passed;python3 -m py_compile cloakbrowser/browser.pycleannpm testandnpm run typecheckcouldn't run locally becausevitest/tscaren't in the sandbox. CI will exercise.Fixes #189