Dynamic atlas: Reduce unnecessary objects generated and draw from ImageBitmap

[Tyriar]

This PR removed several of the in-between objects needed to render using the dynamic texture atlas. It's a bit difficult to measure this currently (#1688), the one piece that's very easy to measure is the impact on using a number instead of a string for the cache key:

Before:

After:

I couldn't find anything else that could contribute to improving #1677. I also experimented with drawing using an ImageBitmap but couldn't find big changes in performance so I left it out of the PR: Tyriar@e8d06b6

Fixes #1677

[jerch]

@Tyriar
Benchmarking with my ls -lR /usr/lib (average of 10 runs):

• master: ~850ms for TextRenderLayer._forEachCell
• this PR: ~750ms for TextRenderLayer._forEachCell
• your 1677_bitmap branch: ~550ms for TextRenderLayer._forEachCell

I see a really big difference with the ImageBitmap on Ubuntu with Chrome 68 and nvidia gc. Welcome to browser version + driver differences? Idk.

[Tyriar]

@jerch oh yeah? I felt like I saw that earlier and then didn't... I can continue polishing that up with the fallback if it looks consistent.

[jerch]

@Tyriar: Retested, its stable between 430 - 550 ms now 👍

[bgw]

I think the removal of IGlyphIdentifier should be split into a separate PR since we don't have clear performance numbers for it yet, and it makes the code a lot uglier.

Otherwise, I think this is a good direction. Thanks!

[Tyriar]

@bgw thanks for the review! I marked all the conversations as resolved, let me know if you have more feedback (and feel free to unresolve if you don't think they are 😃).

[Tyriar]

Ready for another round 😃

[bgw]

Thanks for your work, I know there was a lot of back-and-forth here, but I'm pretty happy with the result.

To help test some of this, I'd recommend setting the DynamicCharAtlas's size to something really small, so you can verify that we still behave correctly when the cache is being thrashed.

[Tyriar]

@bgw thanks again for all the feedback, we're in a much better place than at first. A once over of the last couple of commits would be great before I merge 🙂

[bgw]

LGTM, thanks. Did you test it with a small atlas size to verify that there's no obvious bugs when the atlas gets thrashed?

[Tyriar]

@bgw yes, no errors when I went down to a 100x100 texture.