search

NixOS / nixos-homepage

Sources for nixos.org
https://nixos.org
286 stars 305 forks source link

Force GPU rendering to fix asciinema on Safari 16 and below #1378

Closed albertchae closed 2 months ago

albertchae commented 3 months ago

Use translateZ to force GPU rendering in Safari to fix asciinema rendering

Follow up to remaining issue on https://github.com/NixOS/nixos-homepage/issues/1323

It looks like newer versions of safari have fixed this https://github.com/asciinema/asciinema-player/issues/258 but if we want to support older versions, this translateZ GPU hack seems to work, at least for keeping the black terminal part within the parent containers border.

The sizing of the player font can still be incorrect, so one workaround is to make the cover.scenario asciicast slightly wider than necessary to accommodate

IMO we should only merge this if we expect a decent amount of traffic from Safari 16 and older

Recordings

  1. Previous buggy behavior https://github.com/asciinema/asciinema-player/issues/258#issue-2216829767
  2. After using translateZ, the terminal rendering doesn't bleed to the right anymore, but you can see that some of the text overflows sometimes

https://github.com/NixOS/nixos-homepage/assets/217050/0c165e9e-12b8-4da4-9003-698db2ad1a18

  1. After making the cover.scenario wider than technically necessary, the text doesn't overflow

https://github.com/NixOS/nixos-homepage/assets/217050/62e14f5c-3f47-48ec-91b8-bbd0b24b3d89

github-actions[bot] commented 3 months ago

šŸš€ Deployed on https://660a927275d7e194e13c8247--nixos-homepage.netlify.app

albertchae commented 3 months ago

@thilobillerbeck @garbas do we have any data on the breakdown of browser + version of visitors to nixos.org? If the number of users on Safari < 16 is a small percentage, maybe we should just close this PR instead of merging a hack

github-actions[bot] commented 3 months ago

šŸš€ Deployed on https://660e4c06ca68cdae073735da--nixos-homepage.netlify.app

thilobillerbeck commented 3 months ago

I just did not have the time to review, will do tomorrow

albertchae commented 3 months ago

I just did not have the time to review, will do tomorrow

@thilobillerbeck oh sorry just to be clear I wasn't making a comment about code review time, just wondering if it's worth doing the translateZ hack if it turns out most (maybe > 99%?) Safari visitors to nixos.org are on the latest version. I usually give open source PRs at least 1-2 weeks before pinging for reviews so no worries there :D

github-actions[bot] commented 2 months ago

šŸš€ Deployed on https://6616df38040b5c2926ecab6d--nixos-homepage.netlify.app

albertchae commented 2 months ago

Looks alright, though I am not sure if we want to merge this or not, it seems like a pretty minor issue that would be rarely noticed if at all and is patched already on a browser level anyway

I agree about possibly not merging it because it is already patched in the browser but it is most noticeable on ios mobile views, e.g. an iPhone 14 Pro

And the other thing to consider is that on iOS, all browsers (including Firefox and Chrome) use webkit under the hood (at least prior to this EU ruling)

And AFAICT the only way to upgrade to Safari 17 + the associated webkit is to do a full iOS or macOS major version upgrade. You can't just upgrade the browser.

So those are the only reasons it might be worth doing this hack, if we have the user traffic from these browser versions to support it.