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

refactor!: Unbox the text selection property #494

Closed
wants to merge 2 commits into from

Conversation

mwcampbell
Copy link
Contributor

TextSelection used to be much bigger when NodeId was 128-bit. But now we can unbox that property without increasing the total size of PropertyValue. So this PR does that. Also, I decided that the rect accessor should return a reference to the Rect struct rather than a copy. This makes the accessors for large structs more consistent.

@mwcampbell
Copy link
Contributor Author

I no longer think I want to do this. Instead, I'm thinking about moving the rect property to a direct field on Node. That would increase the size of Node by 32 bytes, but would decrease the size of PropertyValue by 8 bytes, from 40 to 32. That would allow property values to be a little more densely packed, with two property values fitting in a typical 64-byte cache line. And nearly every node has a bounding rectangle, so changing that property into a direct field would eliminate a slot in the Vec of dynamic property values for most nodes, meaning that the initial allocation for that Vec would go a little further.

@mwcampbell mwcampbell closed this Dec 14, 2024
@mwcampbell mwcampbell deleted the unbox-text-selection branch December 15, 2024 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant