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

🚀 Performance tuning & bugfix #1111

Merged
merged 3 commits into from
Dec 14, 2016

Conversation

dotcypress
Copy link
Contributor

@dotcypress dotcypress commented Dec 11, 2016

Fix unicode performance (cause separated html element per character is too slow)

🙌 Make Hyper fast again!

UPD: 🚨 this PR also will fix > 5 opened issues

@matheuss @codetheory @ppot @rauchg

@matheuss matheuss added the 🙅‍♀️ Status: On Hold Issue or PR is on hold label Dec 11, 2016
@matheuss
Copy link
Member

@dotcypress just tested here and the performance looks like the same as before 😰 Each braille char is still alone inside its own span 🤔
Also, if I change the font size via Cmd + + and/or Cmd + i I get some weird behavior 😩

screen shot 2016-12-11 at 8 14 19 am

@matheuss matheuss added the 📊 Type: Performance Issue contains information about a performance issue in Hyper label Dec 11, 2016
@dotcypress
Copy link
Contributor Author

@matheuss yes, we still need to wrap each non latin symbols to span.

But now, huge amount of spans created if string contains at least 1 non latin character, see screenshots below, all spans are red 😂

Before:
screen shot 2016-12-11 at 9 51 19 am

After:
screen shot 2016-12-11 at 9 53 43 am

Currently i'm trying to fix issue with increasing/decreasing font size, stay tuned 😎

@dotcypress
Copy link
Contributor Author

dotcypress commented Dec 12, 2016

Update:

I can reproduce bug with vtop and changing font size even on master branch.
screen shot 2016-12-11 at 4 13 57 pm

After font change, some characters are gone, for example:
" CPU USAGE " -> "CPU USAGE"
screen shot 2016-12-11 at 11 33 39 am

But if you try this: curl https://gist.githubusercontent.com/dotcypress/29ed6c767e824b6241b7465f9d036567/raw/c4578a1e43de8cb4f2fb29991b10baffeb195a79/vtop-snapshot.txt | cat you can change font size without that weird behavior. 🤔

@matheuss

@dotcypress dotcypress changed the title 🚀 Performance tuning 🚀 Performance tuning & bugfix Dec 13, 2016
@dimitrieh
Copy link

will this fix slow scrolling in for example micro-editor.github.io and slow performance with cat long files?

@dotcypress
Copy link
Contributor Author

@dimitrieh not fully fixed but speed of scrolling is better than in 1.0.0

@rauchg rauchg merged commit d6316dd into vercel:master Dec 14, 2016
@dotcypress dotcypress deleted the unicode-perfomance-fix branch December 14, 2016 23:05
@b3ll
Copy link

b3ll commented Dec 18, 2016

@dotcypress whilst this is a clever way of speeding things up, not all characters are displayed as px, which causes layout to break. In my case, I have an emoji in the prompt (OS X, 10.12), and this causes all subsequent characters to be displayed wrongly by about half a character's width (or more).

For this solution to work, any special spans should have their width set by either the DOM automatically, or using canvas's text measuring functions, rather than a global special span width.

In the screenshot, the cursor is supposed to be right after the -la, but it ends up being an entire character's width off.

span

@rauchg
Copy link
Member

rauchg commented Dec 18, 2016 via email

@b3ll
Copy link

b3ll commented Dec 18, 2016

perhaps? I'm not entirely sure what the best fix is, I'm not super familiar with current web technologies. I just do know that setting a global CSS style for all specially marked unicode characters won't work.

A CSS style for each character should work, but I can see other languages / character sets causing performance to take a nose dive.

@rauchg
Copy link
Member

rauchg commented Dec 18, 2016 via email

@alexandernst
Copy link

alexandernst commented Dec 18, 2016 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙅‍♀️ Status: On Hold Issue or PR is on hold 📊 Type: Performance Issue contains information about a performance issue in Hyper
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants