riscv / riscv-isa-manual

RISC-V Instruction Set Manual
https://riscv.org/
Creative Commons Attribution 4.0 International
3.66k stars 639 forks source link

Converting riscv-isa-manuals asciidoc results in text-less wavedrom diagrams because of 3rd party bug #1019

Closed damageboy closed 1 year ago

damageboy commented 1 year ago

I realize the the asciidoc versions of the isa-manual is not complete, and I'm mostly reaching out through this issue to @wmat in hopes there is some secret knowledge about wavedrom-cli that could be shared.

In short, I'm trying to generate the asciidoc html/pdf versions locally after replicating the build environment but I'm getting an annoying artifact in both the pdf and the html versions where the wavedrom diagrams are generated without any text rendered inside them:

image

Trying to render this directly through wavedrom-cli replicated the bug when rendering to PNG only and not with SVG, and seems to be a known issue reported in the wavedrom repo and unfixed in node-svg2img.

Is there a change that a PR for switching to generating SVGs instead of PNGs would be accepted for the asciidoc version given this (unresolved) bug and SVGs generally being slightly more friendty (IMO) for zooming in / printing / hidpi in general?

damageboy commented 1 year ago

Just in case it wasn't entirely clear I'm effectively suggesting to submit a PR of all the modified wavedrom files after running:

sed -i 's/^\[wavedrom, ,\]/[wavedrom, ,svg]/' src/images/wavedrom/*.adoc

Unfortunately I did not find an easy way to perform this change globally through a theme file or otherwise, although my asciidoctor is still very shallow.

wmat commented 1 year ago

Unfortunately I don't have any secret knowledge of wavedrom-cli to share. If you're having issues building locally, I'd suggest trying the docker based build environment found here: https://github.com/riscv/riscv-docs-base-container-image

As for switching to all svg imaging, I'm fine with that. In fact, I've been moving more toward using bytefield-svg for diagramming as it allows for more flexibility.

On Sat, Apr 22, 2023 at 6:34 AM damageboy @.***> wrote:

I realize the the asciidoc versions of the isa-manual is not complete, and I'm mostly reaching out through this issue to @wmat https://github.com/wmat in hopes there is some secret knowledge about wavedrom-cli that could be shared.

In short, I'm trying to generate the asciidoc html/pdf versions locally after replicating the build environment but I'm getting an annoying artifact in both the pdf and the html versions where the wavedrom diagrams are generated without any text rendered inside them:

[image: image] https://user-images.githubusercontent.com/125730/233778335-a07faca0-853d-45b4-a3fe-029f2a7ed2ff.png

Trying to render this directly through wavedrom-cli replicated the bug when rendering to PNG only and not with SVG, and seems to be a known https://github.com/wavedrom/cli/issues/22 issue reported in the wavedrom repo and unfixed in node-svg2img https://github.com/fuzhenn/node-svg2img/issues/41.

Is there a change that a PR for switching to generating SVGs instead of PNGs would be accepted for the asciidoc version given this (unresolved) bug and SVGs generally being slightly more friendty (IMO) for zooming in / printing / hidpi in general?

— Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-isa-manual/issues/1019, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAN6ZFYATACWPP47XZYZHDXCOX4XANCNFSM6AAAAAAXHYCU6A . You are receiving this because you were mentioned.Message ID: @.***>

damageboy commented 1 year ago

I'll submit the change to use svg in that case, while not everything is converted bytefield-svg

wmat commented 1 year ago

It just occurred to me that you may be missing some fonts. WavedRom diagrams require the Helvetica font, I believe. Also, I build locally and it works for me.

wmat commented 1 year ago

Note that this change renders all wavedroms without any color, effectively ignoring the "type" attribute. I'm currently looking for a fix.