#241 Add macOS-specific fixes and tests for Kaleido compatibility #289

joaquinbejar · 2025-05-11T17:54:01Z

Hi 👋,

This PR fixes #241 by omitting --disable-gpu on macOS + arm64.

spec.json:

{"format":"png","width":800,"height":600,"scale":1.0,"data":{"config":{},"data":[{"name":"Surface","type":"surface","x":[1.0,2.0,3.0],"y":[4.0,5.0,6.0],"z":[[1.0,2.0,3.0],[4.0,5.0,6.0],[7.0,8.0,9.0]]}],"layout":{}}}

cat spec.json | ./kaleido plotly \
--disable-gpu \
--allow-file-access-from-files \
--disable-breakpad \
--disable-dev-shm-usage \
--disable-software-rasterizer \
--single-process \
--no-sandbox \
> out.b64

cat out.b64

{"code": 0, "message": "Success", "result": null, "version": "0.2.1"}
{"code":525,"message":"error creating static canvas/context for image server","pdfBgColor":null,"format":"png","result":null,"width":800,"height":600,"scale":1}

so we never get any image data. The fix is still to omit --disable-gpu, but I wanted to be precise in the description.

What the patch changes

// non‑macOS keeps the original flag set
#[cfg(not(target_os = "macos"))]
let cmd_args = vec![
    "plotly",
    "--disable-gpu",
    "--allow-file-access-from-files",
    "--disable-breakpad",
    "--disable-dev-shm-usage",
    "--disable-software-rasterizer",
    "--single-process",
    "--no-sandbox",
];

// macOS (Intel **and** Apple Silicon) – drop `--disable-gpu`
#[cfg(target_os = "macos")]
let cmd_args = vec![
    "plotly",
    "--allow-file-access-from-files",
    "--disable-breakpad",
    "--disable-dev-shm-usage",
    "--disable-software-rasterizer",
    "--single-process",
    "--no-sandbox",
];

Only the --disable-gpu flag is removed on macOS; everything else remains untouched.
Tested on M1, M2 Pro and M4 Max – write_image now succeeds and the tests pass.
Behaviour on Linux and Windows is unchanged.

Link to upstream discussion

plotly/Kaleido#323

Let me know if you’d prefer the macOS check to be target_arch = "aarch64" only (i.e. keep the flag for Intel Macs). Happy to adjust!

— Joaquín

Introduce macOS-specific arguments for Kaleido to address issue #323. Add new tests to validate image generation (e.g., PNG, JPEG, SVG, PDF) on macOS, ensuring proper functionality. This improves cross-platform support and resolves inconsistencies in the Kaleido backend. issue: plotly#241

Cleaned up unnecessary blank lines and adjusted minor formatting issues to improve code readability and maintain consistency. No functional changes were made.

Introduced a macOS-specific `create_test_surface` function in `plotly_kaleido` and adjusted test-related imports in `plotly`. Ensures compatibility with macOS while keeping non-macOS logic intact.

Reorganized and added conditional imports to improve clarity and maintain consistent functionality across platforms. This ensures the required dependencies are properly included, particularly handling the macOS-specific conditions.

Reorganized imports in the tests module to improve clarity and maintain consistency. Non-standard library imports are now grouped separately, following a logical and readable structure.

andrei-ng · 2025-05-12T08:22:20Z

Hi @joaquinbejar ,

Thank you for the PR! Interesting find!

Let me know if you’d prefer the macOS check to be target_arch = "aarch64" only (i.e. keep the flag for Intel Macs). Happy to adjust!

target_os = "macos" is fine for now. If there will be issues, we will change it later.

I have only one commet regarding the PR. I see that you have added new unittests. Any particular reason why you chose to do that and not reuse the existing ones? Do you mind if I merge your tests with the previous ones ( I see you have added some nice extra asserts)?

joaquinbejar · 2025-05-12T08:27:14Z

HI @andrei-ng

Sure, that makes sense!

