-
Notifications
You must be signed in to change notification settings - Fork 51
fix(doc): SVG images not rendering #193
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
Conversation
Summary of ChangesHello @junrushao, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue where SVG images embedded in the documentation were failing to render correctly. The changes focus on modifying the image source URLs to ensure their proper display, specifically by adjusting the path structure and adding a sanitization parameter to the raw GitHub content links, based on suggestions to resolve common SVG rendering problems. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request correctly fixes an issue where SVG images were not rendering in the documentation by appending ?sanitize=true to the image URLs. This works because it forces GitHub to serve the images with the correct image/svg+xml content type, which is required for browsers to render them. While this change is effective, I've added a comment suggesting a more robust long-term solution: including the images directly within this repository. This would make the documentation self-contained and remove the dependency on raw.githubusercontent.com.
| - Last 64 bits: payload that is either a 64-bit integer, a 64-bit floating-point number, or a pointer to a heap-allocated object. | ||
|
|
||
| .. figure:: https://raw.githubusercontent.com/tlc-pack/web-data/refs/heads/main/images/tvm-ffi/stable-c-abi-layout-any.svg | ||
| .. figure:: https://raw.githubusercontent.com/tlc-pack/web-data/main/images/tvm-ffi/stable-c-abi-layout-any.svg?sanitize=true |
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.
While adding ?sanitize=true fixes the rendering, a more robust approach is to store the images within this repository, for example in a docs/_static/images/ directory. This makes the documentation self-contained, removes the dependency on raw.githubusercontent.com, and avoids potential issues with GitHub's file serving policies in the future. You could then reference the image with a relative path, for example: .. figure:: /_static/images/tvm-ffi/stable-c-abi-layout-any.svg.
|
turns out it still doesn't work |
|
Here's what chatgpt says: This happens because the published site is trying to load your SVG from a third‑party origin ( Best fix (recommended)Serve the image from the docs site itself (no hotlinking).
.. figure:: ../_static/img/tvm-ffi/stable-c-abi-layout-any.svg
:alt: Layout of the 128-bit Any tagged union
:name: fig:layout-anySphinx will copy local images into the build output and rewrite the
If you need a single source of truth for images (but still comply with CSP)Option A — Git submodule or sync step
git submodule add https://github.com/tlc-pack/web-data docs/_static/web-dataThen in your .. figure:: ../_static/web-data/images/tvm-ffi/stable-c-abi-layout-any.svg
:alt: Layout of the 128-bit Any tagged union
:name: fig:layout-anyOr add a pre‑build step (in CI) that Option B — Have Sphinx download remote images at build time If you really want to keep URLs in the docs, use an extension that downloads remote images during the build and serves them locally.
What not to do (or why the current approach breaks)
Bonus: if you also build PDFLaTeX can’t embed SVG directly. If you keep SVGs, add one of these to extensions += ['sphinx.ext.imgconverter'] # uses ImageMagick
# or
extensions += ['sphinxcontrib.rsvgconverter'] # converts SVG -> PDF([sphinx-doc.org]5) Bottom line: Move (or vend) the image into the docs repository and reference it locally. That’s the simplest, CSP‑compliant, and most robust fix for |
It turns out that SVG images below [link]:
are not rendering properly.
Not entirely sure why. This commit tries out some fixes chatgpt suggests...