Add experimental WebGL Renderer #2120
Conversation
…rformance/webgl-renderer # Conflicts: # browser/src/Editor/NeovimEditor/NeovimEditor.tsx # yarn.lock
|
@cryza , this is awesome! I just had a go with it and it's really great work. I didn't realize how far along this was based on your description... When I ran it, I thought it would be obvious that it was different, but it looked basically the same on my machine (with no cut-off characters, though!) I had to open the debugger to make sure I was actually running the new code. 😄 Not only that, the performance was really impressive in my quick testing... I did some simple benchmarks of just navigating to the top/bottom of a file (triggering a full redraw). For comparison, this is the old timing with the And this was the new timing with the Going from 22 seconds to 2 seconds for that benchmark is an incredible performance improvement. 💯 Thank you for sharing it out! Hope you don't mind if I post a couple questions, just so I can learn more how it works. 😄 |
| 0, | ||
| gl.RGBA, | ||
| gl.UNSIGNED_BYTE, | ||
| this.glyphCanvas, |
There was a problem hiding this comment.
This is cool - if I understand correctly, this is how we can take our 'Canvas' that we render characters / glyphs to, and supply it as a texture that we can read from the shaders?
There was a problem hiding this comment.
That's exactly what we're doing here :) This way we get the nice font rendering from Chrome while at the same time being able to run our editor through WebGL.
The only trade-off is that we need to manually manage our subpixel offsets: We need to manually manage the kerning.
Without additional effort, there is only one version of a character in the atlas, typically aligned to the full pixel boundary. With non-integer character widths however, this would probably ruin the kerning in many cases. A character might at some point need to start at 0.7 pixels offset within the pixel grid. To approximate the correct position, we have different variants of each rendered character in the atlas, each offset by a different fraction of a full pixel. Then we use the one that is closest to what we need at the given position in the editor panel. The number of different variants is controlled by subpixelDivisor in this file.
This idea was completely stolen from xray, BTW! All credit goes to @nathansobo :)
| ) | ||
| this._gl.drawElementsInstanced(this._gl.TRIANGLES, 6, this._gl.UNSIGNED_BYTE, 0, glyphCount) | ||
|
|
||
| this._gl.useProgram(this.textBlendPass2Program) |
There was a problem hiding this comment.
Was curious - are the two passes needed for subpixel anti-aliasing? Or for handling transparency / background?
There was a problem hiding this comment.
The two-pass approach is required to do correct blending of subpixel-antialiased text onto our destination canvas. Basically we are abusing the RGB output channels of the first pass to do some intermediate calculations and blending with the destination for us which we then use to calculate the final result in the second pass.
I will try to find the article again that explained the equations in a nice way and post it here!
|
@bryphe thanks for the great feedback! I'm really happy that you like it :) So I am planning to do a major refactoring of all the low-level WebGL code so that it is more understandable and encapsulated in a better way. I hope that the code will be more maintainable afterwards also for people who didn't do a deep dive into WebGL. I also still see a few quirks and performance improvements we might want to do and possibly also an adaptation of the What I would like to finish before we merge this PR:
|
That sounds great! I like the idea of refactoring to the high-level constructs (will help me out too, I spend some time reading through WebGL 2 docs, but still not proficient yet)
Your plan sounds perfect to me. Adding the experimental configuration flag sounds great - perhaps we could have something like
Interesting - we actually do something kind of crazy and incorporate the line padding into the This is confusing because the Of course, keep me posted if there is anything I can do 👍 Awesome work @cryza ! |
|
@cryza thanks for the shout-out. I’d definitely appreciate a ping on PRs related to this code path so I can learn from your progress. Good luck. |
…rformance/webgl-renderer # Conflicts: # package.json # yarn.lock
package.json
Outdated
| @@ -750,6 +752,7 @@ | |||
| "@types/sinon": "1.16.32", | |||
| "@types/webgl2": "^0.0.3", | |||
| "autoprefixer": "6.4.0", | |||
| "aws-sdk": "^2.202.0", | |||
There was a problem hiding this comment.
Why is this dependency included ?
There was a problem hiding this comment.
Aw this is a merge from master, sorry 😂
|
Thanks for your help @nathansobo - look forward to seeing where |
| const viewportScaleY = -2 / canvasHeight | ||
| this._gl.viewport(0, 0, canvasWidth, canvasHeight) | ||
|
|
||
| this._solidRenderer.draw( |
There was a problem hiding this comment.
Thanks for refactoring this! Definitely helped make it easier to understand 👍
There was a problem hiding this comment.
I'm happy that it helped! I think I'll try to make it even more concise but that depends on how much time I will find for doing that. :)
| } | ||
|
|
||
| private createVertexArrayObject() { | ||
| this._vertexArrayObject = this._gl.createVertexArray() |
There was a problem hiding this comment.
I was doing some research on WebGL today and stumbled across this library: https://github.com/regl-project/regl
I was wondering if you looked at it and what you thought about it? It seems like it might be able to cut out some of the boilerplate here - but I'm not sure if it exposes enough low-level functionality to handle all the cases we need.
There was a problem hiding this comment.
I havent't seen that library before, and it looks really interesting! I actually really dislike the mutative and OO way of programming that the GL APIs enforce, so this hits the spot for me :)
Unfortunately though it seems like there is no WebGL2-compatible version of this library and I currently don't know if our shaders could be rewritten in a WebGL1-compatible way. I had a quick try and changed the typings of the contexts to WebGL1, which resulted in lots of errors. So my gut feeling for this is that it's not really possible, but one would need to do some more investigation into this to be sure.
There was a problem hiding this comment.
WebGL 1 doesn't support instanced rendering as far as we could tell, so it will be a non-trivial change and slightly less efficient to use.
There was a problem hiding this comment.
@nathansobo Great to have you in the loop! Thanks for the input, that probably just saved us a lot of time :)
There was a problem hiding this comment.
Thanks @cryza and @nathansobo ! That's a bummer that regl doesn't support WebGL 2 (I like the functional approach better too) - but definitely a non-starter if we'd have to switch to WebGL 1 👍 Good to know! Definitely saved us time. 😄
|
Refactoring looks great, @cryza ! I like how you split out the functionality between |
|
Okay, this is slowly coming into shape. My progress thus far: I still have the idea to combine the two Things that are still missing for feature parity with the canvas renderer (for other PRs):
Given that the builds complete, this would put the PR in a reviewable state :) |
|
If there is an experimental flag for this, I would suggest merging it into master as soon as the refactoring is complete. I am very eager to try this out! |
Codecov Report
@@ Coverage Diff @@
## master #2120 +/- ##
==========================================
- Coverage 37.27% 36.79% -0.49%
==========================================
Files 288 293 +5
Lines 11608 11957 +349
Branches 1556 1579 +23
==========================================
+ Hits 4327 4399 +72
- Misses 7037 7311 +274
- Partials 244 247 +3
Continue to review full report at Codecov.
|
|
Giving this a try out now and the performance for full-redraws seems great, and I don't seem to get any "cracks" anymore. One thing I've noticed is that updating the screen in small ways, ie with In both cases, forcing a redraw with |
|
I just went through the code again - @cryza , you did an awesome job refactoring and restructuring the code - it's very readable. I really like the way you architected this strategy 👍 And I tested it out locally and it works great. I'm looking forward to experimenting with it in my day-to-day work 😄 I see no reason to wait, so I'll bring this in now - it's gated by the @cryza - amazing work! Really impressed by this change - it's a huge contribution to the project - not only does it lay the foundation for fixing long-standing issues with our rendering pipeline like #2047, #1252, #1083, it also sets a new bar in terms of performance. Thank you! And thanks @nathansobo for your help! Next step - get this enabled by default! 😀 |
|
I don't know if its related but after this change shows this messages when using webgl: |
|
@adelarsq thanks for the report! This is caused by the webgl renderer as you assumed. It currently happens if the texture that contains all the prepared characters runs out of space. Currently we hard-coded the texture size to 512x512 pixels, so for files with lots of different characters, this limit will at some point cause the renderer to throw. Especially so for large font sizes. My current idea is to implement some logic that catches the error at the level of the text renderer, creates a larger texture and restarts rendering. To find a reasonable default size in any case, it would be great if you could share some information about your font size, font family, line padding, if you have a retina display (or some other setup with devicePixelRatio > 1) and ideally also the content of the file, if that is possible. Cheers! |
|
I also hit a similar issue when doing lots of scrolling very rapidly. I'm using Fira Code as my font at 12px, with the default line padding and a 4k monitor (27" if that matters?) at 150% scaling, with Windows 10 64bit. The file was my Oni config, which can be found here: https://github.com/CrossR/dotfiles/blob/master/oni/config.tsx |
|
oni 0.3.4, macOS 10.13.4 Setting |
|
@cryza Just tested master here, was not working on the latest stable (v0.3.4), it's fixed for me! Looking forward to set webgl on my stable setup when it's released, thanks!! |
|
Great to hear, @badosu! Thanks for giving it another try this quickly :) |
This PR is not ready to be merged yet, I will extend the description once it is. Consider this a sneak peek into my work in progress for nowFor anyone feeling adventurous until then: The renderer should be working roughly now, with bold, italics and underline missing, and no performance optimizations or nice refactorings made so far.This PR adds an experimental new renderer as an alternative to the current canvas-based one. I am doing this with the goal of solving several issues that we currently have with the optimizations done in the canvas renderer. First and foremost #1252 and #1083 should be fixed.
This PR intentionally does not cover all the functionality that we have in the canvas renderer yet, I am planning to add all the missing pieces with several smaller PRs afterwards. See the comments below for details in case you want to jump on the train!