Skip to content
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

Block diagram update #2221

Merged

Conversation

marnovandermaas
Copy link
Contributor

My hackday project, which I think improves the block diagram formatting without doing any major re-organization.

Copy link
Contributor

@elliotb-lowrisc elliotb-lowrisc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks nice in Inkscape on my local computer, but has problems when rendered on GitHub: https://github.com/lowRISC/ibex/tree/c06a6b5c2efa093e0e5a686e3c208b7a6b37cebf

Note the loss of white background behind the "Instruction Memory Interface" box, the "Instruction Fetch" text is not shown as Exo 2 and is overrunning the edge of its containing box, and the tiny text below the Prefetch Buffer is not rendered at all.

Copy link
Contributor

@elliotb-lowrisc elliotb-lowrisc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the CSS approach I suggested for the font doesn't seem to work on GitHub after all.

@marnovandermaas
Copy link
Contributor Author

Ok, I've done the boring thing and moved back to liberation sans.

Copy link
Contributor

@elliotb-lowrisc elliotb-lowrisc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. debug_req_i is unreadable on a dark background (e.g. dark-mode GitHub). Suggest increasing size back to what it was and adding either a white outline or background shape.
  2. Text below Prefetch Buffer not rendering on GitHub. Suggest looking for layer issues.

Nits:

  1. I liked the sans-serif "Ibex Core" title better.
  2. "PMP Check" text is rather large (in both places).

Copy link
Contributor

@elliotb-lowrisc elliotb-lowrisc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good

- Move background to its own layer
- Make font sizes consistent
- Fix icache and pc background
  Previously the background was morphed around the text, this makes it a
  background again.
- Remove redundant rectangle
  The instruction memory interface had two rectangles, one black and one
  purple. I removed the purple one that was bleeding through in the
  corners.
- Instruction fetch alignment
  The Instruction fetch block was not the same height and was not top
  aligned with the other blocks.
- Align text with boxes
  This essentially aligns all the text insides the blocks
- Standardize lines as 0.265mm
  The lines between blocks and the ones making the triangular shapes were
  mostly 0.265mm with a few exceptions.
- Stroke width of block outlines same
  Made all the stroke widths for all the blocks 0.5mm. I've made the outer
  box a nice round 1.0mm.
- Use lowRISC colors
  E0384F for the background (including the start of the gradient)
  A21F4F for the outside line
- Alignment of in/out arrows
  Many of these arrows were not aligned, this improves that alignment.
- Add white background to instr inf
  Instruction memory interface lost its white background when the purple
  outline was removed. This commits adds it back in.
- Use Liberation Sans everywhere
  Exo 2 is not supported natively in browsers and there was no easy way to
  embed fonts in SVG where Inkscape knew about it.
- Fade to white, not transparent
- PMP check font is now smaller
- Add background to debug request input
- Make text under prefetcher bigger so it is rendered on GitHub
- Execute text is now its own block so that it is rendered on GitHub
@marnovandermaas marnovandermaas added this pull request to the merge queue Nov 11, 2024
Merged via the queue into lowRISC:master with commit 496e06f Nov 11, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants