terrastruct / d2

D2 is a modern diagram scripting language that turns text to diagrams.
https://d2lang.com
Mozilla Public License 2.0
16.2k stars 402 forks source link

render: support dark themes #616

Closed vfosnar closed 1 year ago

vfosnar commented 1 year ago

Tracking what needs to be done to merge #613:

vfosnar commented 1 year ago
  • update tests

it looks like tests are failing due to the change of background color property from "white" to "#FFFFFF" which is now being taken from the theme

vfosnar commented 1 year ago

I need some help with the rendering process of SVG. Previous structure of SVG was something like this:

<svg>
... styles and code
... elements
</svg>

But now I need to apply a color that will propagate to the children

<svg>
... styles and code
<rect> - for background and foreground colors
    ... elements
</rect>
</svg>

And this does not work and renders the background over the elements inside. I played with it for like 2 minutes max so there is probably an easy solution.

And the limited solution I use now:

<svg>
... styles and code
<rect /> - for background only
... elements
</svg>

This can also be solved by adding classes to the LaTeX elements directly but that's something I can take a look at later.

vfosnar commented 1 year ago

By the way I fricked the math test up... but idc about marks so no problem to me:p

alixander commented 1 year ago

But now I need to apply a color that will propagate to the children

i don't think that should be the expectation/behavior. we want children to be distinct color from background, giving clarity on depth.

putting a rect is the way to do this as you've done. svg has no z-index, so it's just order of drawing. what's your concern with that solution?

vfosnar commented 1 year ago

Oh I meant the text color, because LaTeX formulas don't have it defined and by default use the parent's one.

20230109_19h50m21s_grim

I can just apply it to the formula (or like the svg parent) directly but I didn't want to do that as that's not what was in the code before.

alixander commented 1 year ago

ah i see. yes, the text stroke color should be used there

vfosnar commented 1 year ago

NO WAY. You mistyped that value. This was my first contribution to an open-source project and you totally made my day!

vfosnar commented 1 year ago

Alright, the hardest question. Should we call it a --dark_theme or --theme_dark in the CLI.

I actually don't know because writing --theme 0 --dark_theme 200 feels kinda wrong.

vfosnar commented 1 year ago

Some working prototype with --theme 0 --dark_theme 100 -s:

(pro tip: try changing the webpage theme)

sql

And a catppuccin one --theme 4 --dark_theme 200 -s:

class

vfosnar commented 1 year ago
d2 --dark_theme 200 source.d2 out.png
err: bad usage: --dark_theme cannot be used while exporting to another format other than .svg
err: Run with --help to see usage.

And the export problem is fixed.

vfosnar commented 1 year ago

Markdown!

20230110_00h15m41s_grim

vfosnar commented 1 year ago
* [ ]  fix the background bug - find it by: TODO the background is not rendered all over the image

@alixander I will need some help with this one. I Have no clue how to debug something like that.

I added a rect element (backgroundEl) to fill the background color. Even though I set its dimensions to match the SVG viewbox it does not render large enough to cover the whole area. I tried to make it a bit larger than the viewbox but it looks like it fails at larger scales. You can try to reproduce it with this sheet I use for testing (layout engine is the default one):

amscd plugin: {
  ex: |tex

    \\begin{CD} B @>{\\text{very long label}}>> C S^{{\\mathcal{W}}_\\Lambda}\\otimes T @>j>> T\\\\ @VVV V \\end{CD}

  |
}

braket plugin: {
  ex: |tex

    \\bra{a}\\ket{b}

  |
}

cancel plugin: {
  ex: |tex

    \\cancel{Culture + 5}

  |
}

color plugin: {
  ex: |tex

    \\textcolor{red}{y} = \\textcolor{green}{\\sin} x

  |
}

gensymb plugin: {
  ex: |tex

    \\lambda = 10.6\\,\\micro\\mathrm{m}

  |
}

mhchem plugin: {
  ex: |tex

    \ce{SO4^2- + Ba^2+ -> BaSO4 v}

  |
}

physics plugin: {
  ex: |tex

    \\var{F[g(x)]}

    \\dd(\\cos\\theta)

  |
}

