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

Use usize for cache keys instead of u64. #69

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

xorgy
Copy link
Collaborator

@xorgy xorgy commented Nov 6, 2024

This improves portability, because AtomicUsize is available on 32-bit targets, but generally AtomicU64 is not.

@xorgy
Copy link
Collaborator Author

xorgy commented Nov 6, 2024

From a safety perspective, I think it's much more likely we run out of memory in a 32-bit process, than exhaust usize font IDs or cache keys, so I don't see this being an issue.
Please don't try to load four billion fonts on a Raspberry Pi 2, and then file an issue about it...

@xorgy xorgy mentioned this pull request Nov 6, 2024
@dfrg
Copy link
Owner

dfrg commented Nov 6, 2024

I’m happy to use an AtomicUsize counter to better support 32-bit targets but I’d prefer to keep the rest of the API the same to avoid breakage. In particular, the [u64; 2] key type is used by parley to store blob ids from peniko. Not ideal and I think we can do better but that’s future work.

@xorgy
Copy link
Collaborator Author

xorgy commented Nov 6, 2024

I can separate the two until we look at a break.

@xorgy xorgy marked this pull request as draft November 6, 2024 22:51
@xorgy
Copy link
Collaborator Author

xorgy commented Nov 6, 2024

I've put up #70 for this non-breaking subset. @dfrg

This improves portability, because `AtomicUsize` is available on
32-bit targets, but generally `AtomicU64` is not.
@xStrom
Copy link
Collaborator

xStrom commented Nov 15, 2024

As main already contains breaking changes, the next Swash release will be breaking anyway. In that light I think including more breaking changes right now makes sense.

Is the Parley/Peniko situation a hard blocker that must be solved first, or can Swash lead the way here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants