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 - Text Rendering: Use texture atlas for characters #53

Closed
bryphe opened this issue Nov 16, 2018 · 5 comments
Closed

Performance - Text Rendering: Use texture atlas for characters #53

bryphe opened this issue Nov 16, 2018 · 5 comments
Assignees
Labels
A-rendering Area: Rendering artifacts, features etc. A-text Area: Text rendering skia-dependent This issue may be impacted or note relevant after Skia is integrated.

Comments

@bryphe
Copy link
Member

bryphe commented Nov 16, 2018

Rendering text is currently very expensive in Revery, because it involves lots of context-switches to jump between shaders (this is made even worse by the fact that we currently regenerate textures every frame - but that's a separate issue).

The text rendering could be significantly improved by having a texture atlas that contains all the rendered glyphs - then, we could render a line of text in a single pass (or at least, a more minimal set of passes), as opposed to the situation today - where we always render each quad / texture in a single pass.

There's an excellent TextureAtlas implementation by @cryza in Oni here: https://github.com/onivim/oni/blob/master/browser/src/Renderer/WebGLRenderer/TextRenderer/GlyphAtlas/GlyphAtlas.ts that could be useful here 👍

@manu-unter
Copy link
Collaborator

I've started working on a Reason port of the v1 texture atlas. I'll tell you once I've come up with something reviewable! It will probably require some changes in reason-fontkit and reglfw as well, though 😅

@bryphe
Copy link
Member Author

bryphe commented Dec 19, 2018

@cryza - awesome! Thank you very much for your help with this 💯

@manu-unter manu-unter self-assigned this Dec 29, 2018
@manu-unter
Copy link
Collaborator

manu-unter commented Dec 29, 2018

It took me quite some time but I think I now have fiddled around enough in a very diffuse way so that I now have a good understanding of what we can do to get a similar approach working as we had in oni v1.

Originally, I wanted to just take the logic we already have there in TypeScript and port it to Reason. That approach failed for now though because we don't yet have all the plumbing available to do that and also because we already have differing abstractions in place that I would be avoiding that way. Anyhow, trying around with it gave me some insights:

I was originally planning on adding support for heterogeneous data inside vertex attribute buffers because both WebGL and OpenGL support mixing different data types within a single vertex attribute buffer. This means a vertex shader could use a UINT_8 along with a FLOAT_32 which are both read from the same buffer. I decided to postpone that to keep this issue focused for now though. Just writing everything into the buffer as a float should be sufficient for now. We might want to revisit this later.

I was also planning to alter the complete rendering logic. What we have now is:

  1. Transfer the data for a single glyph to the GPU as uniforms
  2. Run the text shader for the given glyph

Which results in one GPU upload and one rendering pass per glyph.

I was going to recreate the instanced logic we have in oni v1, which works as follows:

  1. Store all the rendering parameters for all glyphs in succession a single vertex attribute buffer and upload them in bulk
  2. Run the text shader in instanced mode for all the glyphs at once

This logic requires support for the instanced variants of the GL API, which is unfortunately a little troublesome. In oni v1, we could rely on our chromium version providing that API through WebGL 2 but Edge, Internet Explorer and also Safari don't yet support WebGL 2: https://developer.mozilla.org/en-US/docs/Web/API/WebGL2RenderingContext#Browser_compatibility
OpenGL compatibility shouldn't be an issue

This means for now, we need to decide:

  • Keep one render pass per glyph and stay compatible with IE, Edge and Safari
  • Optimise to one render pass per text node and drop compatibility with IE, Edge and Safari

We could also start with the first option because it would also mean less effort to implement and then later check if we actually need more performance at the moment. Then we can still decide if we want to revisit the second option.

What do you say?

@bryphe
Copy link
Member Author

bryphe commented Jan 1, 2019

It took me quite some time but I think I now have fiddled around enough in a very diffuse way so that I now have a good understanding of what we can do to get a similar approach working as we had in oni v1.

Awesome! Thanks for the update @cryza .

I decided to postpone that to keep this issue focused for now though. Just writing everything into the buffer as a float should be sufficient for now. We might want to revisit this later.

Sounds reasonable to me for now.

This means for now, we need to decide:

  • Keep one render pass per glyph and stay compatible with IE, Edge and Safari
  • Optimise to one render pass per text node and drop compatibility with IE, Edge and Safari

Thanks for calling out the options here! Given that the first option sounds like it is less effort to implement for now, I'm on-board with pursuing that. Getting a 'texture atlas' in place gives us a big leg-up when we decide to implement more aggressive optimizations! This also keeps the set of changes more incremental.

One thing to note - there is an extension for WebGL 1 for ANGLE_instanced_arrays, which I believe would add sufficient instancing support for our scenarios. It seems to be very widely supported according to webgl stats: https://webglstats.com/webgl/extension/ANGLE_instanced_arrays - so that might be a future possibility. I logged an issue in the reason-glfw repo to pick this up eventually: https://github.com/bryphe/reason-glfw/issues/27

@akinsho akinsho pinned this issue Jan 2, 2019
@bryphe bryphe unpinned this issue Jan 3, 2019
@bryphe bryphe added the skia-dependent This issue may be impacted or note relevant after Skia is integrated. label Jun 6, 2019
@glennsl glennsl added A-rendering Area: Rendering artifacts, features etc. A-text Area: Text rendering labels Nov 26, 2019
@bryphe
Copy link
Member Author

bryphe commented Jan 30, 2020

Skia integrated in #567 handles this for us, now!

@bryphe bryphe closed this as completed Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rendering Area: Rendering artifacts, features etc. A-text Area: Text rendering skia-dependent This issue may be impacted or note relevant after Skia is integrated.
Projects
None yet
Development

No branches or pull requests

3 participants