multilines: {
  ex: |tex

    \\displaylines{x = a + b \\\\ y = b + c}

    \\sum_{k=1}^{n} h_{k} \\int_{0}^{1} \\bigl(\\partial_{k} f(x_{k-1}+t h_{k} e_{k}) -\\partial_{k} f(a)\\bigr) \\,dt

  |
}

explanation: |md
  # h1
  ## h2
  ### h3
  #### h4
  ##### h5
  ###### h6

  > Lorem ipsum dolor sit amet, consectetur adipiscing elit.

  [fosny.eu](https://fosny.eu)

  - lists
  - lists

  \`\`\`go
  awsSession := From(c.Request.Context())
  client := s3.New(awsSession)

  ctx, cancelFn := context.WithTimeout(c.Request.Context(), AWS_TIMEOUT)
  defer cancelFn()
  `\`\`
|

code: |go
awsSession := From(c.Request.Context())
client := s3.New(awsSession)

ctx, cancelFn := context.WithTimeout(c.Request.Context(), AWS_TIMEOUT)
defer cancelFn()
|
alixander commented 1 year ago

Should we call it a --dark_theme or --theme_dark in the CLI.

--dark-theme. existing flags have convention of

  1. hyphenated (e.g. --dagre-nodesep)
  2. subject before descriptor (e.g. D2_LAYOUT instead of LAYOUT_D2).

those examples you showed show up as black for me (i'm on OSX with forced appearance light theme)

Screen Shot 2023-01-09 at 4 38 20 PM

--dark_theme cannot be used while exporting to another format other than .svg

why not?

markdown looks great!

Even though I set its dimensions to match the SVG viewbox

You should use this function to get the bounding box and set to that. https://github.com/terrastruct/d2/blob/cfed25be63fd4ab810c1e07eac0f456a00688ec6/d2target/d2target.go#L48

On top of setting the dimensions, you have to set the coordinates to match the viewbox.

vfosnar commented 1 year ago

those examples you showed show up as black for me (i'm on OSX with forced appearance light theme)

They show up black when you open them in a new browser window due to github's security policy blocking it. But they should render properly in this thread. This is not a problem with the program if it is the way I described it. Please let me know so I can investigate it @alixander.

Edit: added the error message.

This is the error message in Firefox:

Content Security Policy: The page’s settings blocked the loading of a resource at inline (“default-src”).

This is due to the content-security-policy: default-src 'none'; script-src 'none'; img-src 'self'; media-src 'self'; sandbox header.

why not?

because --dark_theme means "what to do when the user prefers a dark theme". If you want to render a png with a dark theme you should set the primary theme (--theme) to an id of a dark theme and leave --dark_theme empty.

markdown looks great!

Thanks!

You should use this function to get the bounding box and set to that.

Didn't notice that, thanks, will look into it today.

vfosnar commented 1 year ago

You should use this function to get the bounding box and set to that.

I reread my code and I actually used that so this was not the problem.

I looked into it and it seems like I mixed two bugs that are independent of each other.

So:

  1. Firefox for some reason adds some sort of padding to the SVG when opened directly in the browser (like file:///path/to/file.svg). This doesn't happen in chromium based browsers
  2. Generated SVGs placed into an HTML document add some sort of bottom padding that cannot be traced to any existing element in the DOM, but this padding shifts elements beneath it. This happens in both Gecko-based and Chromium-based browsers.

Well I don't care about previews of SVGs so the first one doesn't matter. But I was scared I fricked something up because of the second one. I compared my results to both the current branch and the stable version and it seems like this wasn't working already, so I won't try to patch it ATM.

Results of D2 from a stable version (see that blue line between those diagrams):

image

vfosnar commented 1 year ago

I think I will be able to finish #505 today and I also want to add #101 before the merge due to inner implementation of themes. I hope I can finish this by tomorrow because then I have my part-time job.

alixander commented 1 year ago

let's not put #101 in same PR, it'll be easier to review one at a time

vfosnar commented 1 year ago

btw the way I implemented the themes should make it easier in the future to support viewers without CSS, though this would be a lot of work