I added the new tests because the failure reliably showed up with a Surface trace, while the existing tests use only Scatter. I wanted to prove the fix on the exact setup that was breaking.

Feel totally free to merge my extra assertions into the existing test file (or replace it entirely) — I just separated them to keep the repro crystal-clear. Thanks for the review!

andrei-ng · 2025-05-12T08:31:32Z

HI @andrei-ng

Sure, that makes sense!

I added the new tests because the failure reliably showed up with a Surface trace, while the existing tests use only Scatter. I wanted to prove the fix on the exact setup that was breaking.

Feel totally free to merge my extra assertions into the existing test file (or replace it entirely) — I just separated them to keep the repro crystal-clear. Thanks for the review!

OK, understood. Thank you for clarifying! I will remove the old unittests in favour of yours and merge it afterwards.

- keep only the kaleido unittests added in PR #289 in favor of previous ones Signed-off-by: Andrei Gherghescu <8067229+andrei-ng@users.noreply.github.com>

joaquinbejar mentioned this pull request May 11, 2025

Kaleido doesn't generate the image output on MacOS #241

Open

joaquinbejar added 4 commits May 11, 2025 20:10

Refactor code formatting and remove extraneous whitespace.

70f9d80

Cleaned up unnecessary blank lines and adjusted minor formatting issues to improve code readability and maintain consistency. No functional changes were made.

Add macOS-specific test surface creation and adjust imports

2cb95e6

Introduced a macOS-specific `create_test_surface` function in `plotly_kaleido` and adjusted test-related imports in `plotly`. Ensures compatibility with macOS while keeping non-macOS logic intact.

Refactor import statements in tests module for readability.

3a5f7f7

Reorganized imports in the tests module to improve clarity and maintain consistency. Non-standard library imports are now grouped separately, following a logical and readable structure.

andrei-ng approved these changes May 12, 2025

View reviewed changes

andrei-ng merged commit a7ac37d into plotly:main May 12, 2025
17 of 19 checks passed

andrei-ng added a commit that referenced this pull request May 12, 2025

remove disable-gpu flag for all targets

f4ed8b0

- keep only the kaleido unittests added in PR #289 in favor of previous ones Signed-off-by: Andrei Gherghescu <8067229+andrei-ng@users.noreply.github.com>

andrei-ng mentioned this pull request May 12, 2025

remove disable-gpu flag for all targets #291

Merged

andrei-ng added a commit that referenced this pull request May 12, 2025

remove disable-gpu flag for all targets

ad43939

- keep only the kaleido unittests added in PR #289 in favor of previous ones Signed-off-by: Andrei Gherghescu <8067229+andrei-ng@users.noreply.github.com>

andrei-ng added a commit that referenced this pull request May 12, 2025

remove disable-gpu flag for all targets (#291)

1ecb8ac

- keep only the kaleido unittests added in PR #289 in favor of previous ones Signed-off-by: Andrei Gherghescu <8067229+andrei-ng@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

#241 Add macOS-specific fixes and tests for Kaleido compatibility #289

#241 Add macOS-specific fixes and tests for Kaleido compatibility #289

Uh oh!

joaquinbejar commented May 11, 2025

Uh oh!

andrei-ng commented May 12, 2025

Uh oh!

joaquinbejar commented May 12, 2025 •

edited

Loading

Uh oh!

andrei-ng commented May 12, 2025

Uh oh!

Uh oh!

Uh oh!

#241 Add macOS-specific fixes and tests for Kaleido compatibility #289

#241 Add macOS-specific fixes and tests for Kaleido compatibility #289

Uh oh!

Conversation

joaquinbejar commented May 11, 2025

What the patch changes

Link to upstream discussion

Uh oh!

andrei-ng commented May 12, 2025

Uh oh!

joaquinbejar commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrei-ng commented May 12, 2025

Uh oh!

Uh oh!

Uh oh!

joaquinbejar commented May 12, 2025 •

edited

Loading