-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Update cosmic-text
and resvg
#2416
Conversation
Due to the size of glyphon and the added complexity of updating WGPU and Cosmic Text. I was thinking the other day. That Iced should just implement the Text rendering directly in Iced. Glyphon is not a huge amount of code and wouldn't be that hard. It would give us more control and make it easier to update when WGPU or Cosmic Text updates. And with recent security issues coming up with supply chain attacks. Shrinking the dependency tree can be a good thing. |
@wyatt-herkamp I totally understand that it might be useful to avoid lag in updating wgpu/cosmic-text and avoid a possible area for supply chain attacks by handling the text rendering in iced directly. I'd just ask to consider the positive network effects of code sharing the text renderer in addition to those update/supply chain points. For example, people using glyphon but don't contribute to iced have updated dependencies, improved performance, fixed bugs, added new functionality that could benefit iced in the future, etc. For context, I created glyphon to use in a text-heavy visualization for a commercial application, so I'm committed to keeping it updated and performant while still being flexible about new use cases (e.g., specializing certain use cases where it makes sense). |
@grovesNL I have opened a couple of PRs in |
005dd7d
to
acfa97a
Compare
dda9adf
to
4dde876
Compare
This PR updates
cosmic-text
to0.11
andresvg
to0.41
.It seems there may be a bit of a performance regression with this update—I am noticing a ~5% increase in render time in most of the examples.
I created a new "layered text" benchmark and it caught a 2% regression:
The benchmark tests static text, so I suspect the regression caught here may be caused by the hashing of
CacheKey
—which now includes an additionalCacheKeyFlags
field.I suspect regressions here are normal since these libraries are still evolving, getting more complex, and satisfying more use cases. In any case, I figured I'd share my results just in case someone may feel like investigating further.
Also important to mention that I originally updated my
glyphon
fork to catch up to themain
branch upstream, but apparently that introduces yet another regression—a bigger one!I suspect the main culprit here is the additional bind group changes introduced by grovesNL/glyphon#88, specially when using multiple
TextRenderer
instances (precisely the scenario of the benchmark!).